[SPARK-58082][SQL] Add DSv2 relation and row-level command tree patterns#57178
[SPARK-58082][SQL] Add DSv2 relation and row-level command tree patterns#57178yyanyy wants to merge 2 commits into
Conversation
Add Catalyst tree-pattern identity bits for the post-pushdown DSv2 scan leaf, the pre-pushdown DSv2 relation, and the row-level command nodes, and use them to prune the optimizer/analyzer rules that match them. - Add DATA_SOURCE_V2_RELATION and DATA_SOURCE_V2_SCAN_RELATION and attach them to DataSourceV2Relation / DataSourceV2ScanRelation. - Add DELETE_FROM_TABLE, UPDATE_TABLE, MERGE_INTO_TABLE, REPLACE_DATA and WRITE_DELTA. Command now exposes a nodePatternsInternal() hook (mirroring PlanExpression) so command nodes combine the shared COMMAND bit with their own identity bit; DeleteFromTable/UpdateTable/MergeIntoTable/ReplaceData/ WriteDelta override it. - Guard the previously-unguarded rules with containsPattern/containsAnyPattern: RewriteDeleteFromTable, RewriteUpdateTable, RewriteMergeIntoTable, V2ScanRelationPushDown, V2ScanPartitioningAndOrdering, GroupBasedRowLevelOperationScanPlanning, OptimizeMetadataOnlyDeleteFromTable, RowLevelOperationRuntimeGroupFiltering. - Add tree-pattern contract tests (DataSourceV2RelationSuite, V2CommandTreePatternSuite).
gengliangwang
left a comment
There was a problem hiding this comment.
0 blocking, 1 non-blocking, 0 nits.
Clean, well-scoped performance change with a sound design that mirrors the PlanExpression precedent; the only note is a test/description completeness gap on the two untested command nodes.
Suggestions (1)
- V2CommandTreePatternSuite.scala:29: only 3 of 5 command nodes tested —
ReplaceData/WriteDelta(the bits most guards rely on) are untested, and the PR description overstates coverage — see inline
Verification
Traced all 8 pruning guards for equivalence to the unguarded rules they replace. treePatternBits propagates every node's bits up to the root (TreeNode.getDefaultTreePatternBits), and each rule's matched nodes all declare the guarded bit — commands via nodePatternsInternal(), relations via override val nodePatterns — so a containsPattern guard descends into exactly the subtrees that can match. V2ScanRelationPushDown's top-level early-return is sound because ScanBuilderHolder (which every sub-rule matches) is constructed only from a DataSourceV2Relation, so no relation in the input means no holder and no sub-rule firing. No input produces a different plan.
| * Pins the tree-pattern identity contract for the DSv2 row-level command nodes: each node carries | ||
| * both the shared `COMMAND` bit and its own identity bit, so rules can prune on either. | ||
| */ | ||
| class V2CommandTreePatternSuite extends SparkFunSuite { |
There was a problem hiding this comment.
This suite pins the COMMAND + identity-bit contract for 3 of the 5 command nodes the PR touches — ReplaceData and WriteDelta are missing. Their REPLACE_DATA/WRITE_DELTA bits are the most load-bearing in the PR: GroupBasedRowLevelOperationScanPlanning, OptimizeMetadataOnlyDeleteFromTable, and RowLevelOperationRuntimeGroupFiltering all prune exclusively on them, so a wrong bit there would silently disable those rules with no unit-level signal. Worth adding two cases here; the nodePatternsInternal() overrides return compile-time constants, so a minimal instance pins the bit without needing resolution.
(Relatedly, the PR description's "asserts each command node carries both COMMAND and its own bit" reads as all five — worth adjusting to match the actual coverage.)
… add ReplaceData/WriteDelta tests - Remove the DATA_SOURCE_V2_RELATION pruning guard from V2ScanRelationPushDown. The guard was too aggressive: pushDownSample also validates SYSTEM sampling that cannot be pushed down (e.g. over a DSv1 relation, which has no DataSourceV2Relation) and throws UNSUPPORTED_FEATURE.TABLESAMPLE_SYSTEM_NO_SCAN. With the guard, that validation was skipped and DSv1 SYSTEM sampling surfaced a SparkException at execution instead of the expected AnalysisException, breaking DataSourceV2TableSampleSuite "TABLESAMPLE SYSTEM on DSv1 table errors". Restore the rule to its original unconditional fold. The DATA_SOURCE_V2_RELATION bit stays available for other rules (e.g. V2ScanPartitioningAndOrdering). - Add ReplaceData and WriteDelta cases to V2CommandTreePatternSuite, pinning the COMMAND + REPLACE_DATA / WRITE_DELTA identity bits that GroupBasedRowLevelOperationScanPlanning, OptimizeMetadataOnlyDeleteFromTable and RowLevelOperationRuntimeGroupFiltering prune on.
What changes were proposed in this pull request?
Add Catalyst tree-pattern identity bits for the DSv2 relation leaves and the
row-level command nodes, and use them to prune the optimizer/analyzer rules
that match those nodes.
DATA_SOURCE_V2_RELATIONandDATA_SOURCE_V2_SCAN_RELATIONtoTreePattern, and attach them toDataSourceV2RelationandDataSourceV2ScanRelationrespectively.DATA_SOURCE_V2_SCAN_RELATIONis usedas a pruning guard (see below);
DATA_SOURCE_V2_RELATIONis attached to therelation leaf but has no consumer rule in this PR (reserved for future pruning).
DELETE_FROM_TABLE,UPDATE_TABLE,MERGE_INTO_TABLE,REPLACE_DATAand
WRITE_DELTA. Since every command already carries the sharedCOMMANDbit (declared
finalon theCommandtrait),Commandnow exposes anodePatternsInternal()hook, mirroring howPlanExpressioncombinesPLAN_EXPRESSIONwith a per-subquery bit, so a command node contributes itsown identity bit while
COMMANDstays guaranteed-present.DeleteFromTable,UpdateTable,MergeIntoTable,ReplaceDataandWriteDeltaoverride it.containsPattern/containsAnyPattern:RewriteDeleteFromTable,RewriteUpdateTable,RewriteMergeIntoTable,V2ScanPartitioningAndOrdering,GroupBasedRowLevelOperationScanPlanning,OptimizeMetadataOnlyDeleteFromTableandRowLevelOperationRuntimeGroupFiltering.Why are the changes needed?
A tree-pattern bit is an O(1) bitset lookup on a plan's cached
treePatternBits.Rules that today run an unconditional
transform/resolveOperatorswalk overevery node, on every invocation, even for plans that contain none of the target
nodes, can instead cheaply skip whole subtrees that provably don't contain the
node they rewrite. The DSv2 scan/relation leaves and the row-level command nodes
previously had no identity pattern (the command nodes carried only the coarse
COMMANDbit shared by all commands), so rules matching them either ranunguarded or could only prune on the broad
COMMANDbit. This adds the missingper-node bits and applies them as pruning guards, matching the existing idiom
used throughout the optimizer (for example
ReplaceExceptWithFilteronEXCEPT,ResolveRowLevelCommandAssignmentsonCOMMAND).Does this PR introduce any user-facing change?
No. This only adds internal Catalyst tree-pattern metadata and rule pruning
guards. Query results and plans are unchanged; the guarded rules fire on exactly
the same nodes as before, just skipping trees that cannot contain them.
How was this patch tested?
DataSourceV2RelationSuite(asserts the relation/scan-relation identity bits)and a new
V2CommandTreePatternSuite(asserts each of the five row-levelcommand nodes --
DeleteFromTable,UpdateTable,MergeIntoTable,ReplaceDataandWriteDelta-- carries bothCOMMANDand its own bit).not drop any rule firing:
passing.
DataSourceV2Suite,KeyGroupedPartitioningSuite,ProjectedOrderingAndPartitioningSuite,DataSourceV2TableSampleSuite): all passing, includingTABLESAMPLE SYSTEMvalidation over DSv1 relations.
V2AppendData*,V2Overwrite*ANSI/Strict suites):388 tests, all passing, confirming the
Seq(COMMAND) ++ nodePatternsInternal()composition does not disturb existing
COMMAND-pruned analysis rules../build/sbt catalyst/compile sql/compileandgit diff --check.Was this patch authored or co-authored using generative AI tooling?
yes, opus 4.8