[SQL] Guaranteed CTE shuffle reuse - #58924
Open
AveryQi115 wants to merge 17 commits into
Open
AveryQi115 wants to merge 17 commits into
AveryQi115 wants to merge 17 commits into
Conversation
Port of Databricks Runtime "guaranteed CTE shuffle reuse" to OSS Spark. CTE references whose bodies must be materialized (e.g. non-deterministic) are rewritten to a CTEReuseRelation at the end of optimization and materialized once behind a LOCAL_SHUFFLE_FOR_CTE shuffle shared across all references, so reuse is guaranteed independent of canonical equality. Combines databricks-eng/runtime #223495, #223496, #239461, #240564, #243597, #243961, #248264. Photon, QPL, marshalling and EnsureRequirementsDP are dropped; DatabricksSQLConf flags move to SQLConf (spark.sql.optimizer.replaceCTERefWithCTEReuse.enabled and friends, default off). forceSkipInline (4eaa935) is already upstream as SPARK-58006. NOT COMPILED LOCALLY. Known remaining work is listed in the PR description (plan-reuse repartition base infra, isCreatedForSubplanReuse wiring, and the AQE-on stage conversion). Co-authored-by: Isaac
The parallel port left the test suites referencing config keys without the .enabled suffix (and one wrong name), which do not match the keys defined in SQLConf/StaticSQLConf. Point every test key at its real definition. Co-authored-by: Isaac
…orSubplanReuse Add isCreatedForSubplanReuse to ShuffleExchangeLike (true for the LOCAL_SHUFFLE_FOR_CTE origin, which is only produced for CTE reuse) and guard the EnsureRequirements co-partitioning path so it does not rewrite a reuse shuffle's partitioning in place. A consumer needing a different partitioning now wraps the reuse shuffle in a new shuffle instead, keeping all references to the same CTE canonically equal so they dedup into a single ReusedExchangeExec. Adapted for OSS: drops the runtime PlanReusableRepartition.isForPlanReuse check (part of the un-ported plan-reuse-repartition base) since the CTE origin alone is a sufficient signal here. Co-authored-by: Isaac
Add the base infrastructure the guaranteed CTE reuse rules depend on: - RepartitionIdGenerator, RepartitionOrigin, and the PlanReusableRepartition trait (repartitionId / repartitionOrigin / isForPlanReuse / withRepartitionId / addRepartitionId). - Extend Repartition (localShuffle, id, repartitionOrigin) and RepartitionByExpression (id, repartitionOrigin) to mix in PlanReusableRepartition. New fields are defaulted, so positional construction is unaffected; the few destructuring matches are updated for the new arity. Per scope decision, remove the consumer-driven partitioning selection (OptimizePartitioning / CTEReusePartitioningSelection), its patterns.scala extractors, its SparkOptimizer batch, and its tests. A reused CTE therefore always materializes behind the LocalPartition-equivalent shuffle (SinglePartition here) and consumers add their own shuffle when needed. OSS adaptation: LocalPartition is not ported -- runtime executes it identically to SinglePartition (ConstantPartitioner), and the only divergence (ShuffleSpec.satisfies) mattered solely for the removed OptimizePartitioning local-read path. Co-authored-by: Isaac
Complete the AQE-on path so a reused CTE actually materializes once and is shared under AQE: - Add CTEReuseQueryStageExec (QueryStageExec.scala): a query stage wrapping the shared inner AdaptiveSparkPlanExec. Its output remaps the inner AQE's (primary reference's) attribute ids into this reference's id space (mirroring ReusedExchangeExec); execution and stats delegate to the inner AQE's post-iteration executedPlan. - AdaptiveSparkPlanExec.materialize(): drive the inner AQE to completion of all non-result stages via withFinalPlanUpdate(skipResultStage = true), run once through a lazy Future on a dedicated CTE thread pool (cteExecutionContext, sized by StaticSQLConf.CTE_MATERIALIZATION_MAX_THREAD_THRESHOLD) to avoid nested-CTE pool exhaustion. - Thread skipResultStage through withFinalPlanUpdate / createQueryStages so materialize() creates no ResultQueryStageExec. - createNonResultQueryStages: convert a CTEReuseExchange into a CTEReuseQueryStageExec, resolving the shared inner AQE from AdaptiveExecutionContext.cteAQERegistry (populated by the PlanCTEReuse preprocessing rule). Photon-specific handling from the runtime original is dropped; OSS has no Photon. Co-authored-by: Isaac <no-reply@databricks.com>
…euse Two OSS-cleanup removals: - assignNewExprIds: remove the ASSIGN_NEW_EXPR_IDS_FOR_CTE_REUSE and ASSIGN_EXPR_IDS_REMAP_RUNTIME_FILTERS confs and the flag-gated branch in ReplaceCTERefWithRepartition.deduplicatePlan. CTE-reference deduplication now always uses DeduplicateRelations (AssignNewExprIds is a Databricks-only rule that does not exist in OSS). Drop the corresponding test helper/config. - RepartitionOrigin: remove the origin enum and the repartitionOrigin field from PlanReusableRepartition / Repartition / RepartitionByExpression. Nothing reads it in OSS, so the origin classification is unnecessary. Update destructuring matches for the reduced arity. Co-authored-by: Isaac <no-reply@databricks.com>
A normal CTE def is materialized behind a plain RepartitionByExpression (or left as-is when it already carries a repartition / merged scalar subquery). Only when the def has forceSkipInline set do we consult forcePartitioning: a pinned HashPartitioning becomes a plan-reuse RepartitionByExpression (addRepartitionId) for guaranteed CTE reuse; any other pinned partitioning is rejected; None falls back to the normal path. Drops the REPLACE_CTE_REF_WITH_CTE_REUSE / USE_LOCAL_SHUFFLE_FOR_CTE_REUSE local-shuffle branches here (and the now-unused SQLConf import). Co-authored-by: Isaac <no-reply@databricks.com>
The conf is no longer read by any main code after ReplaceCTERefWithRepartition was simplified. Delete the SQLConf entry and drop it from the CTE reuse test setups. Co-authored-by: Isaac <no-reply@databricks.com>
…s the sole plan-reuse carrier After the CTE-reuse simplifications, no main code constructs a localShuffle Repartition, so the Repartition-side plan-reuse machinery was vestigial. Revert Repartition to a plain RDD-style repartition (numPartitions, shuffle, child) and let RepartitionByExpression alone extend PlanReusableRepartition. Update the CTE-reuse tests to build plan-reuse repartitions as RepartitionByExpression, and the destructuring matches for the reduced Repartition arity. Co-authored-by: Isaac <no-reply@databricks.com>
The CTEReuseRelation strategy planned sharedSubplan into a plain ShuffleExchangeExec (REPARTITION_BY_COL/NUM origin) and never stamped the LOCAL_SHUFFLE_FOR_CTE origin the rest of the feature depends on. As a result isCreatedForSubplanReuse was always false, so EnsureRequirements could rewrite the shared shuffle (diverging the copies), the AQE-off UnwrapCTEReuseExchange path logged an error instead of tracking reuse, and VerifyCTEReuse counted nothing. Re-stamp the origin as LOCAL_SHUFFLE_FOR_CTE(cteId) when building the shuffle so the reuse boundary is recognized on both the AQE-on and AQE-off paths. Co-authored-by: Isaac <no-reply@databricks.com>
Resolve conflicts from upstream changes that landed while this PR was open: - cteOperators.scala (CTERelationDef): keep both the new upstream `materialized: Option[Boolean]` field (SPARK-59372, MATERIALIZED / NOT MATERIALIZED CTE hint) and our `forcePartitioning` field, and merge stringArgs so the materialization keyword and forcePartitioning both render. - QueryExecution.scala: union the exchange-package imports; keep our main-query VerifyCTEReuse in the reuse batch alongside upstream's EnablePipelinedShuffle (SPARK-57399), which still runs last. - AdaptiveSparkPlanExec.scala: keep both new createNonResultQueryStages arms -- our CTEReuseExchange case and upstream's pipelined-ShuffleExchangeExec case. - EnsureRequirements.scala: upstream extracted the co-partition logic into coPartitionChildren; re-apply our CTE-reuse guard (skip in-place partitioning rewrite for a LOCAL_SHUFFLE_FOR_CTE shuffle) at the new rewrite site. Also update CTERelationDef extractor patterns to the new 8-field arity in PushdownPredicatesAndPruneColumnsForCTEDef and PushdownPredicatesForCTEDefStalenessSuite (the appended forcePartitioning field). Co-authored-by: Isaac <no-reply@databricks.com>
The first CI run on the merge (run 35412515110) failed to precompile catalyst: - RepartitionByExpression gained an `id` field on this branch, so its extractor arity is now 5. Update the 4-arity patterns upstream/tests destructure it with: Analyzer (ResolveMissingReferences), Optimizer (CollapseRepartition, OptimizeRepartition), and PythonUDTFSuite. - ReplaceCTERefWithRepartition still referenced the DBR-only `JoinIdHelper` / `assignJoinId`, which do not exist in OSS. Drop them; DeduplicateRelations already re-assigns fresh exprIds to the inlined CTE copy. - LogicalPlan.validateCTEReuseRelations referenced `TreePattern.CTE_REUSE` but only the TreePattern type (not the object) was imported. Import CTE_REUSE. - CTEReuseWithAQESuite / CTEReuseWithoutAQESuite used the DBR-only `catalyst.MetricKey` metric API. Replace those assertions with equivalent plan-based checks (shared inner AQE / ReusedExchangeExec presence), which are the OSS-appropriate way to verify reuse. Co-authored-by: Isaac <no-reply@databricks.com>
Second CI pass (run 35416533594) got past catalyst and failed in sql/core: - CTEReuseExchange.computeStats(): OSS physical SparkPlan has no computeStats contract (only logical LeafNode / AQE QueryStageExec do), and ShuffleExchangeLike has no `.stats`. The override was a DBR-ism that overrides nothing, is never called (the node is always consumed before execution), and referenced a non-existent member. Remove it, and drop the now-unused Statistics import. - exchange/PlanCTEReuse imported org.apache.spark.internal.MDC, but MDC(...) is provided by the Logging trait the rules already mix in, so the import is unused (fatal under -Wconf). Import only Logging. Co-authored-by: Isaac <no-reply@databricks.com>
Third CI pass got past main compilation and failed in test compilation: - CTEReuseWithAQESuite / CTEReuseWithoutAQESuite called `cteReuseConf.key`, but `cteReuseConf` is already the conf-key String, so `.key` does not resolve. Use the String directly. - Drop unused imports flagged fatal under -Wconf: `SparkPlan` in CTEReuseWithoutAQESuite (only referenced in a comment), and `SparkException` and `testImplicits._` in ReplaceCTERefAndRepartitionWithCTEReuseSuite (the suite builds plans directly, no implicits or SparkException usage). Co-authored-by: Isaac <no-reply@databricks.com>
sql/Test compilation failed: ReplaceCTERefAndRepartitionWithCTEReuseSuite passed the CTERelationRef constructor a named argument `_isStreaming`, but the OSS parameter is `isStreaming` (only `_resolved` is underscored). Rename the argument at all seven call sites. Co-authored-by: Isaac <no-reply@databricks.com>
…inding, line length
First test-execution pass (run 35420935083) compiled clean and surfaced runtime
and lint failures:
- ReplaceCTERefWithRepartition never assigned a repartitionId on the normal
materialization path, so every CTE reference became RepartitionByExpression
with id=0. ReplaceRepartitionWithCTEReuse only seals non-zero ids, so no
CTEReuseRelation/CTEReuseExchange/inner-AQE was ever produced -- all 14
CTE-reuse tests saw 0 reuse nodes. Assign a fresh id (addRepartitionId) on both
normal paths; it is built once per CTE def, so all references share it.
- RepartitionByExpression's new `id` field leaked into plan output
("RepartitionByExpression ..., 0"), breaking golden/proto plan comparisons
(pipe-operators, transform, udtf, connect repartition* tests). Never render the
id (internal correlation field) via stringArgs, keeping output deterministic.
- The three new confs lacked a binding policy, failing SparkConfigBindingPolicySuite.
Add SESSION for the SQLConf entries and NOT_APPLICABLE for the static conf.
- Wrap 5 comment/log lines that exceeded 100 chars (scalastyle).
Co-authored-by: Isaac <no-reply@databricks.com>
Removing the DBR-only MetricKey import left three blank lines between the package statement and the first import; scalastyle allows at most one. Collapse to a single blank line in CTEReuseWithAQESuite and CTEReuseWithoutAQESuite. Co-authored-by: Isaac <no-reply@databricks.com>
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
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.
TL;DR
Guarantees that a CTE marked non-inlinable (
CTERelationDef.forceSkipInline = true) is materialized exactly once and physically reused across all of its references. Each such CTE reference is turned into aCTEReuseExchangenode wrapped over a single shuffle of the CTE body. Whether or not Adaptive Query Execution (AQE) is enabled, execution respects theCTEReuseExchange: every reference reads the same shuffle files rather than recomputing the CTE. This closes the gap where stockReuseExchangeAndSubquerycannot guarantee reuse for a non-deterministic CTE body — non-determinism breaks canonical equality, so the stock rule may fail to dedup the copies and silently recompute.Why it is needed
A CTE referenced multiple times can be either inlined (recomputed per reference) or materialized once. For a non-deterministic body (e.g.
rand(),monotonically_increasing_id(), non-deterministic UDFs), recomputation is not just wasteful but semantically wrong — each reference would observe different rows. Stock reuse keys on canonical equality of the physical subtrees, which non-determinism and per-referenceexprIdremapping defeat. This change introduces acteId-keyed mechanism so all references provably share one materialized exchange.Life cycle
The feature threads one identity — the
cteId— from the logical plan down to a single physical shuffle:Logical → physical (common to both modes)
ReplaceCTERefWithRepartitionreplaces each reference to aforceSkipInlineCTE with a plan-reuseRepartitionByExpression(a non-zerorepartitionId, assigned byRepartitionIdGenerator) wrapping the inlined CTE body. If the def carriesforcePartitioning = Some(HashPartitioning), the reference is materialized with that pinned partitioning; otherwise it falls back to the normal path (RepartitionByExpression(Seq.empty, …), unless the body already satisfies the partitioning or the reference is under a subquery). The inlined body is run throughDeduplicateRelationsso each copy gets freshexprIds — therepartitionId, notexprIdequality, is what ties the copies together.ReplaceRepartitionWithCTEReuse(main-query pass only) counts plan-reuse repartitions byrepartitionIdacross the whole plan including subqueries. Every id with more than one consumer is sealed into aCTEReuseRelation(cteId, partitioning, sharedSubplan); single-consumer ids stay as ordinary repartitions and simply inline.sharedSubplanis deliberately not a child ofCTEReuseRelation, so outer-plan re-optimization treats the reference as a leaf and never descends into (or diverges) the shared body.SparkStrategiesplansCTEReuseRelationeagerly (not viaplanLater, since the shuffle lives in metadata that the placeholder collector cannot see) into aShuffleExchangeExecover the body, re-stamps its origin asLOCAL_SHUFFLE_FOR_CTE(cteId), and wraps it as aCTEReuseExchange(cteId, shuffle)leaf. The shuffle is stored ininnerChildren, notchildren, so AQE's bottom-up stage creation does not redundantly plan its substages.AQE on:
CTEReuseExchange → CTEReuseQueryStageExec → inner AdaptiveSparkPlanExec → ShufflePlanCTEReuse(a preprocessing rule alongsidePlanAdaptiveSubqueries) builds one shared innerAdaptiveSparkPlanExecpercteId, registered inAdaptiveExecutionContext.cteAQERegistry. The registry lives on the shared execution context, so the identical inner AQE instance is reused across every reference — main query and subqueries — and across re-plans, preventing duplicate replanning or plan divergence of the shared body.createNonResultQueryStagesreplaces eachCTEReuseExchangewith aCTEReuseQueryStageExec(id, innerAQE, output). All references' stages point at the same inner AQE; each stage remaps the inner AQE's (primary reference's) attribute ids into its own reference's id space, mirroringReusedExchangeExec. Execution and statistics delegate to the inner AQE's post-iterationexecutedPlan, so the shared body is iterated once.AQE off:
CTEReuseExchange → LOCAL_SHUFFLE_FOR_CTE shuffle → ReusedExchangeExecUnwrapCTEReuseExchange(early in the preparation pipeline, in both the main and subquery batches) replaces eachCTEReuseExchangewith its underlyingLOCAL_SHUFFLE_FOR_CTE(cteId)shuffle. From there all normal preparation rules run per copy.cteIdis planned from the same canonicalizedsharedSubplanand the shuffle is protected fromEnsureRequirements, the copies stay canonically equal, and the stockReuseExchangeAndSubquerydeduplicates them intoReusedExchangeExec.VerifyCTEReuseruns at the very end (final main-query pass only) and asserts at most one live shuffle percteId. Underspark.sql.optimizer.failOnCteReuseWithoutAqe(test default) a violation throws; in production it logs. Reuse is a best-effort optimization here, not a correctness invariant.Design considerations and details
When AQE is on
1. Deadlock avoidance (dedicated thread pool). The inner AQE is materialized via
materialize()/ lazymaterializeFuture, which callswithFinalPlanUpdate(skipResultStage = true)on a dedicatedcteExecutionContextthread pool (sized byspark.sql.cteMaterialization.maxThreadThreshold, default 1024). The outer AQE's stage-materialization threads block waiting on the CTE stage to finish; if the inner AQE materialized on the same sharedQueryStageCreatorpool, nested materialization could exhaust that pool while outer threads hold slots waiting on it — a classic thread-pool deadlock. A separate pool breaks the dependency cycle.skipResultStage = truematerializes the shared body's stages without wrapping aResultQueryStageExec, since the CTE stage is an input to other stages, not a query result.2. Runtime filters. The CTE body is materialized once and shared across all consumers, so consumer-specific runtime filters (dynamic partition pruning, bloom filters) must not be pushed below the reuse boundary — doing so would specialize the single shared result to one consumer and corrupt the others. Runtime filters are applied per consumer above the
CTEReuseExchange; the shared subplan itself is never specialized. This is a deliberate limitation of guaranteed reuse: correctness of the shared materialization takes precedence over consumer-local filter pushdown.3. Shuffle planning (
EnsureRequirements). The reuse shuffle's partitioning is fixed at the logical level (CTEReuseRelation.partitioning, derived from the common distribution of all consumers, or a safe fallback).EnsureRequirementsmust not rewrite it, because re-deriving partitioning per consumer would make the copies diverge and defeat reuse. The guard iscase s: ShuffleExchangeExec if !s.isCreatedForSubplanReuse => s.copy(outputPartitioning = …), whereisCreatedForSubplanReuseis true for theLOCAL_SHUFFLE_FOR_CTEorigin. The protected shuffle is an immutable boundary: consumer-dependent operators are added above it, keeping the subtree below canonically identical across copies. The same origin also keeps the shuffle out of the AQEsupportedShuffleOriginsallowlists (coalesce / local read / skew join), so its partition layout stays frozen.When AQE is off
1. Interaction with
planSubqueries. With AQE off, subqueries are prepared separately —PlanSubqueriescallsQueryExecution.prepareExecutedPlan → preparations(subquery = true), applying the preparation rules recursively to each subquery plan on its own. SoUnwrapCTEReuseExchangeis registered in both the main and subquery preparation batches, ensuring CTE references inside subqueries are unwrapped too. Conversely,VerifyCTEReuseruns only on the final main-query pass: during the per-subqueryPlanSubqueriesinvocations, reuse across the whole plan is not yet complete, so verifying there would produce false negatives. Reuse itself is achieved entirely through the stockReuseExchangeAndSubquery(canonical matching) — this change adds no new dedup engine for the AQE-off path, only the protectedLOCAL_SHUFFLE_FOR_CTEboundary that keeps the copies canonically equal for the stock rule to find.Testing
Adds
CTEReuseWithAQESuite,CTEReuseWithoutAQESuite, andReplaceCTERefAndRepartitionWithCTEReuseSuitecovering the optimizer rewrites, the AQE-on inner-AQE path, and the AQE-off canonical-reuse path (including reuse across subqueries). All new confs default to off, so existing behavior is unchanged.Co-authored-by: Maryann Xue maryann.xue@gmail.com
Co-authored-by: Juliusz Sompolski julek@databricks.com
This pull request and its description were written by Isaac.