andygrove opened a new pull request, #5014: URL: https://github.com/apache/datafusion-comet/pull/5014
## Which issue does this PR close? Closes #5012. ## Rationale for this change With `spark.sql.ansi.enabled=true`, `CAST(string AS date)` must raise `CAST_INVALID_INPUT` for malformed input. Comet returned `NULL` instead when the string parsed structurally but did not name a real calendar date, e.g. `'2016-13-01'`, `'2016-02-30'`, `'2016-01-32'`. Beyond the missing exception, this silently changes results: a filter on the cast returns rows for a query that Spark fails outright. ```sql SELECT count(*) FROM t WHERE cast(s as date) > date'2000-01-01' -- spark: SparkDateTimeException -- comet: 1 ``` Spark's `SparkDateTimeUtils#stringToDate` returns `None` whenever `LocalDate.of` throws or the narrowing of the epoch day to an `Int` overflows (`localDateToDays` uses `Math.toIntExact`), and `stringToDateAnsi` turns every `None` into `CAST_INVALID_INPUT`. Comet's port had three paths that produced `Ok(None)` without going through `return_result`, which is the only place the `EvalMode::Ansi` error is raised. ## What changes are included in this PR? In `date_parser` (`native/spark-expr/src/conversion_funcs/string.rs`): * The canonical `yyyy-mm-dd` fast path now routes an invalid calendar date through `return_result` instead of returning `Ok(None)`. * The general parser path does the same. * The `-262143..=262142` year guard is replaced by an explicit `i32::try_from` check on the computed epoch day, so years whose epoch day overflows an `Int` fail exactly where Spark fails. The chrono representability limit is applied afterwards. Years that Spark accepts but chrono cannot represent (roughly `262143` to `5881580` and their negative counterparts) still return `NULL` in every eval mode. Raising for those would diverge from Spark in the other direction, and Comet's downstream date kernels cannot handle those values, so that limitation is preserved and now documented in the code. Behaviour that was already correct is unchanged: non-ANSI mode returns `NULL`, `try_cast` returns `NULL` under ANSI, and structurally malformed strings continue to throw. The `cast(string as boolean)` exception-type discrepancy also mentioned in #5012 is not addressed here and remains open. ## How are these changes tested? * `date_parser_test` in `native/spark-expr/src/conversion_funcs/string.rs` now asserts an error under `EvalMode::Ansi` for invalid calendar dates in both the fast-path and general-parser shapes, and for years whose epoch day overflows `i32`. The chrono-limit cases keep asserting `Ok(None)` in all three eval modes. * `CometCastSuite` "cast StringType to DateType" gains ten invalid calendar dates in its `invalidDates` list. That test compares against Spark in both non-ANSI mode (values) and ANSI mode (exceptions), so it covers both directions. The existing fuzz generator uses a `0123456789/` plus whitespace alphabet and so never produced a hyphenated date shape, which is why this went unnoticed. * New SQL file test `spark/src/test/resources/sql-tests/expressions/cast/cast_string_to_date_ansi.sql` covering the literal casts, `to_date`, the column-backed cast, and the filter case from the issue, plus non-error sentinel queries over valid input so a silent fallback to Spark cannot let the `expect_error` cases pass vacuously. Verified that this fixture fails on the unpatched native library and passes with the fix. * Full `CometCastSuite` (162 passed), full `CometSqlFileTestSuite` (419 passed), `CometTemporalExpressionSuite` (32 passed), and the full `datafusion-comet-spark-expr` Rust test suite (511 passed) are green. -- 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]
