xiedeyantu commented on code in PR #4512:
URL: https://github.com/apache/calcite/pull/4512#discussion_r2312731020
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlTimestampDiffFunction.java:
##########
@@ -112,4 +115,51 @@ private static RelDataType
inferReturnType2(SqlOperatorBinding opBinding) {
validator.validateTimeFrame(call.operand(0));
}
}
+
+ @Override public boolean checkOperandTypes(SqlCallBinding callBinding,
boolean throwOnFailure) {
+ // Coerce type first.
+ SqlValidator validator = callBinding.getValidator();
+ SqlCall call = callBinding.getCall();
+ int leftIndex = 1;
+ int rightIndex = 2;
+ if (call.operand(2) instanceof SqlIntervalQualifier) {
+ leftIndex = 0;
+ rightIndex = 1;
+ }
+
+ RelDataType left = callBinding.getOperandType(leftIndex);
+ RelDataType right = callBinding.getOperandType(rightIndex);
+
+ if (left.getSqlTypeName() == right.getSqlTypeName()) {
Review Comment:
I wonder if lines 133-138 are necessary, because if the following if
judgment does not meet the requirements, super.checkOperandTypes() will still
be used in line 163.
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -14497,11 +14497,11 @@ void testTimestampDiff(boolean coercionEnabled) {
MONTH_VARIANTS.forEach(s ->
f.checkScalar("timestampdiff(" + s + ", "
+ "time '12:42:25', date '2016-06-14')",
- "-1502389", "INTEGER NOT NULL"));
+ "557", "INTEGER NOT NULL"));
Review Comment:
I verified it with MySQL, and while the implementation may be slightly
different, I personally agree with the fix. Here are the MySQL verification
results: [mysql_check](https://onecompiler.com/mysql/43vd5rg2g).
--
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]
