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

   ### Describe the bug
   
   `CometStringLPad` and `CometStringRPad` build their native expression with 
`scalarFunctionExprToProto` and do not wrap it in `withNullShortCircuit`:
   
   - 
https://github.com/apache/datafusion-comet/blob/8f4ce5e643e9c10803369dae6a1646195f36cb72/spark/src/main/scala/org/apache/comet/serde/strings.scala#L568-L620
   - `withNullShortCircuit`: 
https://github.com/apache/datafusion-comet/blob/8f4ce5e643e9c10803369dae6a1646195f36cb72/spark/src/main/scala/org/apache/comet/serde/QueryPlanSerde.scala#L1408
   
   In Spark, `StringLPad` / `StringRPad` are `TernaryExpression`s with 
`nullIntolerant = true`. `TernaryExpression.eval` evaluates the `len` and `pad` 
children only when `str` is non-null. Comet evaluates all children for every 
row, so when a child can fail (for example an ANSI cast), Comet can raise an 
error on a row where `str` is NULL and Spark would have returned NULL without 
evaluating that child.
   
   This came up in review of 
https://github.com/apache/datafusion-comet/pull/7450: 
https://github.com/apache/datafusion-comet/pull/7450#discussion_r4511335598
   
   ### Steps to reproduce
   
   Found by reading the code; I have not run this repro yet.
   
   ```sql
   SET spark.sql.ansi.enabled=true;
   CREATE TABLE tbl(s STRING, t STRING) USING parquet;
   INSERT INTO tbl VALUES (NULL, 'abc');
   SELECT lpad(s, CAST(t AS INT)) FROM tbl;
   SELECT rpad(s, CAST(t AS INT)) FROM tbl;
   ```
   
   ### Expected behavior
   
   Both queries return NULL, as Spark does, because `s` is NULL and Spark never 
evaluates `CAST(t AS INT)` for that row. Comet is expected to raise a cast 
error instead.
   
   ### Additional context
   
   Other serdes already use `withNullShortCircuit` for this (for example in 
`arrays.scala`, `maps.scala` and `datetime.scala`). The fix is probably to wrap 
the lpad/rpad proto in `withNullShortCircuit` and add a SQL file test for the 
case above.
   
   ### Willingness to contribute
   
   I can contribute a fix for this bug independently
   


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