Re: [PR] feat: support multiply_ym_interval with YearMonth interval codegen dispatch [datafusion-comet]

["sunchao (via GitHub)" <gi...@apache.org>]

sunchao commented on code in PR #4715:
URL: https://github.com/apache/datafusion-comet/pull/4715#discussion_r4105203133


##########
spark/src/main/scala/org/apache/comet/serde/literals.scala:
##########
@@ -240,6 +245,9 @@ object CometLiteral extends CometExpressionSerde[Literal] with CometTypeShim wit
         TimestampType | TimestampNTZType | FloatType | DoubleType | StringType | BinaryType =>
       true
     case _: DecimalType => true
+    // Matched as a type rather than a stable identifier: the start/end fields participate in
+    // `equals`, and every (start, end) pair is carried as the same month count.
+    case _: YearMonthIntervalType => true

Review Comment:
   [P2] Preserve nested empty-array types before admitting these literals. With default constant folding and a nonempty Parquet table `t`, `SELECT array(array(CAST(array() AS ARRAY<INTERVAL YEAR TO MONTH>)), array(array(INTERVAL '1' MONTH))) FROM t` now reaches `literal_to_array_ref`. Its empty-child branch constructs an extra list level, so concatenating the empty and populated branches fails with incompatible Arrow types. Spark returns the valid nested interval array, and the base revision declines this literal and falls back successfully. This admission change therefore turns a working query into a native planning error. Correct the empty nested-list element type in the decoder, or retain fallback for affected literals, and add this folded-array regression case.
   
   Evidence: Spark 4.1.3 executed the query and returned the value `[[[]], [[P1M]]]`; its optimized plan contained the folded literal `[[[]],[[1]]]`. An isolated Rust probe used the exact-head `literal_to_array_ref` function unchanged, the corresponding ListLiteral field values, and Cargo.lock's Arrow 59.3.0. It returned `InvalidArgumentError("It is not possible to concatenate arrays of different data types (List(List(Interval(YearMonth))), List(Interval(YearMonth))).")`. A populated-plus-empty two-level control passed. The base revision's recursive list-literal gate rejects YearMonthIntervalType, whereas the added arm admits this input. This was validated at the decoder boundary, not through a full local Comet execution.



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org