mihaibudiu commented on code in PR #4339:
URL: https://github.com/apache/calcite/pull/4339#discussion_r2216541734
##########
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##########
@@ -3417,6 +3417,66 @@ public static ByteString bitNot(ByteString b) {
return new ByteString(result);
}
+ /**
+ * Bitwise function <code>BITXOR</code> applied to {@link Long} values.
+ * Returns {@code 0L} if any operand is null.
+ */
+ public static long bitXor(Long b0, Long b1) {
+ return (b0 == null || b1 == null)
+ ? 0L
+ : b0 ^ b1;
+ }
+
+ /**
+ * Bitwise function <code>BITXOR</code> applied to {@link Integer} values.
+ * Returns {@code 0L} if any operand is null.
+ */
+ public static long bitXor(Integer b0, Integer b1) {
+ return (b0 == null || b1 == null)
+ ? 0L
+ : b0 ^ b1;
+ }
+
+ /**
+ * Bitwise function <code>BITXOR</code> applied to {@link org.joou.UByte}
values.
+ * Returns {@code null} if any operand is null.
+ */
+ public static @PolyNull UByte bitXor(@PolyNull UByte b0, @PolyNull UByte b1)
{
+ return (b0 == null || b1 == null)
+ ? castNonNull(null)
+ : UByte.valueOf(b0.shortValue() ^ b1.shortValue());
+ }
+
+ /**
+ * Bitwise function <code>BITXOR</code> applied to {@link org.joou.UShort}
values.
+ * Returns {@code null} if any operand is null.
+ */
+ public static @PolyNull UShort bitXor(@PolyNull UShort b0, @PolyNull UShort
b1) {
+ return (b0 == null || b1 == null)
+ ? castNonNull(null)
+ : UShort.valueOf(b0.intValue() ^ b1.intValue());
+ }
+
+ /**
+ * Bitwise function <code>BITXOR</code> applied to {@link org.joou.UInteger}
values.
+ * Returns {@code null} if any operand is null.
+ */
+ public static @PolyNull UInteger bitXor(@PolyNull UInteger b0, @PolyNull
UInteger b1) {
+ return (b0 == null || b1 == null)
+ ? castNonNull(null)
+ : UInteger.valueOf(b0.longValue() ^ b1.longValue());
+ }
+
+ /**
+ * Bitwise function <code>BITXOR</code> applied to {@link org.joou.ULong}
values.
+ * Returns {@code null} if any operand is null.
+ */
+ public static @PolyNull ULong bitXor(@PolyNull ULong b0, @PolyNull ULong b1)
{
+ return (b0 == null || b1 == null)
+ ? castNonNull(null)
+ : ULong.valueOf(b0.longValue() ^ b1.longValue());
Review Comment:
I hope this handles values that overflow correctly.
Is there a test for this case (Unsigned value > INT_MAX)
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java:
##########
@@ -1298,6 +1298,19 @@ public class SqlStdOperatorTable extends
ReflectiveSqlOperatorTable {
SqlBasicFunction.create("BITXOR", SqlKind.BITXOR,
ReturnTypes.LARGEST_INT_OR_FIRST_NON_NULL,
OperandTypes.INTEGER_INTEGER.or(OperandTypes.BINARY_BINARY));
+ /**
+ * <code>{@code ^}</code> operator.
+ */
+ public static final SqlBinaryOperator BITXOR_OPERATOR =
+ new SqlBinaryOperator(
+ "^",
+ SqlKind.BITXOR,
+ 40, // Precedence between addition (40) and multiplication
(60)
+ true,
+ ReturnTypes.LARGEST_INT_OR_FIRST_NON_NULL, // Returns same type as
inputs when nullable
Review Comment:
I am not sure what FIRST_NON_NULL means, and the comment makes it even more
ambiguous.
- can the inputs have different types? Type coercion should make sure that
does not happen, but it's not always on
- for different types I expect that the rules for arithmetic could apply
here too
##########
testkit/src/test/java/org/apache/calcite/test/StringAndPosTest.java:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.test;
+
+import org.apache.calcite.sql.parser.StringAndPos;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasToString;
+
+/**
+ * Unit tests for {@link StringAndPos}, particularly around caret handling.
+ */
+public class StringAndPosTest {
Review Comment:
if you don't add these special parsing rules, these tests are no longer
necessary either.
##########
core/src/main/java/org/apache/calcite/sql/parser/StringAndPos.java:
##########
@@ -52,8 +53,54 @@ private StringAndPos(String sql, int cursor, @Nullable
SqlParserPos pos) {
}
/**
- * Looks for one or two carets in a SQL string, and if present, converts
- * them into a parser position.
+ * Checks if the SQL expression is a simple XOR pattern that can be safely
processed.
+ *
+ * <p>This method provides special handling for the BITXOR_OPERATOR (^) in
Apache Calcite
Review Comment:
Let me explain why I think these patterns are not a good idea: they
essentially define a weird language for writing StringAndPos strings, whose
rules cannot be summarized concisely for users. I'd rather have a simple
precise language less convenient for users than a weird one.
--
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]
