From b8b08e56be1c7a45c946d35fc1f86b328759e34d Mon Sep 17 00:00:00 2001 From: kid Date: Sat, 18 Jul 2026 10:05:13 +0800 Subject: [PATCH 1/2] test: promote `try_to_date`/`try_to_timestamp` SQL tests to native coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Investigation of #4556 showed the reported fallback does not exist on current main: in non-ANSI mode Spark's ReplaceExpressions expands try_to_date/try_to_timestamp to the exact same tree as to_date/ to_timestamp (Cast(..., LEGACY) or GetTimestamp(failOnError=false)), which Comet already accelerates. The 15 spark_answer_only markers were conservative annotations from #4555 (that mode only compares answers without asserting native execution), not measured fallbacks. Flip the 15 try_to_date/try_to_timestamp queries to default query mode (native coverage + answer check), keeping the 3 try_make_timestamp markers which belong to #4554. Verified on Spark 4.1: CometSqlFileTestSuite try_datetime passes with all queries running fully native. Also update the expression support matrix (try_to_date/try_to_timestamp 🔜 -> ✅) and add audit entries. Closes #4556 --- .../expression-audits/datetime_funcs.md | 8 +++++ docs/source/user-guide/latest/expressions.md | 4 +-- .../expressions/datetime/try_datetime.sql | 32 +++++++++---------- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/docs/source/contributor-guide/expression-audits/datetime_funcs.md b/docs/source/contributor-guide/expression-audits/datetime_funcs.md index b20fad5899..aad1945f5e 100644 --- a/docs/source/contributor-guide/expression-audits/datetime_funcs.md +++ b/docs/source/contributor-guide/expression-audits/datetime_funcs.md @@ -87,4 +87,12 @@ - Rewrites to `MakeTimestamp(failOnError = false)` and runs through the codegen dispatcher (`CometMakeTimestamp`), so invalid inputs return NULL to match Spark. +## try_to_date + +- Spark 4.1+. Rewrites to `Cast(..., EvalMode.LEGACY)` (no format, native) or `Cast(GetTimestamp(..., failOnError = false))` (with format, via the codegen dispatcher) before Comet sees the plan. In non-ANSI mode the rewritten tree is identical to `to_date`; invalid inputs return NULL to match Spark. + +## try_to_timestamp + +- Rewrites to `Cast(..., EvalMode.LEGACY)` (no format, native) or `GetTimestamp(..., failOnError = false)` (with format, via the codegen dispatcher) before Comet sees the plan. In non-ANSI mode the rewritten tree is identical to `to_timestamp`; invalid inputs return NULL to match Spark. + [Spark Expression Support]: ../../user-guide/latest/expressions.md diff --git a/docs/source/user-guide/latest/expressions.md b/docs/source/user-guide/latest/expressions.md index f45fa21444..ae49f27f87 100644 --- a/docs/source/user-guide/latest/expressions.md +++ b/docs/source/user-guide/latest/expressions.md @@ -294,9 +294,9 @@ The type-name conversion functions (`bigint`, `binary`, `boolean`, `date`, `deci | `trunc` | ✅ | | | `try_make_interval` | 🔜 | Produces legacy CalendarInterval; tracked by [#4540](https://github.com/apache/datafusion-comet/issues/4540) | | `try_make_timestamp` | ✅ | | -| `try_to_date` | 🔜 | Rewrites to `Cast`/`GetTimestamp` but currently falls back; tracked by [#4556](https://github.com/apache/datafusion-comet/issues/4556) | +| `try_to_date` | ✅ | Rewrites to `Cast`/`GetTimestamp` before Comet sees the plan; same support as `to_date` | | `try_to_time` | 🔜 | Spark 4.1 TIME type; tracked by [#4288](https://github.com/apache/datafusion-comet/issues/4288) | -| `try_to_timestamp` | 🔜 | Rewrites to `Cast`/`GetTimestamp` but currently falls back; tracked by [#4556](https://github.com/apache/datafusion-comet/issues/4556) | +| `try_to_timestamp` | ✅ | Rewrites to `Cast`/`GetTimestamp` before Comet sees the plan; same support as `to_timestamp` | | `unix_date` | ✅ | | | `unix_micros` | ✅ | | | `unix_millis` | ✅ | | diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/try_datetime.sql b/spark/src/test/resources/sql-tests/expressions/datetime/try_datetime.sql index 5ed4e445fd..3eb071f4c6 100644 --- a/spark/src/test/resources/sql-tests/expressions/datetime/try_datetime.sql +++ b/spark/src/test/resources/sql-tests/expressions/datetime/try_datetime.sql @@ -43,32 +43,32 @@ INSERT INTO test_try_datetime_str VALUES -- try_to_date: column argument, no format -- ------------------------------------------------------------ --- column argument, default format, falls back to Spark -query spark_answer_only +-- column argument, default format +query SELECT s, try_to_date(s) FROM test_try_datetime_str -- literal valid date -query spark_answer_only +query SELECT try_to_date('2024-06-15') -- literal invalid date -> NULL -query spark_answer_only +query SELECT try_to_date('2020-13-99') -- literal NULL -> NULL -query spark_answer_only +query SELECT try_to_date(NULL) -- column argument with explicit format -query spark_answer_only +query SELECT s, try_to_date(s, 'yyyy-MM-dd') FROM test_try_datetime_str -- literal with explicit format -query spark_answer_only +query SELECT try_to_date('15/06/2024', 'dd/MM/yyyy') -- literal invalid with format -> NULL -query spark_answer_only +query SELECT try_to_date('99/99/9999', 'dd/MM/yyyy') -- ------------------------------------------------------------ @@ -93,35 +93,35 @@ INSERT INTO test_try_datetime_ts VALUES -- ------------------------------------------------------------ -- column argument, default format -query spark_answer_only +query SELECT s, try_to_timestamp(s) FROM test_try_datetime_ts -- literal valid timestamp -query spark_answer_only +query SELECT try_to_timestamp('2024-06-15 10:30:45') -- literal invalid timestamp -> NULL -query spark_answer_only +query SELECT try_to_timestamp('2020-13-99 25:61:61') -- literal NULL -> NULL -query spark_answer_only +query SELECT try_to_timestamp(NULL) -- column argument with explicit format -query spark_answer_only +query SELECT s, try_to_timestamp(s, 'yyyy-MM-dd HH:mm:ss') FROM test_try_datetime_ts -- literal with explicit format -query spark_answer_only +query SELECT try_to_timestamp('15/06/2024 10:30:45', 'dd/MM/yyyy HH:mm:ss') -- literal invalid with format -> NULL -query spark_answer_only +query SELECT try_to_timestamp('99/99/9999 99:99:99', 'dd/MM/yyyy HH:mm:ss') -- date-only string with date format -query spark_answer_only +query SELECT try_to_timestamp('2024-06-15', 'yyyy-MM-dd') -- ------------------------------------------------------------ From d1d867d3c79357628d4a5f5852a2aea13d55becc Mon Sep 17 00:00:00 2001 From: kid Date: Sat, 18 Jul 2026 14:23:12 +0800 Subject: [PATCH 2/2] test: cover try timestamp parser policies natively --- .../spark_configs_support.md | 44 ++++++++++++------- .../expressions/datetime/try_datetime.sql | 2 +- .../try_to_timestamp_time_parser_policy.sql | 12 ++--- ...timestamp_time_parser_policy_corrected.sql | 4 +- ...timestamp_time_parser_policy_exception.sql | 12 ++++- ...to_timestamp_time_parser_policy_legacy.sql | 4 +- 6 files changed, 49 insertions(+), 29 deletions(-) diff --git a/docs/source/contributor-guide/spark_configs_support.md b/docs/source/contributor-guide/spark_configs_support.md index fe1da3cd23..31c48adc83 100644 --- a/docs/source/contributor-guide/spark_configs_support.md +++ b/docs/source/contributor-guide/spark_configs_support.md @@ -39,10 +39,10 @@ The status column uses these values: - `spark.sql.legacy.timeParserPolicy` - Default: `EXCEPTION` - - Status: Falls back (see notes) - - Affected expressions: `date_format`, `from_unixtime`, `unix_timestamp`, `to_unix_timestamp`, `to_timestamp`, `to_timestamp_ntz`, `to_date`, `try_to_timestamp` (Spark 4+) - - Spark versions checked: 3.4.3, 3.5.8, 4.0.1 - - Date: 2026-05-02 + - Status: Partial (see notes) + - Affected expressions: `date_format`, `from_unixtime`, `unix_timestamp`, `to_unix_timestamp`, `to_timestamp`, `to_timestamp_ntz`, `to_date`, `try_to_date` (Spark 4.1+), `try_to_timestamp` (Spark 3.4+) + - Spark versions checked: 3.4.3, 3.5.8, 4.0.1, 4.1.2 + - Date: 2026-07-18 ## Audit Notes @@ -76,27 +76,36 @@ across Spark 3.4, 3.5, 4.0, and 4.1. Three expression classes mix in `Cast` between strings and date / timestamp also reads the policy via the default formatters but is tested separately by `CometCastSuite` and is out of scope here. -**Comet status.** None of the listed expressions consult `legacyTimeParserPolicy` in -their Comet serde. The native implementations of `date_format`, `from_unixtime`, and -`unix_timestamp` use a fixed strftime-style mapping that does not vary with policy; -the remaining four (`to_unix_timestamp`, `to_timestamp`, `to_date`, -`try_to_timestamp`) have no native implementation and fall back to Spark. Today this -works because: +**Comet status.** None of the native implementations consult +`legacyTimeParserPolicy` directly. Comet instead uses native implementations only for +policy-independent cases and routes policy-sensitive expressions through Spark's own +expression code via the Arrow-direct codegen dispatcher. In particular, +`to_timestamp`, `to_timestamp_ntz`, `to_date`, `try_to_date`, and +`try_to_timestamp` are rewritten by Spark to `Cast` or `GetTimestamp` before Comet +sees the plan. Supported casts run natively, while `GetTimestamp` runs through the +codegen dispatcher and therefore preserves Spark's selected parser policy inside the +Comet pipeline. Other affected expressions use a mixture of native, codegen-dispatch, +and fallback paths: - `date_format` is `Compatible` only for a small whitelist of formats under UTC; the whitelisted formats happen to produce identical output under all three policies. -- `from_unixtime` is marked `Incompatible` and falls back unless - `spark.comet.expression.FromUnixTime.allowIncompatible=true` is set. + Other formats route through the codegen dispatcher. +- `from_unixtime` uses the native implementation for its default format when + `spark.comet.expression.FromUnixTime.allowIncompatible=true` is set; otherwise + it routes through the codegen dispatcher. - `unix_timestamp()` does not call the formatter at all; the string-input overload falls back. +- `to_unix_timestamp` routes through the codegen dispatcher. If a Comet contributor adds native string-format parsing or extends the date_format whitelist, this audit should be revisited and the policy must be honored explicitly. **Test coverage.** `spark/src/test/resources/sql-tests/expressions/datetime/`: -- One ConfigMatrix file per expression covering convergent inputs under - `LEGACY,CORRECTED,EXCEPTION` (`*_time_parser_policy.sql`). +- One ConfigMatrix file per originally audited expression covering convergent + inputs under `LEGACY,CORRECTED,EXCEPTION` (`*_time_parser_policy.sql`). + `try_to_date`, added in Spark 4.1, has native-coverage tests in + `try_datetime.sql` but has not yet been added to the parser-policy matrix. - Per-policy files locking in divergent behavior: - `_legacy.sql` -- lenient inputs (single-digit fields, out-of-range values, trailing characters) and legacy-only pattern tokens (`'aaaa'`). @@ -107,6 +116,7 @@ whitelist, this audit should be revisited and the policy must be honored explici `INCONSISTENT_BEHAVIOR_CROSS_VERSION.PARSE_DATETIME_BY_NEW_PARSER` at parse time. **Findings.** All 42 generated test cases pass on Spark 3.4.3, 3.5.8, and 4.0.1. No -Comet bugs were uncovered by the audit. The tests use `query spark_answer_only` so -that result-correctness is enforced regardless of whether Comet runs the expression -natively or falls back. +Comet bugs were uncovered by the original audit. The `try_to_timestamp` policy tests +use the default `query` mode to enforce both result correctness and execution within +the Comet pipeline. Tests for expressions with known fallback paths continue to use +`query spark_answer_only`. diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/try_datetime.sql b/spark/src/test/resources/sql-tests/expressions/datetime/try_datetime.sql index 3eb071f4c6..7305a1d2ca 100644 --- a/spark/src/test/resources/sql-tests/expressions/datetime/try_datetime.sql +++ b/spark/src/test/resources/sql-tests/expressions/datetime/try_datetime.sql @@ -18,7 +18,7 @@ -- Tests for try_to_date, try_to_timestamp, and try_make_timestamp. -- These RuntimeReplaceable functions rewrite to Cast/GetTimestamp/MakeTimestamp(failOnError=false) -- and return NULL on invalid input instead of throwing. --- try_to_date and try_make_timestamp are only available in Spark 4.1+, so gate the file. +-- try_to_date is only available in Spark 4.1+, so gate the file. -- MinSparkVersion: 4.1 -- Config: spark.sql.session.timeZone=UTC diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy.sql b/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy.sql index 9c67d0466f..99725e8b6f 100644 --- a/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy.sql +++ b/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy.sql @@ -16,7 +16,7 @@ -- under the License. -- Convergent try_to_timestamp(string, format) behavior across all three policies. --- MinSparkVersion: 4.0 +-- MinSparkVersion: 3.4 -- ConfigMatrix: spark.sql.legacy.timeParserPolicy=LEGACY,CORRECTED,EXCEPTION -- Config: spark.sql.session.timeZone=UTC @@ -26,10 +26,10 @@ CREATE TABLE test_try_to_ts_policy(s string) USING parquet statement INSERT INTO test_try_to_ts_policy VALUES ('2024-06-15 10:30:45'), ('1970-01-01 00:00:00'), (NULL), ('') -query spark_answer_only +query SELECT try_to_timestamp(s, 'yyyy-MM-dd HH:mm:ss') FROM test_try_to_ts_policy -query spark_answer_only +query SELECT try_to_timestamp(s) FROM test_try_to_ts_policy statement @@ -38,11 +38,11 @@ CREATE TABLE test_try_to_ts_date_policy(s string) USING parquet statement INSERT INTO test_try_to_ts_date_policy VALUES ('2024-06-15'), ('1970-01-01'), (NULL) -query spark_answer_only +query SELECT try_to_timestamp(s, 'yyyy-MM-dd') FROM test_try_to_ts_date_policy -query spark_answer_only +query SELECT try_to_timestamp('2024-06-15', 'yyyy-MM-dd') -query spark_answer_only +query SELECT try_to_timestamp(NULL, 'yyyy-MM-dd') diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy_corrected.sql b/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy_corrected.sql index 1597bfcd11..ee2b97968b 100644 --- a/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy_corrected.sql +++ b/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy_corrected.sql @@ -17,7 +17,7 @@ -- try_to_timestamp() under CORRECTED timeParserPolicy. -- Strict java.time parsing returns null for inputs that legacy would accept. --- MinSparkVersion: 4.0 +-- MinSparkVersion: 3.4 -- Config: spark.sql.legacy.timeParserPolicy=CORRECTED -- Config: spark.sql.session.timeZone=UTC @@ -32,5 +32,5 @@ INSERT INTO test_try_to_ts_strict VALUES ('2024-01-01garbage'), ('2024') -query spark_answer_only +query SELECT s, try_to_timestamp(s, 'yyyy-MM-dd') FROM test_try_to_ts_strict ORDER BY s diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy_exception.sql b/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy_exception.sql index 5d1b91432e..ef4c4c464e 100644 --- a/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy_exception.sql +++ b/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy_exception.sql @@ -19,7 +19,7 @@ -- try_to_timestamp swallows DateTimeException/ParseException, but SparkUpgradeException -- raised by checkParsedDiff propagates through the catch -- so the EXCEPTION case still -- throws on legacy/new divergent inputs. --- MinSparkVersion: 4.0 +-- MinSparkVersion: 3.4 -- Config: spark.sql.legacy.timeParserPolicy=EXCEPTION -- Config: spark.sql.session.timeZone=UTC @@ -29,5 +29,15 @@ CREATE TABLE test_try_to_ts_exception(s string) USING parquet statement INSERT INTO test_try_to_ts_exception VALUES ('2024-1-1') +statement +CREATE TABLE test_try_to_ts_exception_valid(s string) USING parquet + +statement +INSERT INTO test_try_to_ts_exception_valid VALUES ('2024-01-01') + +-- A successful codegen-dispatch query guards against the error case passing via Spark fallback. +query +SELECT try_to_timestamp(s, 'yyyy-MM-dd') FROM test_try_to_ts_exception_valid + query expect_error(INCONSISTENT_BEHAVIOR_CROSS_VERSION) SELECT try_to_timestamp(s, 'yyyy-MM-dd') FROM test_try_to_ts_exception diff --git a/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy_legacy.sql b/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy_legacy.sql index a72c8ae0b4..cee5ed4689 100644 --- a/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy_legacy.sql +++ b/spark/src/test/resources/sql-tests/expressions/datetime/try_to_timestamp_time_parser_policy_legacy.sql @@ -17,7 +17,7 @@ -- try_to_timestamp() under LEGACY timeParserPolicy. -- Lenient SimpleDateFormat parsing accepts inputs that the new formatter rejects. --- MinSparkVersion: 4.0 +-- MinSparkVersion: 3.4 -- Config: spark.sql.legacy.timeParserPolicy=LEGACY -- Config: spark.sql.session.timeZone=UTC @@ -32,5 +32,5 @@ INSERT INTO test_try_to_ts_lenient VALUES ('2024-01-01garbage'), ('2024') -query spark_answer_only +query SELECT s, try_to_timestamp(s, 'yyyy-MM-dd') FROM test_try_to_ts_lenient ORDER BY s