This is an automated email from the ASF dual-hosted git repository.
mihaibudiu 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 f8d9315630 [CALCITE-7923] SUBSTRING index may silently overflow for
BINARY arguments
f8d9315630 is described below
commit f8d9315630557483439122803823af6593b2f769
Author: Mihai Budiu <[email protected]>
AuthorDate: Thu Nov 5 20:44:38 2026 -0800
[CALCITE-7923] SUBSTRING index may silently overflow for BINARY arguments
Signed-off-by: Mihai Budiu <[email protected]>
---
.../org/apache/calcite/runtime/SqlFunctions.java | 100 ++++++------
core/src/test/resources/sql/functions.iq | 177 +++++++++++++++++++++
2 files changed, 225 insertions(+), 52 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 58e68cdb24..8d98883735 100644
--- a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
+++ b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
@@ -1296,57 +1296,46 @@ public static String substring(String c, long s) {
return substring(c, clamp(s));
}
- /** SQL SUBSTRING(string FROM ... FOR ...) function. */
- public static String substring(String c, int s, int l) {
- int lc = c.length();
- long e = (long) s + (long) l;
+ /** Returns the 0-based begin index of {@code SUBSTRING(value FROM s FOR l)}
+ * for a value of length {@code lc}. */
+ private static int substringBegin(int lc, long s) {
+ return (int) Math.min(Math.max(s, 1) - 1, lc);
+ }
+
+ /** Returns the 0-based end index (exclusive) of
+ * {@code SUBSTRING(value FROM s FOR l)} for a value of length {@code lc}.
+ * It is never less than {@link #substringBegin}. */
+ private static int substringEnd(int lc, long s, long l) {
if (l < 0) {
throw RESOURCE.illegalNegativeSubstringLength().ex();
}
- // Prevent overflow in addition
- if (s > lc || e < 1L) {
- return "";
+ if (s < 1) {
+ // Remove the positions before 1; s <= 0 and l >= 0, so s + l cannot
overflow
+ l = Math.max(s + l, 1) - 1;
+ s = 1;
}
- final int s0 = Math.max(s - 1, 0);
- final long e0 = Math.min(e - 1, (long) lc);
- // We know that e0 cannot exceed Integer.MAX_VALUE, since it's smaller
than lc
- return c.substring(s0, (int) e0);
+ if (s > lc) {
+ return lc;
+ }
+ return (int) (s - 1 + Math.min(l, lc - s + 1));
}
- public static String substring(String c, int s, long l) {
- if (s < 0) {
- if (l > 0 && l + s > 0) {
- l += s;
- return substring(c, 0, l);
- }
- }
- int l0 = clamp(l);
- return SqlFunctions.substring(c, s, l0);
+ /** SQL SUBSTRING(string FROM ... FOR ...) function. */
+ public static String substring(String c, long s, long l) {
+ final int lc = c.length();
+ return c.substring(substringBegin(lc, s), substringEnd(lc, s, l));
}
- public static String substring(String c, long s, int l) {
- if (s < 0) {
- s += l;
- if (s > 0) {
- return substring(c, 0, s);
- } else {
- return "";
- }
- }
- int s0 = clamp(s);
- return SqlFunctions.substring(c, s0, l);
+ public static String substring(String c, int s, int l) {
+ return substring(c, (long) s, (long) l);
}
- public static String substring(String c, long s, long l) {
- if (s < 0) {
- if (l > 0) {
- l += s;
- return substring(c, 0, clamp(l));
- }
- }
- int s0 = clamp(s);
- int l0 = clamp(l);
- return SqlFunctions.substring(c, s0, l0);
+ public static String substring(String c, int s, long l) {
+ return substring(c, (long) s, l);
+ }
+
+ public static String substring(String c, long s, int l) {
+ return substring(c, s, (long) l);
}
/** SQL SUBSTRING(binary FROM ...) function for binary. */
@@ -1361,19 +1350,26 @@ public static ByteString substring(ByteString c, int s)
{
return c.substring(s0);
}
+ public static ByteString substring(ByteString c, long s) {
+ return substring(c, clamp(s));
+ }
+
/** SQL SUBSTRING(binary FROM ... FOR ...) function for binary. */
+ public static ByteString substring(ByteString c, long s, long l) {
+ final int lc = c.length();
+ return c.substring(substringBegin(lc, s), substringEnd(lc, s, l));
+ }
+
public static ByteString substring(ByteString c, int s, int l) {
- int lc = c.length();
- int e = s + l;
- if (l < 0) {
- throw RESOURCE.illegalNegativeSubstringLength().ex();
- }
- if (s > lc || e < 1) {
- return ByteString.EMPTY;
- }
- final int s0 = Math.max(s - 1, 0);
- final int e0 = Math.min(e - 1, lc);
- return c.substring(s0, e0);
+ return substring(c, (long) s, (long) l);
+ }
+
+ public static ByteString substring(ByteString c, int s, long l) {
+ return substring(c, (long) s, l);
+ }
+
+ public static ByteString substring(ByteString c, long s, int l) {
+ return substring(c, s, (long) l);
}
/** SQL FORMAT_NUMBER(value, decimalOrFormat) function. */
diff --git a/core/src/test/resources/sql/functions.iq
b/core/src/test/resources/sql/functions.iq
index 1d6ea00768..2c0641534a 100644
--- a/core/src/test/resources/sql/functions.iq
+++ b/core/src/test/resources/sql/functions.iq
@@ -1171,4 +1171,181 @@ select SUBSTRING('text' FROM NULL);
!ok
+# [CALCITE-7923] SUBSTRING index may silently overflow for BINARY arguments
+# Binary SUBSTRING computed start + length in int arithmetic and overflowed.
+# Validated on Postgres.
+!use post
+SELECT SUBSTRING(x'123456' FROM 2 FOR 2147483647) AS c;
++------+
+| C |
++------+
+| 3456 |
++------+
+(1 row)
+
+!ok
+
+SELECT SUBSTRING(b FROM 2 FOR 2147483647) AS c
+FROM (VALUES (x'123456')) AS t(b);
++------+
+| C |
++------+
+| 3456 |
++------+
+(1 row)
+
+!ok
+
+# start + length = Integer.MAX_VALUE does not overflow.
+SELECT SUBSTRING(x'123456' FROM 2 FOR 2147483645) AS c;
++------+
+| C |
++------+
+| 3456 |
++------+
+(1 row)
+
+!ok
+
+# Same query with VARCHAR, fixed in [CALCITE-5810].
+SELECT SUBSTRING('abc' FROM 2 FOR 2147483647) AS c;
++----+
+| C |
++----+
+| bc |
++----+
+(1 row)
+
+!ok
+
+# PostgreSQL SUBSTR on binary had the same overflow.
+!use post-postgresql
+SELECT SUBSTR(x'123456', 2, 2147483647) AS c;
++------+
+| C |
++------+
+| 3456 |
++------+
+(1 row)
+
+!ok
+
+# SUBSTRING with a BIGINT start or length follows the INTEGER rules.
+# Validated on Postgres with INTEGER arguments.
+!use post
+
+# start + length = -1, so the result is empty.
+SELECT '[' || SUBSTRING('abc' FROM CAST(-3 AS BIGINT) FOR CAST(2 AS BIGINT))
|| ']' AS c;
++----+
+| C |
++----+
+| [] |
++----+
+(1 row)
+
+!ok
+
+SELECT '[' || SUBSTRING(v FROM s FOR l) || ']' AS c
+FROM (VALUES ('abc', CAST(-3 AS BIGINT), CAST(2 AS BIGINT))) AS t(v, s, l);
++----+
+| C |
++----+
+| [] |
++----+
+(1 row)
+
+!ok
+
+# start + length = 2, so the result is position 1.
+SELECT SUBSTRING('abc' FROM CAST(-3 AS BIGINT) FOR CAST(5 AS BIGINT)) AS c;
++---+
+| C |
++---+
+| a |
++---+
+(1 row)
+
+!ok
+
+# start + length = 2 again, with values out of the INTEGER range.
+# Postgres has no BIGINT SUBSTRING.
+SELECT SUBSTRING('abc' FROM CAST(-3000000000 AS BIGINT) FOR CAST(3000000002 AS
BIGINT)) AS c;
++---+
+| C |
++---+
+| a |
++---+
+(1 row)
+
+!ok
+
+# A negative length is an error, also when the start is negative.
+SELECT SUBSTRING('abc' FROM CAST(-3 AS BIGINT) FOR -1) AS c;
+Substring error: negative substring length not allowed
+!error
+
+SELECT SUBSTRING(v FROM s FOR l) AS c
+FROM (VALUES ('abc', CAST(-3 AS BIGINT), -1)) AS t(v, s, l);
+Substring error: negative substring length not allowed
+!error
+
+# Binary SUBSTRING with a BIGINT start or length.
+# Validated on Postgres with INTEGER arguments.
+SELECT SUBSTRING(x'123456' FROM 2 FOR 2147483650) AS c;
++------+
+| C |
++------+
+| 3456 |
++------+
+(1 row)
+
+!ok
+
+# start + length = 2, so the result is position 1.
+SELECT SUBSTRING(x'123456' FROM CAST(-3 AS BIGINT) FOR 5) AS c;
++----+
+| C |
++----+
+| 12 |
++----+
+(1 row)
+
+!ok
+
+SELECT SUBSTRING(b FROM s FOR l) AS c
+FROM (VALUES (x'123456', CAST(-3 AS BIGINT), CAST(5 AS BIGINT))) AS t(b, s, l);
++----+
+| C |
++----+
+| 12 |
++----+
+(1 row)
+
+!ok
+
+# Postgres has no BIGINT SUBSTRING.
+SELECT SUBSTRING(x'123456' FROM CAST(-3000000000 AS BIGINT) FOR
CAST(3000000002 AS BIGINT)) AS c;
++----+
+| C |
++----+
+| 12 |
++----+
+(1 row)
+
+!ok
+
+SELECT SUBSTRING(x'123456' FROM CAST(-3000000000 AS BIGINT)) AS c;
++--------+
+| C |
++--------+
+| 123456 |
++--------+
+(1 row)
+
+!ok
+
+SELECT SUBSTRING(x'123456' FROM CAST(-3 AS BIGINT) FOR -1) AS c;
+Substring error: negative substring length not allowed
+!error
+
# End functions.iq
