Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ object VeloxRuleApi {
private def injectSpark(injector: SparkInjector): Unit = {
// Inject the regular Spark rules directly.
injector.injectOptimizerRule(CollectRewriteRule.apply)
injector.injectOptimizerRule(BloomFilterMightContainJointRewriteRule.apply)
injector.injectOptimizerRule(HLLRewriteRule.apply)
injector.injectOptimizerRule(CollapseGetJsonObjectExpressionRule.apply)
injector.injectOptimizerRule(RewriteCastFromArray.apply)
Expand Down Expand Up @@ -81,11 +82,7 @@ object VeloxRuleApi {
injector.injectPreTransform(c => FallbackMultiCodegens.apply(c.session))
injector.injectPreTransform(c => MergeTwoPhasesHashBaseAggregate(c.session))
injector.injectPreTransform(_ => RewriteSubqueryBroadcast())
injector.injectPreTransform(
c =>
BloomFilterMightContainJointRewriteRule.apply(
c.session,
c.caller.isBloomFilterStatFunction()))
injector.injectPreTransform(c => RuntimeBloomFilterRewriteRule(c.session))
injector.injectPreTransform(_ => EliminateRedundantGetTimestamp)

// Legacy: The legacy transform rule.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,63 +21,71 @@ import org.apache.gluten.expression.VeloxBloomFilterMightContain
import org.apache.gluten.expression.aggregate.VeloxBloomFilterAggregate

import org.apache.spark.sql.SparkSession
import org.apache.spark.sql.catalyst.expressions.{BinaryExpression, BloomFilterMightContain, Expression}
import org.apache.spark.sql.catalyst.expressions.aggregate.{BloomFilterAggregate, TypedImperativeAggregate}
import org.apache.spark.sql.catalyst.expressions.{BloomFilterMightContain, Literal, ScalarSubquery}
import org.apache.spark.sql.catalyst.expressions.aggregate.{AggregateExpression, BloomFilterAggregate}
import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
import org.apache.spark.sql.catalyst.rules.Rule
import org.apache.spark.sql.execution.SparkPlan

case class BloomFilterMightContainJointRewriteRule(
spark: SparkSession,
isBloomFilterStatFunction: Boolean)
extends Rule[SparkPlan] {
override def apply(plan: SparkPlan): SparkPlan = {
if (isBloomFilterStatFunction || !GlutenConfig.get.enableNativeBloomFilter) {
/**
* Optimizer rule that rewrites `BloomFilterAggregate` -> `VeloxBloomFilterAggregate` and
* `BloomFilterMightContain` -> `VeloxBloomFilterMightContain` at the logical plan level.
*
* Running as an optimizer rule ensures the substitution is captured in the `originalPlan` snapshot
* that [[org.apache.gluten.extension.columnar.heuristic.ExpandFallbackPolicy]] uses when promoting
* an individual stage fallback to a whole-stage AQE fallback. This guarantees that both sides of
* the bloom-filter pair always produce and consume the same byte format, regardless of whether
* stages fall back to JVM execution after AQE re-planning.
*
* This rule only covers bloom filters that are present in the plan during Spark's Operator
* Optimization batch, i.e. the ones written by the user. Runtime bloom filters are injected by
* `InjectRuntimeFilter`, which runs in a later batch of `SparkOptimizer`, after this rule has
* already fired -- they are handled by the physical-level [[RuntimeBloomFilterRewriteRule]]
* instead. The aggregate (producer) and the might-contain (consumer) are always rewritten as a
* pair, or not at all, so they never end up on different serialized byte formats:
* - `might_contain(ScalarSubquery(...), <non-literal>)`: rewrite both sides to their Velox forms
* (version=1). This is the path that GLUTEN-12013 protects across whole-stage AQE fallback.
* - `might_contain(ScalarSubquery(...), <literal>)` (as in SPARK-54336): leave both vanilla
* (version=0). Rewriting only the outer side to Velox while the inner aggregate stayed vanilla
* `bloom_filter_agg` is exactly what caused the `kBloomFilterV1 == version` (1 vs. 0) crash.
* Leaving both vanilla also preserves vanilla's NULL-on-empty-input semantics.
*
* Standalone `BloomFilterAggregate` (e.g., `DataFrame.stat.bloomFilter()`) is never matched, so its
* bytes stay in Spark-native format.
*/
case class BloomFilterMightContainJointRewriteRule(spark: SparkSession)
extends Rule[LogicalPlan] {

override def apply(plan: LogicalPlan): LogicalPlan = {
if (!GlutenConfig.get.enableNativeBloomFilter) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the early-exit if (!GlutenConfig.get.enableNativeBloomFilter) return plan is untested. Can we add a test case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a test that sets spark.gluten.sql.native.bloomFilter=false and checks the query still returns the correct count and the optimized plan does not contain velox_might_contain.

return plan
}
val out = plan.transformWithSubqueries {
case p =>
applyForNode(p)
}
out
}

private def replaceBloomFilterAggregate[T](
expr: Expression,
bloomFilterAggReplacer: (
Expression,
Expression,
Expression,
Int,
Int) => TypedImperativeAggregate[T]): Expression = expr match {
case BloomFilterAggregate(
child,
estimatedNumItemsExpression,
numBitsExpression,
mutableAggBufferOffset,
inputAggBufferOffset) =>
bloomFilterAggReplacer(
child,
estimatedNumItemsExpression,
numBitsExpression,
mutableAggBufferOffset,
inputAggBufferOffset)
case other => other
}

private def replaceMightContain[T](
expr: Expression,
mightContainReplacer: (Expression, Expression) => BinaryExpression): Expression = expr match {
case BloomFilterMightContain(bloomFilterExpression, valueExpression) =>
mightContainReplacer(bloomFilterExpression, valueExpression)
case other => other
}

private def applyForNode(p: SparkPlan) = {
p.transformExpressions {
case e =>
replaceMightContain(
replaceBloomFilterAggregate(e, VeloxBloomFilterAggregate.apply),
VeloxBloomFilterMightContain.apply)
plan.transformAllExpressions {
case BloomFilterMightContain(subq: ScalarSubquery, v) if !v.isInstanceOf[Literal] =>
// Non-literal value: rewrite both the outer might-contain and the inner aggregate to
// Velox format so that both sides always produce/consume the same byte layout (version=1),
// even when stages fall back to JVM execution via ExpandFallbackPolicy.
val rewrittenPlan = subq.plan.transformAllExpressions {
case ae @ AggregateExpression(b: BloomFilterAggregate, _, _, _, _) =>
ae.copy(aggregateFunction = VeloxBloomFilterAggregate(
b.child,
b.estimatedNumItemsExpression,
b.numBitsExpression,
b.mutableAggBufferOffset,
b.inputAggBufferOffset))
}
VeloxBloomFilterMightContain(subq.withNewPlan(rewrittenPlan), v)
case bfmc @ BloomFilterMightContain(_: ScalarSubquery, _) =>
// Literal value (SPARK-54336: `might_contain((SELECT bloom_filter_agg(col) FROM t), 0L)`).
// Leave BOTH the inner aggregate and the outer might-contain as vanilla Spark expressions
// so they stay on the same Spark-native (version=0) byte format, and so an empty aggregate
// input still yields vanilla's NULL bloom filter. Rewriting only the outer side to Velox
// (which expects version=1) while the inner aggregate stays vanilla `bloom_filter_agg` (no
// Substrait mapping -> JVM -> version=0) is what caused the `kBloomFilterV1 == version`
// (1 vs. 0) crash.
bfmc
case BloomFilterMightContain(bf, v) =>
// Pre-computed literal bloom filter bytes -- rewrite to consume Velox-format bytes.
VeloxBloomFilterMightContain(bf, v)
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.gluten.extension

import org.apache.gluten.config.GlutenConfig
import org.apache.gluten.expression.VeloxBloomFilterMightContain
import org.apache.gluten.expression.aggregate.VeloxBloomFilterAggregate

import org.apache.spark.sql.SparkSession
import org.apache.spark.sql.catalyst.expressions.{BloomFilterMightContain, XxHash64}
import org.apache.spark.sql.catalyst.expressions.aggregate.{AggregateExpression, BloomFilterAggregate}
import org.apache.spark.sql.catalyst.rules.Rule
import org.apache.spark.sql.execution.SparkPlan

/**
* Physical pre-transform rule that rewrites runtime-filter bloom filters (the ones injected by
* Spark's `InjectRuntimeFilter` optimizer rule) to their Velox variants so they offload natively.
*
* Runtime bloom filters cannot be handled by [[BloomFilterMightContainJointRewriteRule]]: that rule
* is registered via `injectOptimizerRule`, which lands in Spark's Operator Optimization batch,
* while `InjectRuntimeFilter` runs in a later batch of `SparkOptimizer`. The runtime-filter
* expressions therefore do not exist yet when the logical rule fires, and a physical-level rewrite
* (as was always done before the logical rule was introduced) is required to keep
* `FilterExecTransformer` and the bloom-filter aggregate native.
*
* The rewrite is restricted to `InjectRuntimeFilter`'s exact expression shapes, which always wrap
* the key in [[XxHash64]] on both the producer and the consumer side:
* - producer: `bloom_filter_agg(xxhash64(key), ...)` -> `velox_bloom_filter_agg(...)`
* - consumer: `might_contain(bf, xxhash64(key))` -> `velox_might_contain(...)`
*
* Because each side is identifiable on its own, both are rewritten consistently to the Velox byte
* format (version=1) even when AQE compiles the bloom-filter subquery separately from the consuming
* filter stage. The `XxHash64` fingerprint also guarantees the other bloom-filter populations are
* never touched:
* - `DataFrame.stat.bloomFilter()` builds `bloom_filter_agg(col, ...)` on the raw column (no
* `XxHash64` wrapper) and deserializes the result with Spark's `BloomFilter.readFrom`, so its
* bytes must stay in Spark-native format.
* - User-facing `might_contain(<scalar subquery>, <value>)` pairs are already rewritten at the
* logical level by [[BloomFilterMightContainJointRewriteRule]] (the GLUTEN-12013 fix), making
* this rule a no-op for them.
* - Literal-value pairs (SPARK-54336) contain no `XxHash64` and stay fully vanilla.
*/
case class RuntimeBloomFilterRewriteRule(spark: SparkSession) extends Rule[SparkPlan] {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brijrajk, thank you so much for the update.

For runtime bloom filters, could you clarify what happens if the producer and consumer become inconsistent, i.e., one is offloaded to Velox while the other falls back due to the fallback policy?

@brijrajk brijrajk Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@philo-he Good question. Verified at three levels: code inspection, Spark-source trace, and runtime probes in a container. Summary:

1. Operator-level mixes are safe by construction. This rule rewrites the expressions at pre-transform. Wherever the operators land (native or JVM), the expressions stay VeloxBloomFilterAggregate / VeloxBloomFilterMightContain, which run on the JVM via JNI with the same version=1 byte format. Byte format travels with the expression class, not the operator.

Probe Non-default configs Result
A none SUCCESS, correct rows
B columnar.filter.enabled=false (consumer operator on JVM) SUCCESS
C columnar.hashagg.enabled=false (producer operator on JVM) SUCCESS

2. Full disclosure: whole-stage fallback reversion is a real gap. HeuristicApplier captures originalPlan before pre-transform, so ExpandFallbackPolicy reversion strips the rewrite from that stage only, while the subquery is prepared independently (PlanSubqueries). With the fallback thresholds enabled, formats can split in both directions:

Probe Non-default configs Result
D columnar.filter.enabled=false + wholeStage.fallback.threshold=1 CRASH: Velox (1 vs. 0) Unsupported BloomFilter version: 0
E wholeStage.fallback.threshold=1 only CRASH: IOException: Unexpected Bloom filter version number (16777218)

3. Caveats bounding the gap:

  • Unreachable with default configs (both thresholds default to -1).
  • Pre-existing: I reran all five probes with main's bloom-filter code in the same environment; main crashes identically on D and E. This PR neither introduces nor widens it.
  • Fails loudly (version-check errors on both sides); never a silent wrong result.
  • The user-facing path is already reversion-safe in this PR: the logical rule bakes the rewrite into originalPlan itself (that is the GLUTEN-12013 fix, proven by the threshold=1/2 tests).

4. Why not fixable the same way here: it needs a logical rewrite after InjectRuntimeFilter, and Spark's extension API has no hook between that batch and physical planning. Re-patching fallen-back plans is the "patcher" we removed earlier at review feedback.

5. Candidate fixes (follow-up):

  • Propose an upstream Spark extension point for post-InjectRuntimeFilter logical rules, then move the runtime-filter rewrite there.
  • Or a narrowly-guarded joint re-rewrite in the final rule phase (applies to fallen-back plans too), restricted to the xxhash64 shape on both sides.

@philo-he @zhztheplayer what do you think the next steps should be for resolving this gap: a follow-up issue (I can file it with the probe D/E reproduction), or do you consider it in scope for this PR?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brijrajk, thanks for the investigation.

A cleaner fix might be a Spark extension API that lets our logical rule run after InjectRuntimeFilter, so the rewrite lands in the originalPlan snapshot (which is the actual plan in stage fallback situation). Not sure upstream would accept it.

Alternatively, we could change the fallback policy to block whole-stage fallback when a stage contains bloom-filter. That keeps both producer and consumer native and consistent, at the cost of overriding the fallback decision.

@zhztheplayer, do you have any comments?

@zhztheplayer zhztheplayer Jul 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, we could change the fallback policy to block whole-stage fallback when a stage contains bloom-filter.

Is there any possible risk, if we always replace bloomfilter_agg and might_contain with velox_bloomfilter_agg and velox_might_contain in the Gluten's physical optimization phase, despite the whole-stage fallback status? If it's doable, perhaps we can directly move the existing physical rule BloomFilterMightContainJointRewriteRule to injectFinal, which is executed after wholestage fallback?

@brijrajk by the way, in the current PR status, why BloomFilterMightContainJointRewriteRule is still kept as a logical rule given it's not a valid injection point, did I miss something?

@brijrajk brijrajk Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhztheplayer @philo-he I prototyped the injectFinal idea and probed it at runtime instead of answering from theory. Findings below, including one correction to my own Jul 14 comment.

injectFinal PoC results (zhztheplayer's Q1)

Implemented as keep-preTransform-plus-add-final (a pure move would regress offload: validation needs the Velox expression before HeuristicTransform). Same xxhash64 guards.

Probe Configs Before PoC With PoC
D filter=false + wholeStage.fallback.threshold=1 CRASH (velox, 1 vs. 0) 10 rows, correct
E wholeStage.fallback.threshold=1 only CRASH (IOException 16777218) 6 of 10 rows, silent wrong result
Guards (stat / SPARK-54336 / native-off) pass pass

Probe E is deterministic (same missing keys across reruns). A bloom filter cannot produce false negatives, so the filter itself was corrupt. Strictly worse than the crash, so I have not pushed it.

Root cause: capacity divergence (new finding)

Probe E's plan shows a phase split: native partial (FlushableHashAggregateExecTransformer[VeloxBloomFilterAggregate:Partial]) feeding a reverted JVM final (ObjectHashAggregateExec[VeloxBloomFilterAggregate:Final]). Same byte format, but the two engines size the buffer differently.

estimatedNumItems/numBits are identical on both engines (1000000/8388608, Spark's plain defaults; my test table has no ANALYZE stats). The divergence is not a different capacity source -- it's two different sizing formulas over the same two literals: JVM's createAggregationBuffer() sizes from estimatedNumItems alone (VeloxBloomFilter.empty(1000000) -> 16,777,216 bits), native's BloomFilterAggAggregate.cpp sizes from numBits alone (capacity_ = numBits/16 -> 8,388,608 bits). Exactly 2x, deterministic, independent of stats or config. Velox's BloomFilter::merge guards the size match with VELOX_DCHECK_EQ (compiled out in release) then ORs the full range: silent corruption.

Correction to my Jul 14 comment: probe E's original crash was this same phase split (vanilla final agg reading native partial buffers), not the consumer reverting; the consumer stayed a native FilterExecTransformer throughout.

Why the logical rule stays (zhztheplayer's Q2)

  1. The literal-vs-non-literal pair decision (SPARK-54336 vs GLUTEN-12013) needs whole-plan visibility; after PlanSubqueries, a producer-side bloom_filter_agg(col) alone is ambiguous, and there is no xxhash64-style fingerprint to disambiguate.
  2. It bakes the rewrite into originalPlan, so the user-facing population is reversion-safe with zero patching (proven by the threshold=1/2 tests).

The three options (incl. philo-he's proposals)

Option Verdict Notes
injectFinal re-rewrite Safe only after capacity alignment Fix: JVM buffer sized the same way as native (e.g. from numBits), or adopt incoming size on first merge (bits_.empty() branch supports it). Also propose to Velox: promote the DCHECK to a user check.
Fallback-policy block (philo-he) Most robust short-term Only option preventing both split kinds (producer/consumer AND partial/final); no capacity prerequisite. Layering solvable: pass a bloom predicate from VeloxRuleApi where ExpandFallbackPolicy is constructed. Cost: overrides user's fallback intent.
Upstream Spark extension point (philo-he) Best long-term Deletes the physical rule; but does not cover the phase split by itself, so capacity alignment is needed regardless. Happy to draft the SPARK JIRA.

Proposed sequencing

  1. Merge this PR as-is: fixes GLUTEN-12013 + SPARK-54336, restores native runtime filters, zero golden changes; the residual gap is default-off, fails loudly, and is byte-identical to main (same probes run against main's bloom code).
  2. Follow-up 1: capacity alignment + a phase-split regression test (prerequisite).
  3. Follow-up 2: close the reversion gap on top, via injectFinal or the policy predicate, your preference.
  4. Long-term: SPARK JIRA for the extension point.

Happy to file the follow-ups with the probe reproductions. Does this sequencing work for you both?

@brijrajk brijrajk Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhztheplayer You're right, injectFinal does always execute, and it does correctly rewrite the expression. That part works. The problem is one level deeper: the rewrite fixes byte format, not bloom capacity.

I re-verified this by building and running in the container with added diagnostics rather than guessing. estimatedNumItems/numBits are identical on both engines (1000000/8388608 -- Spark's plain defaults, since my test table has no ANALYZE stats). There is no source divergence. The real bug: JVM and native compute the buffer size from the same two literals using different formulas, each using only one of the two arguments.

Path Formula Result for (items=1000000, bits=8388608)
JVM VeloxBloomFilterAggregate.createAggregationBuffer() (line 102) VeloxBloomFilter.empty(estimatedNumItems) -> Velox reset(1000000) -> nextPow2(1000000)/4 words. Ignores numBits entirely. 16,777,216 bits
Native BloomFilterAggAggregate.cpp capacity_ = numBits/16 -> reset(capacity_). Ignores raw item count once numBits is present. 8,388,608 bits

Exactly 2x, deterministic, and independent of table stats or session config.

Probe Partial stage Final stage Sizes match? Result
A baseline native native yes 10/10
D filter=false + threshold=1 JVM JVM yes (both JVM formula) 10/10
E threshold=1 only native JVM no 6/10

Probe D reverts the whole subquery stage together, so both ends use the same formula and stay consistent. Probe E reverts only the final-aggregation stage while partial stays native -- native partial sets bits modulo an 8,388,608-bit array; the JVM final buffer (sized for 16,777,216 bits) merges those bits into the first half of a differently-sized array (unchecked, per the DCHECK noted earlier), and later queries hash modulo the larger array, so previously-set bits land at different positions than where they were inserted.

Bottom line unchanged: any fix needs the JVM aggregate's buffer sizing reconciled with the native formula (e.g. size from numBits the same way, or adopt the incoming filter's size on first merge), independent of whether we land on injectFinal, the fallback-policy block, or the upstream extension point.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brijrajk Thanks for the summary.

So can we just fix the JVM buffer creation code to align with the native code? Then keep one single rule in the planner?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhztheplayer Short answer: the capacity fix stands on its own and is worth doing regardless. Consolidating to one rule doesn't work as a simple move though. I went ahead and built it to check.

What I built and ran

Capacity fix plus one unified rule, registered at both preTransform and injectFinal, with the logical rule dropped entirely. Ran it against the existing regression suite (GlutenBloomFilterFallbackSuite).

What failed

4 of the 7 tests crash with the same (1 vs. 0) version mismatch this PR exists to fix:

Test Needs producer/consumer coordination? Result
threshold=2 fallback Yes FAILED, (1 vs. 0)
threshold=1 fallback Yes FAILED, same crash
JVM-mode subquery Yes FAILED, same crash
SPARK-54336 literal Yes FAILED, same crash
stat.bloomFilter No passed
native.bloomFilter=false No passed
runtime-filter-native No, xxhash64 alone is enough signal passed

The split is clean: anything needing "check the consumer's value, then update the producer to match" fails. Anything that doesn't need that coordination passes.

Why it fails

I logged the plan before and after collect() on the threshold=2 test to see what was actually happening:

Filter (consumer) Subquery aggregate (producer)
Before collect() vanilla vanilla
After collect() rewritten correctly still vanilla

Same rule, same match arm, same subq.withNewPlan(...) call. Only half of it takes effect at runtime.

Here's why: InsertAdaptiveSparkPlan.buildSubqueryMap compiles and caches each subquery's physical plan once, keyed by exprId, before Gluten's rule ever runs. A physical rule that reaches into the subquery and rewrites it produces a perfectly valid new object, but the subquery executes off that earlier cached copy, not the new one. The consumer rewrite works because it's just a plain expression swap sitting in the outer plan. The producer rewrite needs the cache itself to change, and there's no way to reach it from a rule running this late.

That also explains why the runtime-filter rule was never affected: it doesn't reach into the subquery manually at all, it matches the aggregate expression wherever the framework's own traversal already visits it, so it's wired correctly by construction. And it explains why the logical rule works: it rewrites the subquery's logical plan before the cache is even built, so the cache is already correct by the time AQE captures it.

Suggestion

Keep the two rules as they are for this PR. File the capacity fix as its own follow-up since it doesn't depend on any of this. And treat "can a physical rule ever be made reversion-safe for the coordinated case" as a separate question, one the Spark extension point idea might actually answer better than anything we can do rule-side.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my understanding, if we can fix the capacity issue first, then the error you met on native partial stage + JVM final stage during experimenting injectFinal can be fixed?

If yes can you first open a PR to align the capacity, which can be merged first, then in this PR you can update to an injectFinal solution? Does that work for you, or did I miss something there?

@brijrajk brijrajk Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhztheplayer Yes, confirmed. I built exactly this (capacity fix, plus registering the existing RuntimeBloomFilterRewriteRule at injectFinal too, nothing else changed) and it works cleanly.

Full existing regression suite (GlutenBloomFilterFallbackSuite): 7/7 passed, since the logical rule is untouched.

The adversarial probe that was silently wrong before (native partial stage + JVM final stage) now returns correct results:

Probe Before capacity fix After capacity fix + injectFinal
Whole-stage reversion, both sides JVM 10/10 correct 10/10 correct
Partial-only reversion, native partial + JVM final silently wrong, 6/10 10/10 correct

This works because RuntimeBloomFilterRewriteRule never reaches into the subquery's plan and rebuilds it. It matches the producer and the consumer independently, each identified purely by the xxhash64 wrapper, and lets Spark's own traversal wire the rewrite in wherever it lands. That's different from what broke in the full-consolidation version I tried earlier, where the rule had to reach into the subquery manually to coordinate the literal check, and that manual rewrite didn't survive to execution.

I opened the capacity fix as its own PR: #12614 (issue #12613). Verified there too: reverting just the fix while keeping the new regression test makes exactly that one test fail, with everything else in the suite unaffected, so the test is genuinely pinned to the bug.

Once #12614 merges, I will update this PR to add the injectFinal registration on top.

override def apply(plan: SparkPlan): SparkPlan = {
if (!GlutenConfig.get.enableNativeBloomFilter) {
return plan
}
plan.transformWithSubqueries {
case p =>
p.transformExpressions {
case ae @ AggregateExpression(b: BloomFilterAggregate, _, _, _, _)
if b.child.isInstanceOf[XxHash64] =>
ae.copy(aggregateFunction = VeloxBloomFilterAggregate(
b.child,
b.estimatedNumItemsExpression,
b.numBitsExpression,
b.mutableAggBufferOffset,
b.inputAggBufferOffset))
case BloomFilterMightContain(bf, value: XxHash64) =>
VeloxBloomFilterMightContain(bf, value)
}
}
}
}
Loading
Loading