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

   ### Describe the bug
   
   Fallback reasons have a separate provenance problem from the coverage tags 
fixed in #5471. If a stale `FALLBACK_REASONS` tag reaches a shared expression 
such as `Literal.TrueLiteral`, an unrelated failed conversion can lift it onto 
its original expression owner. Operator roll-up also collects reasons from 
every descendant. A nonempty stale reason can then satisfy 
`reportUnexplainedFallback`, hiding an otherwise unexplained fallback when 
`spark.comet.explain.fallback.strict.enabled` is enabled.
   
   This is a source-confirmed propagation path conditional on a stale tag; it 
is not a reproduced sequence of two SQL queries. The proposed dynamic partition 
pruning origin still needs investigation. Spark 4.1.3's 
`PlanDynamicPruningFilters` replaces the subquery with a 
`DynamicPruningExpression(TrueLiteral)` wrapper, and its non-AQE rule runs 
before Comet conversion. That source alone does not establish that the shared 
literal receives a fallback reason.
   
   ### Steps to reproduce
   
   Add focused tests that preserve and restore the singleton's previous tags:
   
   1. Seed a distinctive stale fallback reason on `Literal.TrueLiteral`.
   2. Convert an expression containing that literal and decimal arithmetic, so 
decimal promotion rebuilds the surrounding tree. Force the conversion to 
decline and check which reasons are lifted onto the original owner.
   3. Exercise operator roll-up and the strict fallback check with an operator 
that otherwise lacks an explanation. Verify that the planted reason cannot 
masquerade as an explanation for the current conversion.
   4. Also test legitimate literal fallback reasons, including an unsupported 
literal type and explicitly disabled literal support.
   
   These are regression targets, not a claim that all four tests already exist 
or that the singleton contamination producer has been reproduced.
   
   ### Expected behavior
   
   An expression or operator should report reasons belonging to its current 
conversion, without diagnostics from unrelated queries. The strict check should 
continue to detect an otherwise missing reason. Legitimate reasons attached to 
literals must remain visible: applying the coverage-only `isNeverTagged` filter 
to fallback reasons would discard real diagnostics.
   
   ### Additional context
   
   The relevant paths at the reviewed head are [the failure-only fallback 
lift](https://github.com/apache/datafusion-comet/blob/370042350eba68bf720b31f5550f491c94554ba6/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala#L843-L882),
 [operator roll-up and the strict 
check](https://github.com/apache/datafusion-comet/blob/370042350eba68bf720b31f5550f491c94554ba6/spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala#L799-L838),
 and [legitimate literal fallback 
reasons](https://github.com/apache/datafusion-comet/blob/370042350eba68bf720b31f5550f491c94554ba6/spark/src/main/scala/org/apache/comet/serde/literals.scala#L101-L124).
   
   The potential DPP origin should be checked against [Spark 4.1.3's actual 
replacement](https://github.com/apache/spark/blob/v4.1.3/sql/core/src/main/scala/org/apache/spark/sql/execution/dynamicpruning/PlanDynamicPruningFilters.scala#L54-L81),
 not assumed from coverage-tag behavior. Related earlier work: #5229 and #5230. 
Keep this follow-up separate from #5471, which only changes coverage metadata.
   


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