[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #5960: fix: support `date - timestamp` and `timestamp - date`

["alamb (via GitHub)" <gi...@apache.org>]

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


##########
datafusion/core/tests/sqllogictests/test_files/timestamps.slt:
##########
@@ -660,6 +660,19 @@ statement error DataFusion error: Error during planning: Timestamp\(Nanosecond,
 SELECT ts1 + ts2
 FROM foo;
 
+# Timestamp - Timestamp
+query ?
+SELECT '2000-01-01T00:00:00'::timestamp - '2000-01-01T00:00:00'::timestamp;
+----
+0 years 0 mons 0 days 0 hours 0 mins 0.000000000 secs
+
+# large timestamp - small timestamp
+# Todo: Does it need to be fixed for negative values?.

Review Comment:
   I think negative values is correct and a valid interval. The intervals are signed according to the arrow spec:
   
   https://github.com/apache/arrow/blob/776ea3f4f3d665f38fc370d0f284760eafa7de45/format/Schema.fbs#L363-L384



##########
datafusion/expr/src/type_coercion/binary.rs:
##########
@@ -754,51 +751,6 @@ fn temporal_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataTyp
     }
 }
 
-/// Coercion rule for numerical types: The type that both lhs and rhs

Review Comment:
   it is always nice to see a bug fixed by deleting code. Thank you @jackwener  👏 



-- 
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