andygrove opened a new issue, #5058: URL: https://github.com/apache/datafusion-comet/issues/5058
### Describe the bug A null literal of `CalendarIntervalType` in a projection throws a native exception instead of producing a null: ``` org.apache.comet.CometNativeException: General execution error with reason: Interval(MonthDayNano) is not supported in Comet. ``` #4898 mapped `CalendarIntervalType` to Arrow `Interval(MonthDayNano)` and added it to the data-type support checks, the codegen kernels, and plan serialization. The null-literal path in the native planner was not updated to match. `create_null_literal`'s match in `native/core/src/execution/planner.rs:389-421` handles Utf8, Date32, Timestamp, Decimal128, Struct, Map, List, Time64, Duration and friends, but has no `Interval` arm, so it hits the catch-all at `:418`. The result is a hard failure rather than a fallback: because the type now passes `isSupportedDataType`, the plan is accepted and only fails once it reaches native execution. Contrast with `YearMonthIntervalType`, which is not in the supported set and so falls back cleanly: ``` Project [COMET: Unsupported data type YearMonthIntervalType(0,1)] ``` That is the behavior a null calendar interval should have at minimum, and ideally it should just evaluate natively. ### Steps to reproduce ```sql SELECT CAST(NULL AS INTERVAL); ``` This is easy to hit indirectly. `MakeInterval` is `NullIntolerant`, so `NullPropagation` rewrites any call with a literal NULL argument into a null interval literal, and the original expression disappears before Comet sees it: ```sql SELECT make_interval(NULL, 2, 3, 4, 5, 6, 7.008009); ``` Both fail with the exception above. Non-null calendar interval literals are fine, and calendar interval values flowing through a `LocalTableScan` (as in `calendar_interval.sql`) are fine. It is specifically the null literal in a projection. ### Expected behavior `SELECT CAST(NULL AS INTERVAL)` returns a single null row, matching Spark. Adding an `Interval(MonthDayNano)` arm producing `ScalarValue::IntervalMonthDayNano(None)` to the null-literal match should cover it. Worth checking `Interval(YearMonth)` and `Interval(DayTime)` at the same time, since they are absent from that match too, though they are currently shielded by the type check. ### Additional context Found while adding `make_interval` codegen-dispatch support in #5030. That PR works around it by driving its NULL coverage from nullable columns rather than NULL literals, since literal NULL arguments are constant-folded away by `NullPropagation` and never exercise the expression anyway. Related: #4540 (interval type support epic), #4898 (the PR that added `CalendarIntervalType`). -- 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]
