mihaibudiu commented on code in PR #4478:
URL: https://github.com/apache/calcite/pull/4478#discussion_r2294908607
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -3506,6 +3506,152 @@ private static ByteString binaryOperator(
return new ByteString(result);
}
+ /**
+ * Performs bitwise shift on two integers.
+ * If the shift amount is positive, a left shift is applied.
+ * If the shift amount is negative, a logical right shift is applied.
+ * The shift amount is taken modulo 32.
+ */
+
Review Comment:
not crazy about the extra blank line
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -3506,6 +3506,152 @@ private static ByteString binaryOperator(
return new ByteString(result);
}
+ /**
+ * Performs bitwise shift on two integers.
+ * If the shift amount is positive, a left shift is applied.
+ * If the shift amount is negative, a logical right shift is applied.
+ * The shift amount is taken modulo 32.
+ */
+
+ public static int leftShift(int x, int y) {
+ int shift = Math.abs(y % 32);
+ return y >= 0 ? x << shift : x >>> shift;
+ }
+
+ /**
+ * Performs bitwise shift on a long value.
+ * If the shift amount is positive, a left shift is applied.
+ * If the shift amount is negative, a logical right shift is applied.
+ * The shift amount is taken modulo 64.
+ */
+
+ public static long leftShift(long x, int y) {
+ int shift = Math.abs(y % 64);
+ return y >= 0 ? x << shift : x >>> shift;
+ }
+
+ /**
+ * Performs bitwise shift with long shift amount.
+ * Note: input x is int, so shift mod 32 is used.
+ * Returns long to prevent overflow.
+ */
+ public static long leftShift(int x, long y) {
+ int shift = (int) Math.abs(y % 32);
+ return y >= 0 ? (long) x << shift : (long) x >>> shift;
+ }
+
+ /**
+ * Performs bitwise shift on a byte array.
+ * A positive shift amount performs a left shift.
+ * A negative shift amount performs a logical right shift.
+ */
+ public static ByteString leftShift(ByteString bytes, int y) {
+ byte[] result = leftShift(bytes.getBytes(), y);
+ return new ByteString(result);
+ }
+
+ /**
+ * Performs bitwise shift on a byte array.
+ * A positive shift amount performs a left shift.
+ * A negative shift amount performs a logical right shift.
+ */
+ public static byte[] leftShift(byte[] bytes, int y) {
Review Comment:
this function is quite involved, can you please add a few unit tests just
for it.
The unit tests could have a for loop to iterate over a set of values of y.
##########
site/_docs/reference.md:
##########
@@ -2828,6 +2828,7 @@ In the following:
| * | BITAND(value1, value2) | Returns the bitwise AND
of *value1* and *value2*. *value1* and *value2* must both be integer or binary
values. Binary values must be of the same length.
| * | BITOR(value1, value2) | Returns the bitwise OR
of *value1* and *value2*. *value1* and *value2* must both be integer or binary
values. Binary values must be of the same length.
| * | BITXOR(value1, value2) | Returns the bitwise XOR
of *value1* and *value2*. *value1* and *value2* must both be integer or binary
values. Binary values must be of the same length.
+| * | LEFTSHIFT(value1, value2) | Returns the result of
left-shifting *value1* by *value2* bits. *value1* can be integer, unsigned
integer, or binary. For binary, the result has the same length as *value1*. |
Review Comment:
please describe what happens for negative shift amounts.
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -16518,6 +16518,218 @@ private static void
checkLogicalOrFunc(SqlOperatorFixture f) {
f.checkNull("CAST(NULL AS INTEGER UNSIGNED) ^^ CAST(NULL AS INTEGER
UNSIGNED)");
}
+ /**
+ * Test cases for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7109";>[CALCITE-7109]
+ * Implement SHIFT_LEFT operator </a>.
+ */
+ @Test void testLeftShiftScalarFunc() {
+ final SqlOperatorFixture f = fixture();
+ f.setFor(SqlStdOperatorTable.BIT_LEFT_SHIFT, VmName.EXPAND);
+
+ // === Basic functionality ===
+ f.checkScalar("2 << 2", "8", "INTEGER NOT NULL");
+ f.checkScalar("1 << 10", "1024", "INTEGER NOT NULL");
+ f.checkScalar("0 << 5", "0", "INTEGER NOT NULL");
+
+ // === Type coercion and signed behavior ===
+ f.checkScalar("CAST(2 AS INTEGER) << CAST(3 AS BIGINT)", "16", "INTEGER
NOT NULL");
+ f.checkScalar("-5 << 2", "-20", "INTEGER NOT NULL");
+ f.checkScalar("-5 << 3", "-40", "INTEGER NOT NULL");
+ f.checkScalar("CAST(-5 AS TINYINT) << CAST(2 AS TINYINT)", "-20", "TINYINT
NOT NULL");
+
+ // === Verify return type matches first argument type ===
+ f.checkType("CAST(2 AS TINYINT) << CAST(3 AS TINYINT)", "TINYINT NOT
NULL");
+ f.checkType("CAST(2 AS SMALLINT) << CAST(3 AS SMALLINT)", "SMALLINT NOT
NULL");
+ f.checkType("CAST(2 AS INTEGER) << CAST(3 AS INTEGER)", "INTEGER NOT
NULL");
+ f.checkType("CAST(2 AS BIGINT) << CAST(3 AS BIGINT)", "BIGINT NOT NULL");
+
+ // === BigInt shifts with explicit BIGINT inputs ===
+ f.checkScalar("CAST(1 AS BIGINT) << 62", "4611686018427387904", "BIGINT
NOT NULL"); // 2^62
+ f.checkScalar("CAST(1 AS BIGINT) << 63", "-9223372036854775808", "BIGINT
NOT NULL"); // overflow
+ f.checkScalar("CAST(4611686018427387904 AS BIGINT) << 1",
+ "-9223372036854775808", "BIGINT NOT NULL"); // overflow
+ f.checkScalar("CAST(2305843009213693952 AS BIGINT) << 2",
+ "-9223372036854775808", "BIGINT NOT NULL"); // overflow
+ f.checkScalar("CAST(-4611686018427387904 AS BIGINT) << 1",
+ "-9223372036854775808", "BIGINT NOT NULL");
+ f.checkScalar("CAST(-1 AS BIGINT) << 63",
+ "-9223372036854775808", "BIGINT NOT NULL");
+ f.checkScalar("CAST(9223372036854775807 AS BIGINT) << 0",
+ "9223372036854775807", "BIGINT NOT NULL");
+ f.checkScalar("CAST(1000000000 AS BIGINT) << 35",
+ "-2533749779419103232", "BIGINT NOT NULL");
+ f.checkScalar("CAST(9223372036854775807 AS BIGINT) << 1",
+ "-2", "BIGINT NOT NULL");
+
+ // === Java shift semantics: bits masked to 5/6 bits ===
+ f.checkScalar("CAST(1 AS BIGINT) << 32",
+ "4294967296", "BIGINT NOT NULL");
+ f.checkScalar("CAST(1 AS BIGINT) << 50",
+ "1125899906842624", "BIGINT NOT NULL");
+ f.checkScalar("CAST(1 AS BIGINT) << 100",
+ "68719476736", "BIGINT NOT NULL");
+ f.checkScalar("CAST(100 AS BIGINT) << 50",
+ "112589990684262400", "BIGINT NOT NULL");
+
+ f.checkScalar("CAST(100 AS BIGINT) << 50",
+ "112589990684262400", "BIGINT NOT NULL");
+
+ // Test left shift (<<) with unsigned types
+ // 63 << 2 = 252
+ f.checkScalar("CAST(63 AS TINYINT UNSIGNED) << 2",
+ "252", "TINYINT UNSIGNED NOT NULL");
+
+ // 255 << 8 = 65280
+ f.checkScalar("CAST(255 AS SMALLINT UNSIGNED) << 8",
+ "65280", "SMALLINT UNSIGNED NOT NULL");
+
+ // 65535 << 16 = 4294901760
+ f.checkScalar("CAST(65535 AS INTEGER UNSIGNED) << 16",
+ "4294901760", "INTEGER UNSIGNED NOT NULL");
+
+ // 1 << 31 = 2147483648 — requires long intermediate result
+ f.checkScalar("CAST(1 AS INTEGER UNSIGNED) << 31",
+ "2147483648", "INTEGER UNSIGNED NOT NULL");
+
+ // Shift by negative — logical right shift by 1 = 0
+ f.checkScalar("CAST(1 AS INTEGER UNSIGNED) << -1",
+ "0", "INTEGER UNSIGNED NOT NULL");
+
+
+ // === Negative shift counts ===
+ f.checkScalar("8 << -1", "4", "INTEGER NOT NULL");
+ f.checkScalar("16 << -2", "4", "INTEGER NOT NULL");
+
+ // === Shift by zero and large (but valid) shifts ===
+ f.checkScalar("0 << 32", "0", "INTEGER NOT NULL");
Review Comment:
can you also shift a non-zero value with a large amount?
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -16518,6 +16518,218 @@ private static void
checkLogicalOrFunc(SqlOperatorFixture f) {
f.checkNull("CAST(NULL AS INTEGER UNSIGNED) ^^ CAST(NULL AS INTEGER
UNSIGNED)");
}
+ /**
+ * Test cases for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7109";>[CALCITE-7109]
+ * Implement SHIFT_LEFT operator </a>.
+ */
+ @Test void testLeftShiftScalarFunc() {
+ final SqlOperatorFixture f = fixture();
+ f.setFor(SqlStdOperatorTable.BIT_LEFT_SHIFT, VmName.EXPAND);
+
+ // === Basic functionality ===
+ f.checkScalar("2 << 2", "8", "INTEGER NOT NULL");
+ f.checkScalar("1 << 10", "1024", "INTEGER NOT NULL");
+ f.checkScalar("0 << 5", "0", "INTEGER NOT NULL");
+
+ // === Type coercion and signed behavior ===
+ f.checkScalar("CAST(2 AS INTEGER) << CAST(3 AS BIGINT)", "16", "INTEGER
NOT NULL");
+ f.checkScalar("-5 << 2", "-20", "INTEGER NOT NULL");
+ f.checkScalar("-5 << 3", "-40", "INTEGER NOT NULL");
+ f.checkScalar("CAST(-5 AS TINYINT) << CAST(2 AS TINYINT)", "-20", "TINYINT
NOT NULL");
+
+ // === Verify return type matches first argument type ===
+ f.checkType("CAST(2 AS TINYINT) << CAST(3 AS TINYINT)", "TINYINT NOT
NULL");
+ f.checkType("CAST(2 AS SMALLINT) << CAST(3 AS SMALLINT)", "SMALLINT NOT
NULL");
+ f.checkType("CAST(2 AS INTEGER) << CAST(3 AS INTEGER)", "INTEGER NOT
NULL");
+ f.checkType("CAST(2 AS BIGINT) << CAST(3 AS BIGINT)", "BIGINT NOT NULL");
+
+ // === BigInt shifts with explicit BIGINT inputs ===
+ f.checkScalar("CAST(1 AS BIGINT) << 62", "4611686018427387904", "BIGINT
NOT NULL"); // 2^62
Review Comment:
how were these results validated?
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -3506,6 +3506,152 @@ private static ByteString binaryOperator(
return new ByteString(result);
}
+ /**
+ * Performs bitwise shift on two integers.
+ * If the shift amount is positive, a left shift is applied.
+ * If the shift amount is negative, a logical right shift is applied.
+ * The shift amount is taken modulo 32.
+ */
+
+ public static int leftShift(int x, int y) {
+ int shift = Math.abs(y % 32);
+ return y >= 0 ? x << shift : x >>> shift;
+ }
+
+ /**
+ * Performs bitwise shift on a long value.
+ * If the shift amount is positive, a left shift is applied.
+ * If the shift amount is negative, a logical right shift is applied.
+ * The shift amount is taken modulo 64.
+ */
+
+ public static long leftShift(long x, int y) {
+ int shift = Math.abs(y % 64);
+ return y >= 0 ? x << shift : x >>> shift;
+ }
+
+ /**
+ * Performs bitwise shift with long shift amount.
+ * Note: input x is int, so shift mod 32 is used.
+ * Returns long to prevent overflow.
+ */
+ public static long leftShift(int x, long y) {
+ int shift = (int) Math.abs(y % 32);
+ return y >= 0 ? (long) x << shift : (long) x >>> shift;
+ }
+
+ /**
+ * Performs bitwise shift on a byte array.
+ * A positive shift amount performs a left shift.
+ * A negative shift amount performs a logical right shift.
+ */
+ public static ByteString leftShift(ByteString bytes, int y) {
+ byte[] result = leftShift(bytes.getBytes(), y);
+ return new ByteString(result);
+ }
+
+ /**
+ * Performs bitwise shift on a byte array.
+ * A positive shift amount performs a left shift.
+ * A negative shift amount performs a logical right shift.
+ */
+ public static byte[] leftShift(byte[] bytes, int y) {
+ int shift = Math.abs(y % 32);
+
+ if (shift == 0) {
+ return bytes.clone();
+ }
+
+ byte[] result = new byte[bytes.length];
+
+ if (y >= 0) {
+ // Left shift
+ int byteShift = shift / 8;
+ int bitShift = shift % 8;
+
+ if (shift >= bytes.length * 8) {
+ return new byte[bytes.length];
+ }
+
+ if (bitShift == 0) {
+ System.arraycopy(bytes, 0, result, byteShift, bytes.length -
byteShift);
+ } else {
+ int carry = 0;
+ for (int i = bytes.length - 1 - byteShift; i >= 0; i--) {
+ int current = (bytes[i] & 0xFF) << bitShift;
+ result[i + byteShift] = (byte) ((current | carry) & 0xFF);
+ carry = (current >> 8) & 0xFF;
+ }
+ if (byteShift > 0) {
+ result[byteShift - 1] = (byte) carry;
+ }
+ }
+
Review Comment:
I would remove this empty line too
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -895,6 +897,9 @@ void populate1() {
defineMethod(TYPEOF, BuiltInMethod.TYPEOF.method, NullPolicy.STRICT);
defineMethod(VARIANTNULL, BuiltInMethod.VARIANTNULL.method,
NullPolicy.STRICT);
+ // shift
+ defineMethod(LEFTSHIFT, BuiltInMethod.LEFT_SHIFT.method,
NullPolicy.STRICT);
Review Comment:
Can you add a comment describing the difference between these two?
--
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]
