brijrajk opened a new pull request, #12669: URL: https://github.com/apache/gluten/pull/12669
## What changes are proposed in this pull request? Fixes #12668. Discussed in review of #12151 (GLUTEN-12013 bloom filter fix): https://github.com/apache/gluten/pull/12151#issuecomment-5103480155 ### Background Today, `ExpandFallbackPolicy`'s whole-stage-fallback revert target (`originalPlan` in `HeuristicApplier.makeRule`) is captured before *any* physical rule runs -- including `injectPreTransform` rules. This means a rule registered at `injectPreTransform` gets its rewrite stripped away whenever `ExpandFallbackPolicy` promotes an individual-stage fallback to a whole-stage one, requiring a second re-application registered at `injectFinal` as a workaround (see `RuntimeBloomFilterRewriteRule` in #12151). ### Proposal Add a new `injectPre` hook to `GlutenInjector.LegacyInjector`, with its own `"pre"` phase in `HeuristicApplier.makeRule`, running before `"transform"`. `fallbackPolicies` 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` -- removing the need for the `injectFinal` re-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 five `FallbackStrategiesSuite`/`GlutenFallbackStrategiesSuite` test files across `gluten-ut/spark{33,34,35,40,41}` construct `HeuristicApplier` directly (bypassing the injector layer) and needed a trivial update (an extra `Nil` argument) for the new constructor parameter. Once merged, #12151 will be rebased to use `injectPre` for `RuntimeBloomFilterRewriteRule`, collapsing its current two registrations (`injectPreTransform` + `injectFinal`) down to one. ## How was this patch tested? - Full 7-suite TPC-DS/TPC-H plan-stability set (322/322): confirms the new no-op-by-default `"pre"` phase changes no existing plan or behavior. - `backends-clickhouse` builds, links, and packages successfully unmodified against the changed `gluten-core` (`CHRuleApi.scala` never calls `injectPre`). - Prototyped switching `RuntimeBloomFilterRewriteRule` (#12151) to a single `injectPre` registration: 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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
