brijrajk opened a new issue, #12668:
URL: https://github.com/apache/gluten/issues/12668

   ### Background
   
   Discussed in review of #12151 (GLUTEN-12013 bloom filter fix): 
https://github.com/apache/gluten/pull/12151#issuecomment-5103480155
   
   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.
   
   ### Verification done
   
   - Verified compiles/builds cleanly for both Velox (Scala 2.13) and 
ClickHouse (Scala 2.12) backends.
   - Prototyped switching `RuntimeBloomFilterRewriteRule` (from #12151) to a 
single `injectPre` registration (dropping both current `injectPreTransform` + 
`injectFinal` registrations): full Velox test suite passes, including 
whole-stage-reversion scenarios and native-offload preservation, with the full 
TPC-DS/TPC-H plan-stability suite (322/322) unchanged.
   - `backends-clickhouse`'s `CHRuleApi.scala` requires zero changes and 
builds/links successfully against the modified `gluten-core`, confirmed via a 
full native + Java/Scala build.
   
   Once merged, #12151 will be rebased to use `injectPre` for 
`RuntimeBloomFilterRewriteRule`, collapsing its current two registrations down 
to one.


-- 
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]

Reply via email to