[SPARK-XXXXX][SQL] Reuse AttributeSeq when normalizing a sequence in QueryPlan canonicalization - #58256
Open
yifei-yang-db wants to merge 1 commit into
Open
Conversation
…QueryPlan canonicalization QueryPlan.normalizeExpressions(e, input: AttributeSeq) rewrites an expression's AttributeReference exprIds to positional ordinals using input's exprIdToOrdinal map, an instance-scoped lazy val. Several doCanonicalize implementations normalize a whole sequence with seq.map(QueryPlan.normalizeExpressions(_, attrs)), passing a bare Seq[Attribute]; the implicit Seq[Attribute] => AttributeSeq conversion is then re-applied per element, rebuilding the lookup map over all of attrs each time -- O(seq.size * attrs.size). Normalizing a relation's own output against itself is therefore quadratic in the number of columns and allocates a large amount of transient garbage on the driver for wide, unpruned relations. Add an overload normalizeExpressions(exprs: Seq[T], input: AttributeSeq) that binds the AttributeSeq once (map built once, O(n)), and route the canonicalization call sites through it: LogicalRelation, DataSourceV2ScanRelation, FileSourceScanExec, BatchScanExec, InMemoryRelation, InMemoryTableScanExec, SubqueryBroadcastExec, SubqueryAdaptiveBroadcastExec. The normalized output is identical to the per-element form.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
QueryPlan.normalizeExpressions(e, input: AttributeSeq)rewrites an expression'sAttributeReferenceexprIds to positional ordinals usinginput'sexprIdToOrdinalmap, which is an instance-scopedlazy val. SeveraldoCanonicalizeimplementations normalize a whole sequence withseq.map(QueryPlan.normalizeExpressions(_, attrs)), passing a bareSeq[Attribute]asinput. The implicitSeq[Attribute] => AttributeSeqconversion is then re-applied on every element, so a freshAttributeSeq(and a freshexprIdToOrdinalmap over all ofattrs) is built for each element --O(seq.size * attrs.size).This PR adds an overload
QueryPlan.normalizeExpressions(exprs: Seq[T], input: AttributeSeq): Seq[T]that binds theAttributeSeqa single time (so its lookup map is built once for the whole sequence,O(n)) and routes the affected canonicalization call sites through it:LogicalRelation,DataSourceV2ScanRelation,FileSourceScanExec,BatchScanExec,InMemoryRelation,InMemoryTableScanExec,SubqueryBroadcastExec,SubqueryAdaptiveBroadcastExec.Why are the changes needed?
When a relation's own output is normalized against itself (
output.map(normalizeExpressions(_, output))), the current code is quadratic in the number of output columns. Rebuilding theexprIdToOrdinalmap once per column allocates a large amount of transient garbage on the driver while canonicalizing wide, unpruned relations; binding theAttributeSeqonce makes it linear. The normalized result is unchanged.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Added a
QueryPlanSuitetest asserting the newSeqoverload returns the same result as the per-element form. Existing canonicalization/sameResultcoverage is unchanged.Was this patch authored or co-authored using generative AI tooling?
No.