Uh oh!
There was an error while loading. Please reload this page.
[SPARK-28250][SQL] QueryPlan#references should exclude producedAttributes - #25052
[SPARK-28250][SQL] QueryPlan#references should exclude producedAttributes#25052cloud-fan wants to merge 2 commits into
Conversation
cloud-fan
commented
Jul 4, 2019
SparkQA
commented
Jul 4, 2019
Test build #107234 has finished for PR 25052 at commit
|
There was a problem hiding this comment.
It seems like cheating if we override this method. We want to know if the plan is valid or not by this method, overriding this method means we skip this validation.
There was a problem hiding this comment.
Ok. I think it makes sense to me. We should update this comment.
SparkQA
commented
Jul 4, 2019
Test build #107242 has finished for PR 25052 at commit
|
| override def outputPartitioning: Partitioning = child.outputPartitioning | ||
| override def producedAttributes: AttributeSet = AttributeSet(outputObjAttr) |
There was a problem hiding this comment.
Isn't outputObjAttr produced by FlatMapGroupsInRExec?
There was a problem hiding this comment.
This is a no-op override, it's the same as ObjectProducerExec.producedAttributes
SparkQA
commented
Jul 5, 2019
Test build #107247 has finished for PR 25052 at commit
|
HyukjinKwon
commented
Jul 9, 2019
Merged to master. |
What changes were proposed in this pull request?
This is a followup of the discussion in #24675 (comment)
QueryPlan#referencesis an important property. TheColumnPrunningrule relies on it.Some query plan nodes have
Seq[Attribute]parameter, which is used as its output attributes. For example, leaf nodes,Generate,MapPartitionsInPandas, etc. These nodes overrideproducedAttributesto makemissingInputscorrect.However, these nodes also need to override
referencesto make column pruning work. This PR proposes to excludeproducedAttributesfrom the default implementation ofQueryPlan#references, so that we don't need to overridereferencesin all these nodes.Note that, technically we can remove
producedAttributesand always ask query plan nodes to overridereferences. But I do find the code can be simpler withproducedAttributesin some places, where there is a base class for some specific query plan nodes.How was this patch tested?
existing tests