zabetak commented on code in PR #3364:
URL: https://github.com/apache/calcite/pull/3364#discussion_r1304060467


##########
core/src/main/java/org/apache/calcite/util/Bug.java:
##########
@@ -208,6 +208,12 @@ public abstract class Bug {
    * MILLISECOND and MICROSECOND units in INTERVAL literal</a> is fixed. */
   public static final boolean CALCITE_5422_FIXED = false;
 
+  /** Whether
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-5678";>[CALCITE-5678]
+   * Calcite should reject date literals not satisfying Gregorian calendar,
+   * per SQL standard</a> is fixed. */
+  public static final boolean CALCITE_5678_FIXED = false;

Review Comment:
   This is confusing since CALCITE-5678 is already fixed although not yet 
released. It may be better to replace this with a ticket pointing to the 
Avatica 1.24.0 upgrade.



##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -1253,15 +1254,28 @@ void testCastStringToDateTime(CastType castType, 
SqlOperatorFixture f) {
       f.checkScalar("cast('1945-02-24 12:42:25.34' as TIMESTAMP(2))",
           "1945-02-24 12:42:25.34", "TIMESTAMP(2) NOT NULL");
     }
+    if (Bug.CALCITE_5678_FIXED) {
+      if (castType == CastType.CAST) {
+        f.checkFails("cast('1945-2-2 12:2:5' as TIMESTAMP)",
+            "Invalid DATE value, '1945-2-2 12:2:5'", true);
+        f.checkFails("cast('1241241' as TIMESTAMP)",
+            "Invalid DATE value, '1241241'", true);
+        f.checkFails("cast('1945-20-24 12:42:25.34' as TIMESTAMP)",
+            "Invalid DATE value, '1945-20-24 12:42:25.34'", true);
+        f.checkFails("cast('1945-01-24 25:42:25.34' as TIMESTAMP)",
+            "Value of HOUR field is out of range in '1945-01-24 25:42:25.34'", 
true);
+        f.checkFails("cast('1945-1-24 12:23:34.454' as TIMESTAMP)",
+            "Invalid DATE value, '1945-1-24 12:23:34.454'", true);
+      } else {
+        // test cases for 'SAFE_CAST' and 'TRY_CAST'
+        f.checkNull("cast('1945-2-2 12:2:5' as TIMESTAMP)");
+        f.checkNull("cast('1241241' as TIMESTAMP)");
+        f.checkNull("cast('1945-20-24 12:42:25.34' as TIMESTAMP)");
+        f.checkNull("cast('1945-01-24 25:42:25.34' as TIMESTAMP)");
+        f.checkNull("cast('1945-1-24 12:23:34.454' as TIMESTAMP)");
+      }

Review Comment:
   Would it be possible to avoid this if-else block by using `f.checkCastFails` 
or something similar that takes the `castType` as parameter?



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

Reply via email to