Use structured condition tokens for filter translation

Author: caicancai

arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowFilter.java

@@ -34,7 +34,7 @@
  * relational expression in Arrow.
  */
 class ArrowFilter extends Filter implements ArrowRel {
-  private final List<List<String>> match;
+  private final List<List<List<String>>> match;
 
   ArrowFilter(RelOptCluster cluster, RelTraitSet traitSet, RelNode input, RexNode condition) {
     super(cluster, traitSet, input, condition);

arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowRel.java

@@ -41,15 +41,15 @@ public interface ArrowRel extends RelNode {
    * {@link ArrowRel} nodes into a SQL query. */
   class Implementor {
     @Nullable List<Integer> selectFields;
-    final List<List<String>> whereClause = new ArrayList<>();
+    final List<List<List<String>>> whereClause = new ArrayList<>();
     @Nullable RelOptTable table;
     @Nullable ArrowTable arrowTable;
 
     /** Adds new predicates.
      *
      * @param predicates Predicates in CNF form (outer list is AND, inner list is OR)
      */
-    void addFilters(List<List<String>> predicates) {
+    void addFilters(List<List<List<String>>> predicates) {
       whereClause.addAll(predicates);
     }
 

arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowTable.java

@@ -97,7 +97,7 @@ public class ArrowTable extends AbstractTable
    * {@link org.apache.calcite.adapter.arrow.ArrowMethod#ARROW_QUERY}. */
   @SuppressWarnings("unused")
   public Enumerable<Object> query(DataContext root, ImmutableIntList fields,
-      List<List<String>> conditions) {
+      List<List<List<String>>> conditions) {
     requireNonNull(fields, "fields");
     final Projector projector;
     final Filter filter;
@@ -120,10 +120,10 @@ public Enumerable<Object> query(DataContext root, ImmutableIntList fields,
       projector = null;
 
       final List<TreeNode> conjuncts = new ArrayList<>(conditions.size());
-      for (List<String> orGroup : conditions) {
+      for (List<List<String>> orGroup : conditions) {
         final List<TreeNode> disjuncts = new ArrayList<>(orGroup.size());
-        for (String condition : orGroup) {
-          disjuncts.add(parseSingleCondition(condition));
+        for (List<String> conditionParts : orGroup) {
+          disjuncts.add(parseSingleCondition(conditionParts));
         }
         if (disjuncts.size() == 1) {
           conjuncts.add(disjuncts.get(0));
@@ -178,30 +178,39 @@ private static RelDataType deduceRowType(Schema schema,
     return builder.build();
   }
 
-  /** Parses a single condition string into a Gandiva {@link TreeNode}.
+  /** Parses a single condition into a Gandiva {@link TreeNode}.
    *
-   * <p>The condition string format is:
+   * <p>The condition token format is:
    * <ul>
-   *   <li>Binary: {@code "fieldName operator value type"},
-   *       e.g. {@code "intField equal 12 integer"}
-   *   <li>Unary: {@code "fieldName operator"},
-   *       e.g. {@code "intField isnull"}
+   *   <li>Binary: {@code [fieldName, operator, value, type]},
+   *       e.g. {@code ["intField", "equal", "12", "integer"]}</li>
+   *   <li>Unary: {@code [fieldName, operator]},
+   *       e.g. {@code ["intField", "isnull"]}</li>
    * </ul>
+   *
+   * <p>Using structured tokens avoids string splitting and safely supports
+   * values with spaces (for example, {@code "'literal with space'"}) and
+   * empty-string literals ({@code "''"}).
    */
-  private TreeNode parseSingleCondition(String condition) {
-    final String[] data = condition.split(" ");
+  private TreeNode parseSingleCondition(List<String> conditionParts) {
+    final int size = conditionParts.size();
+    if (size != 2 && size != 4) {
+      throw new IllegalArgumentException("Invalid condition: " + conditionParts);
+    }
+    final String fieldName = conditionParts.get(0);
+    final String operator = conditionParts.get(1);
+    final boolean isBinary = size == 4;
+
     final List<TreeNode> treeNodes = new ArrayList<>(2);
     treeNodes.add(
         TreeBuilder.makeField(schema.getFields()
-            .get(schema.getFields().indexOf(schema.findField(data[0])))));
+            .get(schema.getFields().indexOf(schema.findField(fieldName)))));
 
-    // if the split condition has more than two parts it's a binary operator
-    // with an additional literal node
-    if (data.length > 2) {
-      treeNodes.add(makeLiteralNode(data[2], data[3]));
+    if (isBinary) {
+      treeNodes.add(makeLiteralNode(conditionParts.get(2),
+          conditionParts.get(3)));
     }
 
-    final String operator = data[1];
     return TreeBuilder.makeFunction(operator, treeNodes, new ArrowType.Bool());
   }
 

arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowTranslator.java

@@ -29,6 +29,8 @@
 import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.util.DateString;
 
+import com.google.common.collect.ImmutableList;
+
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 import java.text.SimpleDateFormat;
@@ -41,7 +43,7 @@
 import static java.util.Objects.requireNonNull;
 
 /**
- * Translates a {@link RexNode} expression to a Gandiva string.
+ * Translates a {@link RexNode} expression to Gandiva predicate tokens.
  */
 class ArrowTranslator {
   final RexBuilder rexBuilder;
@@ -65,14 +67,14 @@ public static ArrowTranslator create(RexBuilder rexBuilder,
    * If exceeded, the original expression is returned unchanged. */
   private static final int MAX_CNF_NODE_COUNT = 256;
 
-  List<List<String>> translateMatch(RexNode condition) {
+  List<List<List<String>>> translateMatch(RexNode condition) {
     // Convert to CNF; SEARCH nodes are already expanded
     // by ArrowFilterRule before reaching here.
     final RexNode cnf = RexUtil.toCnf(rexBuilder, MAX_CNF_NODE_COUNT, condition);
 
-    final List<List<String>> result = new ArrayList<>();
+    final List<List<List<String>>> result = new ArrayList<>();
     for (RexNode conjunct : RelOptUtil.conjunctions(cnf)) {
-      final List<String> orGroup = new ArrayList<>();
+      final List<List<String>> orGroup = new ArrayList<>();
       for (RexNode disjunct : RelOptUtil.disjunctions(conjunct)) {
         orGroup.add(translateMatch2(disjunct));
       }
@@ -109,9 +111,9 @@ private static Object literalValue(RexLiteral literal) {
    *
    * @param node A RexNode that always evaluates to a boolean expression.
    *             Currently, this method is only called from translateAnd.
-   * @return The translated SQL string for the relation.
+   * @return The translated predicate tokens for the relation.
    */
-  private String translateMatch2(RexNode node) {
+  private List<String> translateMatch2(RexNode node) {
     switch (node.getKind()) {
     case EQUALS:
       return translateBinary("equal", "=", (RexCall) node);
@@ -135,7 +137,7 @@ private String translateMatch2(RexNode node) {
       return translateUnary("isnotfalse", (RexCall) node);
     case INPUT_REF:
       final RexInputRef inputRef = (RexInputRef) node;
-      return fieldNames.get(inputRef.getIndex()) + " istrue";
+      return ImmutableList.of(fieldNames.get(inputRef.getIndex()), "istrue");
     case NOT:
       return translateUnary("isfalse", (RexCall) node);
     default:
@@ -147,10 +149,10 @@ private String translateMatch2(RexNode node) {
    * Translates a call to a binary operator, reversing arguments if
    * necessary.
    */
-  private String translateBinary(String op, String rop, RexCall call) {
+  private List<String> translateBinary(String op, String rop, RexCall call) {
     final RexNode left = call.operands.get(0);
     final RexNode right = call.operands.get(1);
-    @Nullable String expression = translateBinary2(op, left, right);
+    @Nullable List<String> expression = translateBinary2(op, left, right);
     if (expression != null) {
       return expression;
     }
@@ -162,7 +164,7 @@ private String translateBinary(String op, String rop, RexCall call) {
   }
 
   /** Translates a call to a binary operator. Returns null on failure. */
-  private @Nullable String translateBinary2(String op, RexNode left, RexNode right) {
+  private @Nullable List<String> translateBinary2(String op, RexNode left, RexNode right) {
     if (right.getKind() != SqlKind.LITERAL) {
       return null;
     }
@@ -180,8 +182,8 @@ private String translateBinary(String op, String rop, RexCall call) {
     }
   }
 
-  /** Combines a field name, operator, and literal to produce a predicate string. */
-  private String translateOp2(String op, String name, RexLiteral right) {
+  /** Combines a field name, operator, and literal to produce predicate tokens. */
+  private List<String> translateOp2(String op, String name, RexLiteral right) {
     Object value = literalValue(right);
     String valueString = value.toString();
     String valueType = getLiteralType(right.getType());
@@ -193,13 +195,13 @@ private String translateOp2(String op, String name, RexLiteral right) {
         valueString = "'" + valueString + "'";
       }
     }
-    return name + " " + op + " " + valueString + " " + valueType;
+    return ImmutableList.of(name, op, valueString, valueType);
   }
 
   /** Translates a call to a unary operator. */
-  private String translateUnary(String op, RexCall call) {
+  private List<String> translateUnary(String op, RexCall call) {
     final RexNode opNode = call.operands.get(0);
-    @Nullable String expression = translateUnary2(op, opNode);
+    @Nullable List<String> expression = translateUnary2(op, opNode);
 
     if (expression != null) {
       return expression;
@@ -209,7 +211,7 @@ private String translateUnary(String op, RexCall call) {
   }
 
   /** Translates a call to a unary operator. Returns null on failure. */
-  private @Nullable String translateUnary2(String op, RexNode opNode) {
+  private @Nullable List<String> translateUnary2(String op, RexNode opNode) {
     if (opNode.getKind() == SqlKind.INPUT_REF) {
       final RexInputRef inputRef = (RexInputRef) opNode;
       final String name = fieldNames.get(inputRef.getIndex());
@@ -219,9 +221,9 @@ private String translateUnary(String op, RexCall call) {
     return null;
   }
 
-  /** Combines a field name and a unary operator to produce a predicate string. */
-  private static String translateUnaryOp(String op, String name) {
-    return name + " " + op;
+  /** Combines a field name and a unary operator to produce predicate tokens. */
+  private static List<String> translateUnaryOp(String op, String name) {
+    return ImmutableList.of(name, op);
   }
 
   private static String getLiteralType(RelDataType  type) {

arrow/src/test/java/org/apache/calcite/adapter/arrow/ArrowAdapterTest.java

@@ -580,14 +580,13 @@ static void initializeArrowState(@TempDir Path sharedTempDir)
         .explainContains(plan);
   }
 
-  @Disabled("literal with space is not supported")
   @Test void testLiteralWithSpace() {
     String sql = "select \"intField\", \"stringField\" as \"my Field\"\n"
         + "from arrowdata\n"
         + "where \"stringField\" = 'literal with space'";
     String plan = "PLAN=ArrowToEnumerableConverter\n"
-        + "  ArrowProject(intField=[$0], my Field=[$1])\n"
-        + "    ArrowFilter(condition=[=($1, '2')])\n"
+        + "  ArrowProject(intField=[$0], stringField=[$1])\n"
+        + "    ArrowFilter(condition=[=($1, 'literal with space')])\n"
         + "      ArrowTableScan(table=[[ARROW, ARROWDATA]], fields=[[0, 1, 2, 3]])\n\n";
     String result = "";
 
@@ -615,6 +614,23 @@ static void initializeArrowState(@TempDir Path sharedTempDir)
         .explainContains(plan);
   }
 
+  @Test void testLiteralWithEmptyString() {
+    String sql = "select \"intField\", \"stringField\"\n"
+        + "from arrowdata\n"
+        + "where \"stringField\" = ''";
+    String plan = "PLAN=ArrowToEnumerableConverter\n"
+        + "  ArrowProject(intField=[$0], stringField=[$1])\n"
+        + "    ArrowFilter(condition=[=($1, '')])\n"
+        + "      ArrowTableScan(table=[[ARROW, ARROWDATA]], fields=[[0, 1, 2, 3]])\n\n";
+    String result = "";
+
+    CalciteAssert.that()
+        .with(arrow)
+        .query(sql)
+        .returns(result)
+        .explainContains(plan);
+  }
+
   @Test void testTinyIntProject() {
     String sql = "select DEPTNO from DEPT";
     String plan = "PLAN=ArrowToEnumerableConverter\n"