[GLUTEN-12668][CORE] Add injectPre hook so rules survive whole-stage AQE fallback - #12669
Merged
Merged
Conversation
…AQE fallback ExpandFallbackPolicy's whole-stage-fallback revert target (originalPlan in HeuristicApplier.makeRule) is captured before any physical rule runs, including injectPreTransform rules. A rule registered at injectPreTransform therefore gets its rewrite stripped away whenever ExpandFallbackPolicy promotes an individual-stage fallback to a whole-stage one, requiring a second re-application at injectFinal as a workaround (see RuntimeBloomFilterRewriteRule in apache#12151). Adds a new injectPre hook to GlutenInjector.LegacyInjector, with its own "pre" phase in HeuristicApplier.makeRule running before "transform". fallbackPolicies now closes over the post-"pre" plan instead of the raw originalPlan, so a whole-stage revert can no longer strip away anything registered at injectPre. Purely additive: a new empty-by-default builder list threaded through HeuristicApplier's constructor. No existing backend needs to change unless it opts in; backends-clickhouse builds and links unmodified against this change. Once merged, apache#12151 will be rebased to register RuntimeBloomFilterRewriteRule at injectPre only, collapsing its current two registrations (injectPreTransform + injectFinal) down to one.
|
Run Gluten Clickhouse CI on x86 |
Contributor
Author
|
@zhztheplayer @philo-he This is the standalone Summary of what's here and what's verified:
Once this merges, I'll rebase #12151 to switch |
zhztheplayer
approved these changes
Aug 4, 2026
brijrajk
added a commit
to brijrajk/incubator-gluten
that referenced
this pull request
Aug 4, 2026
…QE fallback (incl. SPARK-54336) Rewrites of bloom-filter expressions to their Velox variants are split by where the expressions become visible in the optimizer pipeline: - User-facing might_contain(<scalar subquery>, <non-literal>) pairs are rewritten by BloomFilterMightContainJointRewriteRule, now a logical rule registered via injectOptimizerRule (Operator Optimization batch). This bakes the substitution into the originalPlan snapshot ExpandFallbackPolicy uses when promoting a stage fallback to a whole-stage AQE fallback, so both sides stay on the same serialized byte format even if stages revert to JVM execution. This fixes the GLUTEN-12013 crash (java.io.IOException: Unexpected Bloom filter version number). - Literal-valued pairs (SPARK-54336) are left fully vanilla on both sides, preserving vanilla NULL-on-empty-input semantics and a consistent version=0 byte format. - Runtime-filter pairs are injected by Spark's InjectRuntimeFilter, which runs in a later SparkOptimizer batch than the Operator Optimization batch hosting the logical rule, so the logical rule never sees them. They are rewritten by a new physical rule, RuntimeBloomFilterRewriteRule, restricted to InjectRuntimeFilter's exact shape (both sides wrapped in xxhash64). This keeps FilterExecTransformer and the bloom-filter aggregate native, matching the pre-change behavior; no TPC-DS plan-stability goldens change. The xxhash64 fingerprint also means DataFrame.stat.bloomFilter() (raw-column child) is never matched, so the CallerInfo.isBloomFilterStatFunction stack-walk hack is removed. RuntimeBloomFilterRewriteRule is registered once, at injectPre (GLUTEN-12668, apache#12669). injectPre runs before HeuristicTransform's offload validator (so the rewrite is visible for native offload) and is baked into the plan that ExpandFallbackPolicy reverts to on whole-stage fallback -- unlike injectPreTransform, a revert can no longer strip the rewrite away, so no second re-application at injectFinal is needed. This is only safe because VeloxBloomFilterAggregate's JVM-side buffer capacity agrees with the native bloom_filter_agg aggregate's (GLUTEN-12613, apache#12614): before that fix, a reverted stage merging with a still-native stage could silently corrupt the filter instead of crashing on a version mismatch. Also adds GlutenBloomFilterFallbackSuite covering whole-stage fallback at thresholds 1 and 2, the SPARK-54336 literal path, JVM-mode subquery aggregation, DataFrame.stat.bloomFilter(), native-bloom-filter-disabled, native offload of runtime bloom filters, and the whole-stage reversion scenario described above.
brijrajk
added a commit
to brijrajk/incubator-gluten
that referenced
this pull request
Aug 5, 2026
…QE fallback (incl. SPARK-54336) Rewrites of bloom-filter expressions to their Velox variants are split by where the expressions become visible in the optimizer pipeline: - User-facing might_contain(<scalar subquery>, <non-literal>) pairs are rewritten by BloomFilterMightContainJointRewriteRule, now a logical rule registered via injectOptimizerRule (Operator Optimization batch). This bakes the substitution into the originalPlan snapshot ExpandFallbackPolicy uses when promoting a stage fallback to a whole-stage AQE fallback, so both sides stay on the same serialized byte format even if stages revert to JVM execution. This fixes the GLUTEN-12013 crash (java.io.IOException: Unexpected Bloom filter version number). - Literal-valued pairs (SPARK-54336) are left fully vanilla on both sides, preserving vanilla NULL-on-empty-input semantics and a consistent version=0 byte format. - Runtime-filter pairs are injected by Spark's InjectRuntimeFilter, which runs in a later SparkOptimizer batch than the Operator Optimization batch hosting the logical rule, so the logical rule never sees them. They are rewritten by a new physical rule, RuntimeBloomFilterRewriteRule, restricted to InjectRuntimeFilter's exact shape (both sides wrapped in xxhash64). This keeps FilterExecTransformer and the bloom-filter aggregate native, matching the pre-change behavior; no TPC-DS plan-stability goldens change. The xxhash64 fingerprint also means DataFrame.stat.bloomFilter() (raw-column child) is never matched, so the CallerInfo.isBloomFilterStatFunction stack-walk hack is removed. RuntimeBloomFilterRewriteRule is registered once, at injectPre (GLUTEN-12668, apache#12669). injectPre runs before HeuristicTransform's offload validator (so the rewrite is visible for native offload) and is baked into the plan that ExpandFallbackPolicy reverts to on whole-stage fallback -- unlike injectPreTransform, a revert can no longer strip the rewrite away, so no second re-application at injectFinal is needed. This is only safe because VeloxBloomFilterAggregate's JVM-side buffer capacity agrees with the native bloom_filter_agg aggregate's (GLUTEN-12613, apache#12614): before that fix, a reverted stage merging with a still-native stage could silently corrupt the filter instead of crashing on a version mismatch. Also adds GlutenBloomFilterFallbackSuite covering whole-stage fallback at thresholds 1 and 2, the SPARK-54336 literal path, JVM-mode subquery aggregation, DataFrame.stat.bloomFilter(), native-bloom-filter-disabled, native offload of runtime bloom filters, and the whole-stage reversion scenario described above.
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.
What changes are proposed in this pull request?
Fixes #12668.
Discussed in review of #12151 (GLUTEN-12013 bloom filter fix): #12151 (comment)
Background
Today,
ExpandFallbackPolicy's whole-stage-fallback revert target (originalPlaninHeuristicApplier.makeRule) is captured before any physical rule runs -- includinginjectPreTransformrules. This means a rule registered atinjectPreTransformgets its rewrite stripped away wheneverExpandFallbackPolicypromotes an individual-stage fallback to a whole-stage one, requiring a second re-application registered atinjectFinalas a workaround (seeRuntimeBloomFilterRewriteRulein #12151).Proposal
Add a new
injectPrehook toGlutenInjector.LegacyInjector, with its own"pre"phase inHeuristicApplier.makeRule, running before"transform".fallbackPoliciescloses over the post-"pre"plan instead of the raworiginalPlan, so a whole-stage revert can no longer strip away anything registered atinjectPre-- removing the need for theinjectFinalre-application workaround.This is purely additive: a new empty-by-default builder list threaded through
HeuristicApplier's constructor. No existing backend (Velox, ClickHouse) needs to change unless it opts in. The fiveFallbackStrategiesSuite/GlutenFallbackStrategiesSuitetest files acrossgluten-ut/spark{33,34,35,40,41}constructHeuristicApplierdirectly (bypassing the injector layer) and needed a trivial update (an extraNilargument) for the new constructor parameter.Once merged, #12151 will be rebased to use
injectPreforRuntimeBloomFilterRewriteRule, collapsing its current two registrations (injectPreTransform+injectFinal) down to one.How was this patch tested?
"pre"phase changes no existing plan or behavior.backends-clickhousebuilds, links, and packages successfully unmodified against the changedgluten-core(CHRuleApi.scalanever callsinjectPre).RuntimeBloomFilterRewriteRule([GLUTEN-12013][VL] Fix bloom-filter bytes corruption on whole-stage AQE fallback #12151) to a singleinjectPreregistration: full Velox bloom-filter suite passes, including both whole-stage-reversion scenarios and native-offload preservation, with the TPC-DS/TPC-H plan-stability suite unchanged.