tanclary commented on code in PR #3608:
URL: https://github.com/apache/calcite/pull/3608#discussion_r1442016985
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -12230,6 +12235,8 @@ void testTimestampDiff(boolean coercionEnabled) {
f.checkScalar("time_trunc(time '12:34:56', hour)",
"12:00:00", "TIME(0) NOT NULL");
f.checkNull("time_trunc(cast(null as time), second)");
+ f.checkNull("time_trunc(cast(null as time), minute)");
Review Comment:
I don't really see how these tests help us. We already have a `checkNull`
test for `second`, the value of the 2nd operand isn't going to influence the
behavior (as long as it is the same type).
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -12292,6 +12299,18 @@ void testTimestampDiff(boolean coercionEnabled) {
+ "'TIMESTAMP_TRUNC\\(<TIME\\(0\\)>, <INTERVAL HOUR>\\)'\\. "
+ "Supported form\\(s\\): 'TIMESTAMP_TRUNC\\(<TIMESTAMP>,
<DATETIME_INTERVAL>\\)'",
false);
+ f.checkNull("timestamp_trunc(CAST(NULL AS TIMESTAMP), second)");
Review Comment:
Same comment as above. Maybe one or two of these is good but an exhaustive
list doesn't add much value other than making the test methods longer than
necessary.
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -12348,6 +12367,18 @@ void testTimestampDiff(boolean coercionEnabled) {
+ "'DATETIME_TRUNC\\(<TIME\\(0\\)>, <INTERVAL HOUR>\\)'\\. "
+ "Supported form\\(s\\): 'DATETIME_TRUNC\\(<TIMESTAMP>,
<DATETIME_INTERVAL>\\)'",
false);
+ f.checkNull("datetime_trunc(CAST(NULL AS TIMESTAMP), second)");
Review Comment:
Are there tests for when the second operand is null? If not, that would be a
worthwhile test to add. Same comment for other methods.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
