[SPARK-59012][SQL][4.3] Improve SupportsRuntimeCatalystFiltering test coverage and reject nested filter attributes - #58356
Closed
szehon-ho wants to merge 1 commit into
Conversation
…rage and reject nested filter attributes ### What changes were proposed in this pull request? `SupportsRuntimeCatalystFiltering` was added by SPARK-58523, which covered row-level operations (`RowLevelOperationCatalystRuntimeFilterSuiteBase` and its group- and delta-based subclasses) and a core set of pushdown tests. It was not exercised in the two other places the predicate-based runtime filtering interfaces are: dynamic partition pruning and storage-partitioned joins. This PR closes that gap by reusing the existing suites rather than cloning them, and hardens the `filterAttributes()` contract. **Behavior change** - `DataSourceV2ScanRelation.runtimeFilterAttrs` (and `fullyPushedRuntimeFilterAttrs`) now reject a nested (multi-part) runtime-filter reference, including one over a struct column. Both runtime filtering interfaces require each `filterAttributes()` reference to be a top-level read-schema attribute; a nested reference such as `s.tz` previously resolved and widened to the enclosing struct column `s`, making runtime filters over every field of `s` eligible. **Test fixtures** - `InMemoryBaseTable`: `CatalystRuntimeFilteringScan` now prunes on nested partition keys. It previously looked up a partition attribute by joining `fieldNames` into a single top-level name, so a `GetStructField` chain such as `s.part` never matched and no pruning happened. Partition attributes now keep their `fieldNames` as name-part sequences, and `partitionAttrFor` matches the expression's path component-wise, so a quoted top-level column `` `a.b` `` stays distinct from a nested `a`.`b`. - `InMemoryCatalystRuntimeFilterTable`: threads the full table-creation metadata (constraints, distribution, ordering, partition counts, advisory size, strictness, and `numRowsPerSplit`) through to `InMemoryBaseTable`, and derives `filterAttributes()` / `fullyPushedFilterAttributes()` from a shared helper. - `InMemoryTableCatalystRuntimeFilterCatalog`: adds `InMemoryCatalystRuntimeFilterCatalog`, the `InMemoryCatalog` counterpart, so the Catalyst fixture can be used where functions and procedures are needed. - `InMemoryTableWithV2Filter`: threads the same table-creation metadata through to `InMemoryBaseTable`. **Reused suites** - `DynamicPartitionPruningSuite`: adds `DynamicPartitionPruningV2CatalystFilterSuiteAEOff` / `AEOn`, mirroring the existing `DynamicPartitionPruningV2FilterSuite` pair. - `KeyGroupedPartitioningSuite`: the shared SPJ fixtures move to a new `KeyGroupedPartitioningSuiteBase`, and the three runtime-filtering tests move to a `KeyGroupedPartitioningRuntimeFilterTests` trait. Two small suites then run those three tests once per interface: `KeyGroupedPartitioningRuntimeFilterSuite` (predicate-based, the default `InMemoryCatalog`) and `KeyGroupedPartitioningCatalystRuntimeFilterSuite`. - `DistributionAndOrderingSuiteBase`: `catalogClassName` becomes overridable so a subclass can vary the catalog behind `testcat`. Note for reviewers: `SPARK-42038: partially clustered: with dynamic partition filtering` and `SPARK-45652: SPJ should handle empty partition after dynamic filtering` are unchanged, but now report under `KeyGroupedPartitioningRuntimeFilterSuite` instead of `KeyGroupedPartitioningSuite`. Most of the diff in that file is this movement. **New tests in `DataSourceV2CatalystRuntimeFilterSuite`** Covering behavior specific to pushing Catalyst expressions: a DPP filter on a nested partition field arriving with the nested access intact, multiple predicates pushed in a single `filter()` call, a filter with no V2 translation being pushed instead of dropped, a scan implementing both runtime filtering interfaces being rejected, two partition columns whose dotted names collide (a quoted top-level `` `x.y` `` and a nested `x`.`y`) binding to the correct partition slot, and the `filterAttributes()` contract when a reported attribute is not a top-level scan attribute -- a missing attribute and a nested reference (over both an int and a struct column) are all rejected. One test calls `PushDownUtils.replanWithRuntimeFilters` directly, to reach the SPJ partitioning-preservation checks that a well-behaved source cannot trigger: dropping `HasPartitionKey`, reporting a partition key that was not in the original partitioning, or growing a key's split count. ### Why are the changes needed? `SupportsRuntimeCatalystFiltering` is the path a scan takes when runtime filters are pushed as Catalyst expressions instead of connector predicates. Dynamic partition pruning and storage-partitioned joins are the two features that produce those filters, and neither was tested against this interface, so regressions in the Catalyst path would not have been caught by the suites that cover the equivalent predicate-based path. ### Does this PR introduce _any_ user-facing change? No end-user-facing behavior change. For connector authors, a scan that declares a nested runtime-filter attribute -- a violation of the `filterAttributes()` contract -- is now rejected with an internal error instead of silently widening to the enclosing top-level column. ### How was this patch tested? Existing and new unit tests. Locally, on the rebased branch: - `DataSourceV2CatalystRuntimeFilterSuite`: 16 tests passed, including the new dotted/nested collision test and the nested-reference rejection. - `DynamicPartitionPruningV2CatalystFilterSuiteAEOff` / `AEOn` and `DynamicPartitionPruningV2FilterSuiteAEOff` / `AEOn`: 154 tests passed. - `KeyGroupedPartitioningSuite`: 96 tests passed. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Cursor with Claude Opus 5 Closes apache#58296 from szehon-ho/catalyst-runtime-filter-coverage. Authored-by: Szehon Ho <szehon.apache@gmail.com> Signed-off-by: Szehon Ho <szehon.apache@gmail.com> (cherry picked from commit d7e22ce) Signed-off-by: Szehon Ho <szehon.apache@gmail.com>
Member
Author
|
@cloud-fan @HeartSaVioR can we get this in for 4.3? the public DSV2 API is added in 4.3 and we changed a little behavior (from review comment, mostly increase validation). But somehow due to conflicts I couldn't merge #58296 directly to 4.3 |
gengliangwang
approved these changes
Aug 27, 2026
Member
Author
|
holding on this for the moment, looking more deep if the extra validation make sense or not |
szehon-ho
marked this pull request as draft
August 27, 2026 23:43
Member
Author
|
actually found a breaking issue, fixing it first: #58370, glad we did not backport it to 4.3 |
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
2 participants
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?
This backports #58296 to
branch-4.3.The patch improves test coverage for
SupportsRuntimeCatalystFilteringin dynamic partitionpruning and storage-partitioned joins, and rejects nested runtime-filter attributes that violate
the connector contract.
The cherry-pick had one conflict in
KeyGroupedPartitioningSuite.scala, resolved to preserve the4.3 branch's existing test organization and tagging while adding the new runtime-filter suites.
Why are the changes needed?
Spark 4.3 contains
SupportsRuntimeCatalystFiltering, but lacks the coverage and contractvalidation added by #58296. Backporting the change prevents regressions in Catalyst-expression
runtime filtering and rejects invalid nested filter attributes consistently.
Does this PR introduce any user-facing change?
No new user-facing change beyond #58296. For connector authors, a scan that declares a nested
runtime-filter attribute is rejected with an internal error instead of silently widening it to the
enclosing top-level column.
How was this patch tested?
Run locally with
SPARK_LOCAL_IP=127.0.0.1:277 tests succeeded, 0 failed, and 2 were ignored.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: Cursor with Claude Opus 5 for the original patch.
Generated-by: OpenAI Codex with GPT-5 for the backport and conflict resolution.