brijrajk commented on PR #12151:
URL: https://github.com/apache/gluten/pull/12151#issuecomment-4907243122
@zhztheplayer -- you are absolutely right, and thank you for pushing on this.
The `FilterExec` fallback is **not** intentional; it is a regression
introduced by an overly-broad pattern match in the logical rule. Here is what
went wrong and what the fix does:
**Root cause**
After the rule was moved to `injectOptimizerRule` (Operator Optimization
batch, offset 487), `InjectRuntimeFilter` (offset 130) runs **before** it. So
when our rule fires, runtime-filter bloom filter expressions of the form
`might_contain(ScalarSubquery, xxhash64(col, seed))` are already present in the
plan.
The rule had two cases:
```scala
// Case 1 -- matched only Attribute values (user-facing bloom filter)
case BloomFilterMightContain(subq: ScalarSubquery, v: Attribute) =>
...rewrite to Velox...
// Case 2 -- catch-all, intended only for SPARK-54336 literals
case bfmc @ BloomFilterMightContain(_: ScalarSubquery, _) => bfmc // leave
vanilla
```
Because `_` in case 2 matches **any** expression, including `xxhash64(col,
seed)`, runtime-filter nodes fell through to the vanilla path. The
`FilterExecTransformer` transformation then had nothing to work with and fell
back to JVM `FilterExec` with C2R/R2C conversions.
My earlier comment that the rule does not observe runtime-filter expressions
was incorrect; I have since verified the batch ordering.
**Fix**
Change the first case guard from `v: Attribute` to
`!v.isInstanceOf[Literal]`:
```scala
// Covers user-facing (Attribute) AND runtime-filter (xxhash64) -- rewrite
both to Velox
case BloomFilterMightContain(subq: ScalarSubquery, v) if
!v.isInstanceOf[Literal] =>
...rewrite inner bloom_filter_agg to VeloxBloomFilterAggregate and wrap in
VeloxBloomFilterMightContain...
// Only Literals reach here (SPARK-54336: might_contain(subq, 0L)) -- leave
both vanilla
case bfmc @ BloomFilterMightContain(_: ScalarSubquery, _) => bfmc
```
**Verification**
- `GlutenTPCDSModifiedPlanStabilitySuite` and
`GlutenTPCDSV1_4_PlanStabilitySuite` (Spark 4.0): q59 now shows
`FilterExecTransformer` (native), matching the committed golden exactly. No
golden regeneration needed since the committed golden was already in the
correct native state.
- `GlutenInjectRuntimeFilterSuite` (Spark 4.0): 13/13 tests pass.
- `GlutenBloomFilterFallbackSuite`: all 6 tests pass, including SPARK-54336.
Also included in this update: added `gluten-delta` as a test dependency to
`gluten-ut/test/pom.xml` and `gluten-ut/spark40/pom.xml`. Without it,
`VeloxDeltaComponent` throws `NoClassDefFoundError` during
`GlutenSessionExtensions.apply()`, which Spark silently catches, resulting in
no Gluten optimizer rules being injected and tests silently running as vanilla
Spark.
--
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]
