brijrajk commented on PR #12151:
URL: https://github.com/apache/gluten/pull/12151#issuecomment-5196402896
@zhztheplayer Yes, still needed. I tried both ways of removing it; both
fail, for independent reasons. Branch is rebased onto current main, where the
baseline is `GlutenBloomFilterFallbackSuite` 8/8 and
`GlutenBloomFilterAggregateQuerySuite` + `GlutenInjectRuntimeFilterSuite` 43/43.
**1. Joint rule ported to a physical rule at `injectPre`** -- 3/8 fallback
tests fail with the original crash back (`kBloomFilterV1 == version`, 1 vs 0).
The producer is unreachable from a physical rule. Instrumenting the reach-in
shows:
```
subqPlanClass=SubqueryExec child0=AdaptiveSparkPlanExec
aggsReachedByTransform=0 planUnchanged=true
treeStringMentionsVanillaAgg=true
```
The vanilla `bloom_filter_agg` is present in the subquery's tree, but
`AdaptiveSparkPlanExec` is a `LeafExecNode`, so traversal stops there and the
aggregate is untouchable. The consumer still gets rewritten to Velox, which is
exactly the v1/v0 split that crashes. The mismatch is concrete: Velox
serializes the version as a single `int8` of 1, while Spark's
`BloomFilterImpl.writeTo` writes a 4-byte big-endian int, so vanilla bytes
present `0x00` as their first byte, hence "version: 0".
Independently, AQE compiles the subquery from the logical plan
(`InsertAdaptiveSparkPlan.compileSubquery(LogicalPlan)`), so a physical
mutation would not be what executes.
This is not about choosing an earlier hook. AQE compiles the scalar subquery
into its own `AdaptiveSparkPlanExec` with independent fallback decisions, so
the last point at which any rule Gluten can inject sees both sides in one
rewritable tree is the optimized logical plan.
**2. One unified logical rule registered twice** (normal optimizer batch
plus `spark.experimental.extraOptimizations`, so it also fires after
`InjectRuntimeFilter`), replacing both rules. This passes the fallback suite
8/8 with no native crashes, but fails 11/43:
```
scala.MatchError: velox_bloom_filter_agg(xxhash64(c2#10018, 42), ...) AS
bloomFilter#10054
at
...InjectRuntimeFilterSuite...applyOrElse(InjectRuntimeFilterSuite.scala:259)
```
`getNumBloomFilters` matches over `optimizedPlan` expecting the vanilla
shape. One of the 11 is `GLUTEN-9849: bloom filter applied to partition
filter`, which is Gluten's own test, so this cannot be waved off as inherited
expectations. Also, `extraOptimizations` is a public mutable var: any user
assigning to it silently drops the rule and reintroduces a native crash.
**Why the split is right.** The two populations differ in one property:
`InjectRuntimeFilter` stamps `XxHash64` on both producer and consumer, so each
side is self-identifying and can be rewritten independently at the physical
level, keeping the logical plan vanilla. User-written `might_contain(<scalar
subquery>, <value>)` has no such marker, so the rewrite must be joint, and
therefore logical.
Longer term this constraint could be dissolved by teaching the native side
to read Spark's bloom-filter layout, but that is a Velox-side change.
--
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]
