feat: Enable typed strings expressions for VALUES clause #3018
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3018 +/- ##
==========================================
- Coverage 85.84% 85.84% -0.01%
==========================================
Files 283 283
Lines 51658 51665 +7
==========================================
+ Hits 44346 44352 +6
- Misses 7312 7313 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
There was a problem hiding this comment.
Looks great @stuartcarnie . Thanks!
I also tested in the datafusion-cli to make sure that nothing extra in the physical plan was needed and it all looked good 👍
DataFusion CLI v10.0.0
❯ SELECT col1, col2 FROM (VALUES (TIMESTAMP '2021-06-10 17:01:00Z', DATE '2004-04-09')) as t (col1, col2)
;
+---------------------+------------+
| col1 | col2 |
+---------------------+------------+
| 2021-06-10 17:01:00 | 2004-04-09 |
+---------------------+------------+
1 row in set. Query took 0.053 seconds.
datafusion/sql/src/planner.rs
Outdated
| SQLExpr::TypedString { | ||
| ref data_type, | ||
| ref value, | ||
| } => Ok(Expr::Cast { | ||
| expr: Box::new(lit(&**value)), | ||
| data_type: convert_data_type(data_type)?, | ||
| }), |
There was a problem hiding this comment.
In case you wanted to make this work with somewhat fewer * and & you can do something like:
| SQLExpr::TypedString { | |
| ref data_type, | |
| ref value, | |
| } => Ok(Expr::Cast { | |
| expr: Box::new(lit(&**value)), | |
| data_type: convert_data_type(data_type)?, | |
| }), | |
| SQLExpr::TypedString { | |
| data_type, | |
| value, | |
| } => Ok(Expr::Cast { | |
| expr: Box::new(lit(value)), | |
| data_type: convert_data_type(&data_type)?, | |
| }), |
There was a problem hiding this comment.
Cool – I'll make that change as it is much cleaner! I copied this pattern from another part of the file:
so I've updated that too.
|
Not that it matters, but I found it quite neat that these casts are evaluated at plan time (as in after simplify_expressions we see that timestamps have been resolved to ❯ explain verbose SELECT col1, col2 FROM (VALUES (TIMESTAMP '2021-06-10 17:01:00Z', DATE '2004-04-09')) as t (col1, col2);
+-------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| plan_type | plan |
+-------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| initial_logical_plan | Projection: #t.col1, #t.col2 |
| | Projection: #t.column1 AS col1, #t.column2 AS col2, alias=t |
| | Projection: #column1, #column2, alias=t |
| | Values: (CAST(Utf8("2021-06-10 17:01:00Z") AS Timestamp(Nanosecond, None)), CAST(Utf8("2004-04-09") AS Date32)) |
| logical_plan after simplify_expressions | Projection: #t.col1, #t.col2 |
| | Projection: #t.column1 AS col1, #t.column2 AS col2, alias=t |
| | Projection: #column1, #column2, alias=t |
| | Values: (TimestampNanosecond(1623344460000000000, None) AS CAST(Utf8("2021-06-10 17:01:00Z") AS Timestamp(Nanosecond, None)), Date32("12517") AS CAST(Utf8("2004-04-09") AS Date32)) | |
|
Thanks for the review, @alamb – I made those changes you suggested, thanks! TIL: |
|
Thanks again @stuartcarnie |
|
Benchmark runs are scheduled for baseline = 9a5f17e and contender = c57fc86. c57fc86 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #3017.
What changes are included in this PR?
Support for transforming typed string literal expressions to
CASTexpressions in aVALUESclause.Are there any user-facing changes?
This extends the supported expressions in a
VALUESlist clause.