You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/05/08 18:15:43 UTC

[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #6270: Support `interval '1 month' + date/timestamp`: Handle binary op interval in logical AST builder

alamb commented on code in PR #6270:
URL: https://github.com/apache/arrow-datafusion/pull/6270#discussion_r1187738619


##########
datafusion/core/tests/sqllogictests/test_files/type_coercion.slt:
##########
@@ -44,10 +44,10 @@ SELECT '2023-05-01 12:30:00'::timestamp - interval '1 month';
 
 # TODO: https://github.com/apache/arrow-datafusion/issues/6180

Review Comment:
   I wonder if we can remove the link to the issue as it now works



##########
datafusion/core/tests/sqllogictests/test_files/interval.slt:
##########
@@ -126,8 +126,89 @@ select interval '5' nanoseconds
 ----
 0 years 0 mons 0 days 0 hours 0 mins 0.000000005 secs
 
+# Interval with string literal addition

Review Comment:
   it is so beautiful 😍 



##########
datafusion/sql/src/expr/value.rs:
##########
@@ -191,6 +200,76 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
             SQLExpr::Value(
                 Value::SingleQuotedString(s) | Value::DoubleQuotedString(s),
             ) => s,
+            SQLExpr::BinaryOp { left, op, right } => {

Review Comment:
   Your PR description is beautiful -- I thin some part if it would be valuable to include in the comments as well for future readers:
   
   ```suggestion
               // Support expressions like `interval '1 month' + date/timestamp`. 
               // Such expressions are parsed like this by sqlparser-rs 
               // 
               // Interval
               // BinaryOp
               //   Value(StringLiteral)
               //   Cast
               //     Value(StringLiteral)
               // 
               // This code rewrites them to the following:
               //
               // BinaryOp
               //   Interval
               //     Value(StringLiteral)
               //   Cast
               //      Value(StringLiteral)
               SQLExpr::BinaryOp { left, op, right } => {
   ```



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org