[GLUTEN-12753][CORE] Mark natively-handled PushedFilters with * for all file-source scans - #12626
Draft
felipepessoto wants to merge 3 commits into
Draft
[GLUTEN-12753][CORE] Mark natively-handled PushedFilters with * for all file-source scans#12626felipepessoto wants to merge 3 commits into
* for all file-source scans#12626felipepessoto wants to merge 3 commits into
Conversation
|
Run Gluten Clickhouse CI on x86 |
felipepessoto
changed the title
* for all file-source scans* for all file-source scans
felipepessoto
force-pushed
the
vl-mark-pushedfilters-native-handled
branch
from
August 11, 2026 20:56
92aee41 to
1bb3bd4
Compare
|
Run Gluten Clickhouse CI on x86 |
felipepessoto
force-pushed
the
vl-mark-pushedfilters-native-handled
branch
from
August 11, 2026 21:35
1bb3bd4 to
7f697b6
Compare
|
Run Gluten Clickhouse CI on x86 |
felipepessoto
force-pushed
the
vl-mark-pushedfilters-native-handled
branch
from
August 11, 2026 22:08
7f697b6 to
9375f7d
Compare
|
Run Gluten Clickhouse CI on x86 |
felipepessoto
marked this pull request as draft
August 12, 2026 16:47
… all file-source scans Gluten's file-source scan transformers push every filter conjunct into the native scan via PushDownFilterToScan and evaluate them as exact row-level filters, so the paired FilterExecTransformer becomes a no-op (FilterExecTransformerBase.isNoop) and no separate post-scan Filter is needed. This is exactly the state Spark marks with a leading `*` in a scan's `PushedFilters` metadata (RowDataSourceScanExec's convention for a filter the source fully handles itself). Gluten inherited FileSourceScanLike's rendering, which leaves `PushedFilters` unmarked, so its plan output under-reported what the native scan actually does. Mark each pushed-filter entry with `*` in the rendered plan string of FileSourceScanExecTransformerBase. `metadata` is a lazy val and cannot be super-overridden, so the marking is applied to the rendered node string in both paths that print it: - simpleString (executedPlan.toString), and - verboseStringWithOperatorId (FormattedMode / plan-stability golden files). The mark is gated on the backend actually accepting Gluten's full filter pushdown (SparkPlanExecApi.supportPushDownFilterToScan). ClickHouse deliberately declines it for Parquet to keep vanilla-Spark best-effort semantics, and there BasicScanExecTransformer.filterExprs() silently drops filters the backend cannot evaluate, leaving a real (non-no-op) FilterExecTransformer above the scan -- marking those would be a false claim. Velox does not override the default (true), so its rendering is unaffected. Only FileSourceScanExecTransformer nodes are marked. AQE FormattedMode also prints an Initial Plan whose scans are vanilla FileSourceScanExec (`Scan parquet`); those are left unmarked, matching runtime output and the semantics of `*` (the vanilla Initial-Plan scan still has a real Filter above it). The change lives in the common gluten-substrait module (hence [CORE]). Golden plans updated accordingly (TPC-H approved-plan, TPC-DS plan-stability, and gluten-tpch-plan-stability across the supported Spark versions): every offloaded FileSourceScanExecTransformer PushedFilters entry now carries `*`, while vanilla Initial-Plan scan entries stay unmarked. The goldens contain no DataSourceV2 BatchScan PushedFilters, so no V2 golden is affected. Add FileSourceScanExecTransformerPushedFiltersSuite covering the rendering helpers directly: entry marking, column names containing spaces, commas nested inside an entry (e.g. `In(id, [1,2,3])`), empty/non-list values, that only the PushedFilters list is rewritten, and that an unbalanced list degrades to unchanged text rather than being corrupted. It needs no native library, so it runs in the regular JVM test job. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 02286396-395f-41db-ad89-ad69a192cded
felipepessoto
force-pushed
the
vl-mark-pushedfilters-native-handled
branch
from
August 14, 2026 20:46
9375f7d to
0299776
Compare
|
Run Gluten Clickhouse CI on x86 |
Removed known failures related to Delta CDC tests from the list.
|
Run Gluten Clickhouse CI on x86 |
…al-char columns
Delta's CDF filter-pushdown tests over a column-mapping table
(DeltaCDC{Id,Name}ColumnMappingSuite / SQL variants, "filters with special
characters in name should be pushed down") assert:
PushedFilters: [*IsNotNull(id with space), *LessThan(id with space,5)]
Vanilla Delta serves a CDF read through a single PrunedFilteredScan
(DeltaCDFRelation), whose V1 PushedFilters are translated with nested-predicate
pushdown DISABLED, so a column name is rendered verbatim (unquoted). Gluten
offloads the CDF read to a parquet-backed scan, where Spark translates the same
filters with nested pushdown ENABLED and back-quotes any name that needs quoting:
PushedFilters: [*IsNotNull(`id with space`), *LessThan(`id with space`,5)]
The `*` (added for all Gluten file scans in the prior commits) was already
correct; only the display-only back-quoting differed. Drop it for CDF scans to
match vanilla Delta. Scope is limited to CDF scans (relation.location is
CdcAddFileIndex / TahoeRemoveFileIndex / TahoeChangeFileIndex): a regular Delta
read is a parquet scan in vanilla Spark too and legitimately keeps the quoting.
`metadata` is a lazy val and cannot be super-overridden, so both rewrites (`*`
marking in the base, back-quote removal here) are applied to the rendered plan
string. Factor the list-locating logic in FileSourceScanExecTransformerBase into a
reusable `rewritePushedFiltersList` so DeltaScanTransformer layers the CDF
un-quoting over the base `*`-marking in simpleString / verboseStringWithOperatorId.
|
Run Gluten Clickhouse CI on x86 |
felipepessoto
marked this pull request as ready for review
August 15, 2026 08:47
|
Run Gluten Clickhouse CI on x86 |
|
Run Gluten Clickhouse CI on x86 |
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.
Fixes #12753
What changes are proposed in this pull request?
Gluten's file-source scan transformers push every filter conjunct into the native scan via
PushDownFilterToScanand evaluate them as exact row-level filters, so the pairedFilterExecTransformerbecomes a no-op (FilterExecTransformerBase.isNoop) and no separate post-scanFilteris needed. This is exactly the state Spark marks with a leading*in a scan'sPushedFiltersmetadata —RowDataSourceScanExec's convention for a filter the source fully handles itself.Gluten inherited
FileSourceScanLike's rendering, which leavesPushedFiltersunmarked, so its plan output under-reported what the native scan actually does. This PR marks each pushed-filter entry with*in the rendered plan string ofFileSourceScanExecTransformerBase. Becausemetadatais alazy valand cannot besuper-overridden, the marking is applied to the rendered node string in both paths that print it:simpleString(executedPlan.toString), andverboseStringWithOperatorId(FormattedMode/ plan-stability golden files).Only
FileSourceScanExecTransformernodes are marked. AQEFormattedModealso prints an Initial Plan whose scans are vanillaFileSourceScanExec(Scan parquet); those are left unmarked, which matches runtime output and the semantics of*(the vanilla Initial-Plan scan still has a realFilterabove it).The change lives in the common
gluten-substraitmodule (hence[CORE]). Golden plans are updated accordingly (TPC-H approved-plan, TPC-DS plan-stability, and gluten-tpch-plan-stability across the supported Spark versions): every offloadedFileSourceScanExecTransformerPushedFiltersentry now carries*, while vanilla Initial-Plan scan entries stay unmarked. The goldens contain no DataSource V2BatchScanPushedFilters, so no V2 golden is affected.How was this patch tested?
gluten-substraitcompiles againstmain(spark-3.5/scala-2.12).Scan parquetnodes marked and 0FileSourceScanExecTransformernodes left unmarked across all 1672 golden files. The golden diff is purely the*toggling (no unrelated plan drift).PushedFiltersrendering is exercised by the existingVeloxTPCHSuitegolden-file checks and the TPC-DS / TPC-H plan-stability suites.Was this patch authored or co-authored using generative AI tooling?
Generated-by: GitHub Copilot CLI (Claude Opus 4.8)