feat: add try_to_time, unify to_time under SparkTime#2094
Open
davidlghellin wants to merge 3 commits into
Open
feat: add try_to_time, unify to_time under SparkTime#2094davidlghellin wants to merge 3 commits into
try_to_time, unify to_time under SparkTime#2094davidlghellin wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces Spark 4.1-style try_to_time and re-routes to_time planning through Sail’s SparkTime UDF implementation, aligning the planner with Sail’s TIME parsing/casting behavior and adding BDD coverage for both strict and safe variants.
Changes:
- Added new BDD feature coverage for
try_to_time(NULL-on-failure) andto_time(error-on-failure). - Updated the datetime scalar function planner to implement
to_time/try_to_timeviaSparkTimeand to registertry_to_timeas a built-in function. - Refactored
SparkTimeUDF to support both default-format parsing and per-row format parsing, plus non-string casting withsafecasting behavior fortry_to_time.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| python/pysail/tests/spark/function/features/try_to_time.feature | Adds end-to-end SQL BDD coverage for try_to_time across valid inputs, invalid inputs, and per-row formats. |
| python/pysail/tests/spark/function/features/to_time.feature | Adds end-to-end SQL BDD coverage for strict to_time behavior (success, error cases, NULL propagation). |
| crates/sail-plan/src/function/scalar/datetime.rs | Implements planner routing for to_time/try_to_time through SparkTime and registers try_to_time. |
| crates/sail-function/src/scalar/datetime/spark_time.rs | Updates the SparkTime UDF implementation to parse/cast TIME with strict vs safe behavior and optional formats. |
Comment on lines
+405
to
+412
| } else if input.arguments.len() == 2 { | ||
| let (expr, format) = input.arguments.two()?; | ||
| let expr = cast(expr, DataType::Utf8); | ||
| let format = to_chrono_fmt(format); | ||
| Ok(udf.call(vec![expr, format])) | ||
| } else { | ||
| Err(PlanError::invalid("to_time requires 1 or 2 arguments")) | ||
| } |
Comment on lines
+111
to
+114
| "to_time: value array length ({}) does not match format array length ({})", | ||
| value_arr.len(), | ||
| format_arr.len() | ||
| ); |
Comment on lines
+153
to
+155
| "to_time format argument must be a string, got {}", | ||
| fmt.data_type() | ||
| ); |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #2094 +/- ##
==========================================
- Coverage 77.28% 74.66% -2.62%
==========================================
Files 996 1004 +8
Lines 159613 168185 +8572
==========================================
+ Hits 123353 125576 +2223
- Misses 36260 42609 +6349
*This pull request uses carry forward flags. Click here to find out more.
... and 88 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Spark 3.5.7 Test ReportCommit Information
Test Summary
Test DetailsError CountsPassed Tests Diff(empty) Failed Tests |
Spark 4.1.1 Test ReportCommit Information
Test Summary
Test DetailsError CountsPassed Tests Diff--- before.txt 2026-06-18 16:09:09.362138605 +0000
+++ after.txt 2026-06-18 16:09:09.774143675 +0000
@@ -338,0 +339 @@
+pyspark/sql/functions/builtin.py::pyspark.sql.functions.builtin.map_entries
@@ -478,0 +480 @@
+pyspark/sql/functions/builtin.py::pyspark.sql.functions.builtin.to_time
@@ -499,0 +502 @@
+pyspark/sql/functions/builtin.py::pyspark.sql.functions.builtin.try_to_time
@@ -1470,0 +1474 @@
+pyspark/sql/tests/connect/test_connect_function.py::SparkConnectFunctionTests::test_map_collection_functions
@@ -1778,0 +1783 @@
+pyspark/sql/tests/connect/test_parity_functions.py::FunctionsParityTests::test_to_time
@@ -1785,0 +1791 @@
+pyspark/sql/tests/connect/test_parity_functions.py::FunctionsParityTests::test_try_to_timeFailed Tests(truncated) |
Ibis Test ReportCommit Information
Test Summary
Test DetailsError CountsPassed Tests Diff--- before.txt 2026-06-18 16:07:50.556066156 +0000
+++ after.txt 2026-06-18 16:07:50.763069970 +0000
@@ -502,0 +503 @@
+ibis/backends/tests/test_export.py::test_table_to_csv[pyspark]Failed Tests |
Gold Data ReportNotes
Commit Information
Summary
DetailsGold Data Metrics
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.