peterxcli commented on PR #5133: URL: https://github.com/apache/datafusion-comet/pull/5133#issuecomment-5441852668
Verified the test protects the fix: with the `Interval(MonthDayNano)` arm removed and everything else unchanged, `CometSqlFileTestSuite calendar_interval` fails with exactly the issue's error (`Interval(MonthDayNano) is not supported in Comet`), and passes with it. So `ConstantFolding` doesn't fold the literal away — it reaches the native planner. On the other types: a null `Interval(YearMonth)` literal can't reach this match today. `CometLiteral.getSupportLevel` gates on `supportedDataType`, which doesn't include `YearMonthIntervalType`, so those literals fall back to Spark rather than failing natively. I'd rather add that arm in #5161, where the type actually becomes reachable. `LargeUtf8`/`Utf8View`/`FixedSizeBinary` can't appear here at all — `to_arrow_datatype` never produces them from the proto. So after this PR the explicit match covers every type the serde can emit, which is the state I wanted before considering the `try_from` collapse (per the earlier review round). -- 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]
