Skip to content

[SPARK-59452][SQL] Optimize DeduplicateRelations for wide plans - #58753

Open
bhollis-dbx wants to merge 1 commit into
apache:masterfrom
bhollis-dbx:deduplicate-relations-linear-union
Open

[SPARK-59452][SQL] Optimize DeduplicateRelations for wide plans#58753
bhollis-dbx wants to merge 1 commit into
apache:masterfrom
bhollis-dbx:deduplicate-relations-linear-union

Conversation

@bhollis-dbx

@bhollis-dbx bhollis-dbx commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Process UNION children in one left-to-right pass when DeduplicateRelations adds output projections. This preserves branch order, expression-ID deduplication, projection tags, and streaming checkpoint sharing.

Add a direct helper for assigning fresh expression IDs across plans. Use it instead of synthetic self-joins in InlineCTE, ReplaceCTERefWithRepartition, and PushDownJoinThroughUnion.

Why are the changes needed?

The UNION projection path compares every branch with every later branch and repeatedly rebuilds the remaining sequence. This work grows quadratically with the branch count.

Several optimizer paths also construct and analyze synthetic self-joins solely to renew expression IDs on one side.

These costs affect generated compatibility views and other plans with hundreds of UNION branches or wide CTE outputs.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Added focused semantic and allocation-scaling tests covering:

  • UNION branch order and overlapping expression IDs
  • Projection tags and streaming children
  • Equivalence with synthetic self-join deduplication
  • Non-timing allocation bounds

Ran:

  • AnalysisSuite focused DeduplicateRelations tests
  • PushDownJoinThroughUnionSuite
  • InlineCTESuite
  • CTEInlineSuiteAEOff
  • CTEInlineSuiteAEOn

Focused allocation measurements:

  • 500-branch UNION: 39,284,232 → 9,325,920 bytes
  • 500-column right-side renewal: 4,391,304 → 2,394,288 bytes

A local 500-branch compatibility-view benchmark measured median planning time at 303.95 → 289.28 ms. Whole-query allocation was effectively unchanged at 159.70 → 159.69 MiB because relation renewal dominates this workload.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: OpenAI Codex (GPT-5)

@bhollis-dbx bhollis-dbx changed the title [SQL] Optimize DeduplicateRelations for wide plans [SPARK-59452][SQL] Optimize DeduplicateRelations for wide plans Sep 12, 2026
@bhollis-dbx
bhollis-dbx force-pushed the deduplicate-relations-linear-union branch from 0731b49 to 267c9f7 Compare September 12, 2026 00:47
type ExprIdMap = mutable.HashMap[Class[_], mutable.HashSet[Long]]

/** Renews `right` against expression IDs collected from `left`. */
private[sql] def deduplicateRight(left: LogicalPlan, right: LogicalPlan): LogicalPlan = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(non-blocking): This calls renewDuplicatedRelations directly, skipping apply()'s top-level dispatch for Union/Merge/Join cases. For self-dedup (deduplicateRight(plan, plan)) this is correct since we just want ID renewal. The equivalence test covers Project-based plans -- would it be worth adding a case with a nested Union inside the plan to confirm the two paths match?

// Use projection-based de-duplication for Union to avoid breaking the checkpoint sharing
// feature in streaming.
val newChildren =
unionWithChildOutputsDeduplicated.children.foldRight(Seq.empty[LogicalPlan]) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This changes the dedup direction from foldRight (last-child-wins) to left-to-right (first-child-wins). The behavior is equivalent for correctness but the set of branches getting Project wrappers is reversed. Might be worth a one-line note in the PR description.

@shrirangmhalgi shrirangmhalgi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a solid optimization. The O(n²) → O(n) for UNION dedup is a clear win for wide plans, and eliminating the synthetic self-join pattern removes unnecessary plan construction overhead.

Observations (non-blocking)

  1. UNION dedup direction change: The old foldRight kept the last child's attributes and wrapped earlier children; the new map keeps the first child's attributes and wraps later children. Both produce correct unique IDs, but the set of branches that get Project wrappers is different. The new test asserts the new behavior - worth noting in the PR description that this is an intentional direction change, not a preservation.

  2. deduplicateRight traversal difference: The new helper calls renewDuplicatedRelations directly, bypassing apply()'s top-level Union/Merge/Join dispatch. For the self-join use case this is correct. The equivalence test covers Project-based plans - would it be worth adding a case where the plan contains a nested Union to confirm the traversal matches?

  3. Allocation-based tests: The ThreadMXBean.getThreadAllocatedBytes approach is JVM-dependent. The small * 7 bound is generous enough for most JVMs, but these tests may need adjustment if Spark moves to a JDK with significantly different allocation characteristics. Not a blocker - just flagging for future CI flakiness awareness.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants