Fix test framework to handle correctly carets everywhere

Signed-off-by: Mihai Budiu <mbudiu@feldera.com>

Author: mihaibudiu

core/src/main/java/org/apache/calcite/sql/parser/SqlParserUtil.java

@@ -742,10 +742,16 @@ public static int lineColToIndex(String sql, int line, int column) {
     return i + column;
   }
 
+  /** Given a string with carets, double each of them.
+   * (This should probably be more sophisticated, and ignore carets within string literals). */
+  public static String escapeCarets(String sql) {
+    return sql.replace("^", "^^");
+  }
+
   /**
    * Converts a string to a string with one or two carets in it. For example,
    * <code>addCarets("values (foo)", 1, 9, 1, 12)</code> yields "values
-   * (^foo^)".
+   * (^foo^)".  Existing carets are escaped by doubling.
    */
   public static String addCarets(
       String sql,
@@ -755,8 +761,8 @@ public static String addCarets(
       int endCol) {
     String sqlWithCarets;
     int cut = lineColToIndex(sql, line, col);
-    sqlWithCarets = sql.substring(0, cut) + "^"
-        + sql.substring(cut);
+    sqlWithCarets = escapeCarets(sql.substring(0, cut)) + "^"
+        + escapeCarets(sql.substring(cut));
     if ((col != endCol) || (line != endLine)) {
       cut = lineColToIndex(sqlWithCarets, endLine, endCol);
       if (line == endLine) {
@@ -765,7 +771,7 @@ public static String addCarets(
       if (cut < sqlWithCarets.length()) {
         sqlWithCarets =
             sqlWithCarets.substring(0, cut)
-                + "^" + sqlWithCarets.substring(cut);
+                + "^" + escapeCarets(sqlWithCarets.substring(cut));
       } else {
         sqlWithCarets += "^";
       }

core/src/main/java/org/apache/calcite/sql/parser/StringAndPos.java

@@ -21,11 +21,10 @@
 import java.util.Objects;
 
 /**
- * Contains a string, the offset of a token within the string, and a parser position containing the
- * beginning and end line number.
+ * Contains a string, the offset of a token within the string, and a parser
+ * position containing the beginning and end line number.
  */
 public class StringAndPos {
-
   public final String sql;
   public final int cursor;
   public final @Nullable SqlParserPos pos;
@@ -53,66 +52,8 @@ private StringAndPos(String sql, int cursor, @Nullable SqlParserPos pos) {
   }
 
   /**
-   * 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
-   * to identify common XOR patterns that can be evaluated directly without complex rewriting.
-   *
-   * <p>Supported patterns include:
-   * <ul>
-   *   <li><b>Literal numbers:</b> {@code number ^ number}, e.g.,
-   *   {@code "5 ^ 3"}, {@code "-2 ^ 7"}</li>
-   *   <li><b>Null handling:</b>
-   *     <ul>
-   *       <li>{@code NULL ^ number} or {@code CAST(NULL AS TYPE) ^ number}</li>
-   *       <li>{@code number ^ NULL} or {@code number ^ CAST(NULL AS TYPE)}</li>
-   *     </ul>
-   *   </li>
-   *   <li><b>Typed casts:</b> {@code CAST(...) ^ CAST(...)},
-   *   including types with precision or UNSIGNED,
-   *       e.g., {@code CAST(5 AS INTEGER) ^ CAST(3 AS BIGINT)},
-   *       {@code CAST(255 AS INTEGER UNSIGNED) ^ CAST(1 AS INTEGER UNSIGNED)}</li>
-   * </ul>
-   *
-   * @param sql the SQL expression containing the XOR (^) operator
-   * @return true if the expression matches a simple XOR pattern, false otherwise
-   */
-  private static boolean isSimpleXorPattern(String sql) {
-    int caretIndex = sql.indexOf('^');
-    if (caretIndex == -1) {
-      return false;
-    }
-    String before = sql.substring(0, caretIndex).trim();
-    String after = sql.substring(caretIndex + 1).trim();
-
-    // Pattern: number ^ number
-    if (before.matches(".*-?\\d$") && after.matches("^-?\\d.*")) {
-      return true;
-    }
-    if (before.matches(".*CAST\\s*\\(\\s*NULL\\s+AS\\s+\\w+(\\(\\d+\\))?\\s*\\)$")
-        && after.matches("^\\d.*")) {
-      return true;
-    }
-
-    // number ^ CAST(NULL AS ...)
-    if (before.matches(".*\\d$")
-        && after.matches("^CAST\\s*\\(\\s*NULL\\s+AS\\s+\\w+(\\(\\d+\\))?\\s*\\).*")) {
-      return true;
-    }
-    // Pattern: CAST(...) ^ CAST(...)
-    String castPattern =
-        "CAST\\s*\\(.*?AS\\s+[A-Z]+(?:\\s*\\(\\d+(?:,\\d+)?\\))?(?:\\s+UNSIGNED)?\\s*\\)";
-    if (before.matches(".*" + castPattern + "$")
-        && after.matches("^" + castPattern + ".*")) {
-      return true;
-    }
-
-    return false;
-  }
-
-  /**
-   * Looks for one or two carets in a SQL string, and if present, converts them into a parser
-   * position.
+   * Looks for one or two carets in a SQL string, and if present, converts
+   * them into a parser position.
    *
    * <p>Examples:
    *
@@ -124,15 +65,10 @@ private static boolean isSimpleXorPattern(String sql) {
    * </ul>
    */
   public static StringAndPos of(String sql) {
-
     int firstCaret = sql.indexOf('^');
     if (firstCaret < 0) {
       return new StringAndPos(sql, -1, null);
     }
-    // check for bitxor operator test cases
-    if (isSimpleXorPattern(sql)) {
-      return new StringAndPos(sql, -1, null);
-    }
     int secondCaret = sql.indexOf('^', firstCaret + 1);
     if (secondCaret == firstCaret + 1) {
       // If SQL contains "^^", it does not contain error positions; convert each
@@ -169,7 +105,7 @@ public static StringAndPos of(String sql) {
   }
 
   public String addCarets() {
-    return pos == null ? sql
+    return pos == null ? SqlParserUtil.escapeCarets(sql)
         : SqlParserUtil.addCarets(sql, pos.getLineNum(), pos.getColumnNum(),
             pos.getEndLineNum(), pos.getEndColumnNum() + 1);
   }

core/src/test/java/org/apache/calcite/test/SqlOperatorUnparseTest.java

@@ -19,6 +19,8 @@
 import org.apache.calcite.sql.SqlNode;
 import org.apache.calcite.sql.parser.SqlParseException;
 import org.apache.calcite.sql.parser.SqlParser;
+import org.apache.calcite.sql.parser.SqlParserUtil;
+import org.apache.calcite.sql.parser.StringAndPos;
 import org.apache.calcite.sql.test.SqlOperatorFixture;
 import org.apache.calcite.sql.test.SqlTestFactory;
 
@@ -77,21 +79,26 @@ TesterImpl withFactory(UnaryOperator<SqlTestFactory> transform) {
       return new UnparseTester(transform.apply(this.factory));
     }
 
-    String rewrite(String sql) throws SqlParseException {
-      final SqlParser parser = factory.createParser(sql);
+    String rewrite(StringAndPos sap) throws SqlParseException {
+      final SqlParser parser = factory.createParser(sap.sql);
       final SqlNode sqlNode = parser.parseStmt();
-      return sqlNode.toSqlString(c -> c.withClauseStartsLine(false)).getSql();
+      String result = sqlNode.toSqlString(c -> c.withClauseStartsLine(false)).getSql();
+      return SqlParserUtil.escapeCarets(result);
     }
 
     @Override public void forEachQuery(
-        SqlTestFactory factory, String expression, Consumer<String> consumer) {
-      consumer.accept(buildQuery2(factory, expression));
+        SqlTestFactory factory, String expressionWithCarets, Consumer<String> consumer) {
+      String query = buildQuery2(factory, expressionWithCarets);
+      query = SqlParserUtil.escapeCarets(query);
+      consumer.accept(query);
     }
 
-    @Override public void check(SqlTestFactory factory, String sql, TypeChecker typeChecker,
+    @Override public void check(
+        SqlTestFactory factory, String queryWithCarets, TypeChecker typeChecker,
         ParameterChecker parameterChecker, ResultChecker resultChecker) {
       try {
-        String optQuery = this.rewrite(sql);
+        StringAndPos sap = StringAndPos.of(queryWithCarets);
+        String optQuery = this.rewrite(sap);
         super.check(factory, optQuery, typeChecker, parameterChecker, resultChecker);
       } catch (SqlParseException e) {
         throw new RuntimeException(e);

testkit/src/main/java/org/apache/calcite/sql/test/AbstractSqlTester.java

@@ -222,7 +222,7 @@ private static ValidatedNodeConsumer checkColumnTypeAction(
   }
 
   @Override public void check(SqlTestFactory factory,
-      String query, TypeChecker typeChecker,
+      String queryWithCarets, TypeChecker typeChecker,
       ParameterChecker parameterChecker, ResultChecker resultChecker) {
     // This implementation does NOT check the result!
     // All it does is check the return type.
@@ -232,12 +232,14 @@ private static ValidatedNodeConsumer checkColumnTypeAction(
 
     // Parse and validate. There should be no errors.
     // There must be 1 column. Get its type.
-    RelDataType actualType = getColumnType(factory, query);
+    RelDataType actualType = getColumnType(factory, queryWithCarets);
 
     // Check result type.
-    typeChecker.checkType(() -> "Query: " + query, actualType);
+    typeChecker.checkType(() -> "Query: " + queryWithCarets, actualType);
 
-    Pair<SqlValidator, SqlNode> p = parseAndValidate(factory, query);
+    // Unescape the carets that may be present in the query
+    final StringAndPos pos = StringAndPos.of(queryWithCarets);
+    Pair<SqlValidator, SqlNode> p = parseAndValidate(factory, pos.sql);
     SqlValidator validator = p.left;
     SqlNode n = p.right;
     final RelDataType parameterRowType = validator.getParameterRowType(n);
@@ -265,12 +267,12 @@ private static ValidatedNodeConsumer checkColumnTypeAction(
     if (runtime) {
       // We need to test that the expression fails at runtime.
       // Ironically, that means that it must succeed at prepare time.
-      final String sql = buildQuery(sap.addCarets());
+      final String sql = buildQuery(sap);
       Pair<SqlValidator, SqlNode> p = parseAndValidate(factory, sql);
       SqlNode n = p.right;
       assertNotNull(n);
     } else {
-      StringAndPos sap1 = StringAndPos.of(buildQuery(sap.addCarets()));
+      StringAndPos sap1 = StringAndPos.of(buildQuery(sap));
       checkQueryFails(factory, sap1, expectedError);
     }
   }
@@ -296,8 +298,23 @@ private static ValidatedNodeConsumer checkColumnTypeAction(
     }
   }
 
-  public static String buildQuery(String expression) {
-    return "values (" + expression + ")";
+  /**
+   * Build a legal query from an expression with carets.
+   *
+   * <p>@deprecated Please use {@link AbstractSqlTester#buildQuery(StringAndPos)}
+   */
+  @Deprecated
+  public static String buildQuery(String expressionWithCarets) {
+    StringAndPos sap = StringAndPos.of(expressionWithCarets);
+    return buildQuery(sap);
+  }
+
+  public static String buildQuery(StringAndPos sap) {
+    return "values (" + sap.sql + ")";
+  }
+
+  public static StringAndPos buildQueryWithPos(StringAndPos sap) {
+    return StringAndPos.of("values (" + sap.addCarets() + ")");
   }
 
   public static String buildQueryAgg(String expression) {
@@ -322,21 +339,24 @@ public static String buildQueryAgg(String expression) {
    * {@code CASE 1 WHEN 2 THEN 'a' ELSE NULL END} are left as is.
    *
    * @param factory Test factory
-   * @param expression Scalar expression
+   * @param expressionWithCarets Scalar expression that may contain carets
    * @return Query that evaluates a scalar expression
    */
-  protected String buildQuery2(SqlTestFactory factory, String expression) {
-    if (expression.matches("(?i).*(percentile_(cont|disc)|convert|sort_array|cast)\\(.*")) {
+  protected String buildQuery2(SqlTestFactory factory, String expressionWithCarets) {
+    if (expressionWithCarets.matches(
+        "(?i).*(percentile_(cont|disc)|convert|sort_array|cast)\\(.*")) {
       // PERCENTILE_CONT requires its argument to be a literal,
       // so converting its argument to a column will cause false errors.
       // Similarly, MSSQL-style CONVERT.
-      return buildQuery(expression);
+      return buildQuery(expressionWithCarets);
     }
     // "values (1 < 5)"
     // becomes
     // "select p0 < p1 from (values (1, 5)) as t(p0, p1)"
     SqlNode x;
-    final String sql = "values (" + expression + ")";
+    // Expression may contain escaped carets; unescape them.
+    final StringAndPos sap = StringAndPos.of(expressionWithCarets);
+    final String sql = "values (" + sap.sql + ")";
     try {
       x = parseQuery(factory, sql);
     } catch (SqlParseException e) {
@@ -447,12 +467,13 @@ private boolean isNull(SqlNode sqlNode) {
   }
 
   @Override public void forEachQuery(SqlTestFactory factory,
-      String expression, Consumer<String> consumer) {
+      String expressionWithCarets, Consumer<String> consumer) {
     // Why not return a list? If there is a syntax error in the expression, the
     // consumer will discover it before we try to parse it to do substitutions
     // on the parse tree.
-    consumer.accept("values (" + expression + ")");
-    consumer.accept(buildQuery2(factory, expression));
+    consumer.accept("values (" + expressionWithCarets + ")");
+    final String query2 = buildQuery2(factory, expressionWithCarets);
+    consumer.accept(SqlParserUtil.escapeCarets(query2));
   }
 
   @Override public void assertConvertsTo(SqlTestFactory factory,

testkit/src/main/java/org/apache/calcite/sql/test/SqlTester.java

@@ -62,10 +62,10 @@ enum VmName {
    * evaluate it, and calls a given action with each.
    *
    * @param factory    Factory
-   * @param expression Scalar expression
+   * @param expressionWithCarets Scalar expression, possibly with carets
    * @param consumer   Action to be called for each query
    */
-  void forEachQuery(SqlTestFactory factory, String expression,
+  void forEachQuery(SqlTestFactory factory, String expressionWithCarets,
       Consumer<String> consumer);
 
   /** Parses a query. */
@@ -165,14 +165,14 @@ default void check(SqlTestFactory factory,
    * and {@link ResultChecker} functors.
    *
    * @param factory       Factory
-   * @param query         SQL query
+   * @param queryWithCarets SQL query, possibly with carets
    * @param typeChecker   Checks whether the result is the expected type
    * @param parameterChecker Checks whether the parameters are of expected
    *                      types
    * @param resultChecker Checks whether the result has the expected value
    */
   void check(SqlTestFactory factory,
-      String query,
+      String queryWithCarets,
       TypeChecker typeChecker,
       ParameterChecker parameterChecker,
       ResultChecker resultChecker);

testkit/src/main/java/org/apache/calcite/test/SqlOperatorFixtureImpl.java

@@ -156,15 +156,16 @@ void forEachQueryValidateAndThen(StringAndPos expression,
 
   @Override public void checkFails(StringAndPos sap, String expectedError,
       boolean runtime) {
-    final String sql = "values (" + sap.addCarets() + ")";
     if (runtime) {
       // We need to test that the expression fails at runtime.
       // Ironically, that means that it must succeed at prepare time.
+      final String sql = "values (" + sap.sql + ")";
       SqlValidator validator = factory.createValidator();
       SqlNode n = parseAndValidate(validator, sql);
       assertNotNull(n);
       tester.checkFails(factory, sap, expectedError, runtime);
     } else {
+      final String sql = "values (" + sap.addCarets() + ")";
       checkQueryFails(StringAndPos.of(sql),
           expectedError);
     }