coderfender opened a new issue, #6442:
URL: https://github.com/apache/datafusion-comet/issues/6442

   ### Describe the bug
   
   When a broadcast (or shuffled) hash join declines Comet conversion and falls 
back to Spark, the fallback reason recorded via `withFallbackReason` is present 
on the **first** `CometExecRule` pass but is **missing from the AQE-final 
executed plan**. As a result the `[COMET: <reason>]` annotation does not appear 
in `EXPLAIN` output (nor in `df.queryExecution.executedPlan`) when AQE is 
enabled (the default).
   
   Correctness is unaffected — the join still falls back and produces correct 
results. This is purely about the explain-time fallback-reason annotation.
   
   ### Root cause
   
   `CometExecRule` tags a broadcast join's fallback reason only via the 
broadcast-child branch:
   
   ```scala
   case plan if plan.children.exists(_.isInstanceOf[BroadcastExchangeExec]) =>
   ```
   
   On the first pass the child is a raw `BroadcastExchangeExec`, so the join 
serde runs and the reason is copied onto the returned (declined) join node. But 
AQE's `reOptimize` re-plans from the logical plan and produces a **new** join 
instance whose build side is now a `BroadcastQueryStageExec`. TreeNode tags do 
not travel to the new instance, and on the re-optimize pass the branch above no 
longer matches (the child is a query stage, not a `BroadcastExchangeExec`), so 
the serde never re-runs and the new join is never re-tagged.
   
   A *convertible* broadcast join survives because its child becomes a 
`CometBroadcastExchangeExec` → `BroadcastQueryStageExec` (matched at the 
`BroadcastQueryStageExec(_, _: CometBroadcastExchangeExec, _)` case), giving 
the join native children so it re-converts (and re-tags) through the `allExecs` 
handler. Only the *declined* case loses its reason.
   
   ### Scope
   
   - Not specific to existence joins — any broadcast/hash join that declines 
conversion loses its reason under AQE. Surfaced by the existence-join fallback 
tests (`CometJoinSuite`), which had to disable AQE to observe the tagged reason.
   - Explain-only, AQE-only. With `COMET_EXPLAIN_FALLBACK_ENABLED` the reason 
still reaches the warning log from the first pass; it is just absent from the 
final plan tree.
   
   ### Suggested fix
   
   Re-run the join serde for its tagging side effect (or copy the reason 
forward) when a broadcast/hash join declined conversion and its build side is 
now a `BroadcastQueryStageExec`, so the reason lands on the AQE-final plan node.
   
   ### Context
   
   Split out from PR #4587 (native ExistenceJoin), where `CometJoinSuite` tests 
for residual-condition and computed-key fallbacks disable AQE as a workaround 
pending this fix.


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