(calcite) branch main updated: [CALCITE-6211] SUBSTRING with Integer.MIN_VALUE as a second parameter raise unexpected exception

[mbudiu]

This is an automated email from the ASF dual-hosted git repository.

mbudiu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/main by this push:
     new 91a8ab8c06 [CALCITE-6211] SUBSTRING with Integer.MIN_VALUE as a second 
parameter raise unexpected exception
91a8ab8c06 is described below

commit 91a8ab8c061f80115ce64fb5be62e02937a0703a
Author: zstan <zs...@apache.com>
AuthorDate: Thu Jan 18 15:38:54 2024 +0300

    [CALCITE-6211] SUBSTRING with Integer.MIN_VALUE as a second parameter raise 
unexpected exception
---
 .../org/apache/calcite/runtime/SqlFunctions.java   |  8 +++----
 .../org/apache/calcite/test/SqlOperatorTest.java   | 26 ++++++++++++++++++++++
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java 
b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
index abc2b7e3cf..99bfcfb281 100644
--- a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
+++ b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
@@ -977,13 +977,13 @@ public class SqlFunctions {
 
   /** SQL SUBSTRING(string FROM ...) function. */
   public static String substring(String c, int s) {
-    final int s0 = s - 1;
-    if (s0 <= 0) {
+    if (s <= 1) {
       return c;
     }
     if (s > c.length()) {
       return "";
     }
+    final int s0 = s - 1;
     return c.substring(s0);
   }
 
@@ -1038,13 +1038,13 @@ public class SqlFunctions {
 
   /** SQL SUBSTRING(binary FROM ...) function for binary. */
   public static ByteString substring(ByteString c, int s) {
-    final int s0 = s - 1;
-    if (s0 <= 0) {
+    if (s <= 1) {
       return c;
     }
     if (s > c.length()) {
       return ByteString.EMPTY;
     }
+    final int s0 = s - 1;
     return c.substring(s0);
   }
 
diff --git a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java 
b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
index 24bf994591..eff34aff58 100644
--- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
+++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -9143,6 +9143,22 @@ public class SqlOperatorTest {
     final SqlOperatorFixture f = fixture();
     checkSubstringFunction(f);
     checkSubstringFunction(f.withConformance(SqlConformanceEnum.BIG_QUERY));
+    checkSubstringFunctionOverflow(f);
+    
checkSubstringFunctionOverflow(f.withConformance(SqlConformanceEnum.BIG_QUERY));
+  }
+
+  /**
+   * Test case for <a 
href="https://issues.apache.org/jira/browse/CALCITE-6211";>
+   * SUBSTRING with Integer.MIN_VALUE as a second parameter raise unexpected 
exception</a>. */
+  private static void checkSubstringFunctionOverflow(SqlOperatorFixture f) {
+    f.setFor(SqlStdOperatorTable.SUBSTRING);
+    f.checkScalar(
+        String.format(Locale.ROOT, "{fn SUBSTRING('abcdef', %d)}", 
Integer.MIN_VALUE),
+        "abcdef", "VARCHAR(6) NOT NULL");
+
+    f.checkScalar(
+        String.format(Locale.ROOT, "{fn SUBSTRING('abcdef', CAST(%d AS 
BIGINT))}",
+            Integer.MIN_VALUE), "abcdef", "VARCHAR(6) NOT NULL");
   }
 
   private static void checkSubstringFunction(SqlOperatorFixture f) {
@@ -9168,6 +9184,9 @@ public class SqlOperatorTest {
       f.checkFails("substring(x'aabbcc' from 1 for -1)",
           "Substring error: negative substring length not allowed",
           true);
+      f.checkFails("{fn SUBSTRING('abc', 2, -1)}",
+          "Substring error: negative substring length not allowed",
+          true);
     }
 
     if (Bug.FRG296_FIXED) {
@@ -9318,6 +9337,12 @@ public class SqlOperatorTest {
       case BIG_QUERY:
       case MYSQL:
       case ORACLE:
+        // BIG_QUERY has different implementation, check SubstrConvertlet
+        if (library == SqlLibrary.BIG_QUERY) {
+          assertReturns("abc", Integer.MIN_VALUE, "abc");
+        } else {
+          assertReturns("abc", Integer.MIN_VALUE, "");
+        }
         assertReturns("abc", -2, "bc");
         assertReturns("abc", -1, "c");
         assertReturns("abc", -2, 1, "b");
@@ -9333,6 +9358,7 @@ public class SqlOperatorTest {
         assertReturns("abc", -1, 4, "c");
         break;
       case POSTGRESQL:
+        assertReturns("abc", Integer.MIN_VALUE, "abc");
         assertReturns("abc", -2, "abc");
         assertReturns("abc", -1, "abc");
         assertReturns("abc", -2, 1, "");