mihaibudiu commented on code in PR #4848:
URL: https://github.com/apache/calcite/pull/4848#discussion_r2997947591


##########
arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowTranslator.java:
##########
@@ -61,13 +61,20 @@ public static ArrowTranslator create(RexBuilder rexBuilder,
     return new ArrowTranslator(rexBuilder, rowType);
   }
 
-  List<String> translateMatch(RexNode condition) {
-    List<RexNode> disjunctions = RelOptUtil.disjunctions(condition);
-    if (disjunctions.size() == 1) {
-      return translateAnd(disjunctions.get(0));
-    } else {
-      throw new UnsupportedOperationException("Unsupported disjunctive 
condition " + condition);
+  List<List<String>> translateMatch(RexNode condition) {
+    // Expand SEARCH nodes and convert to CNF
+    final RexNode expanded = RexUtil.expandSearch(rexBuilder, null, condition);
+    final RexNode cnf = RexUtil.toCnf(rexBuilder, expanded);

Review Comment:
   This could be much larger than the original condition.
   You should add some tests with deeper conditions (multiple nested levels of 
parens).



##########
arrow/src/test/java/org/apache/calcite/adapter/arrow/ArrowAdapterTest.java:
##########
@@ -274,23 +263,35 @@ static void initializeArrowState(@TempDir Path 
sharedTempDir)
         .explainContains(plan);
   }
 
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-6636";>[CALCITE-6636]
+   * Support CNF condition of Arrow adapter</a>. */
+  @Test void testArrowProjectFieldsWithCnfFilter() {
+    String sql = "select \"intField\", \"stringField\"\n"
+        + "from arrowdata\n"
+        + "where (\"intField\" > 1 and \"stringField\" = '2') or \"intField\" 
= 0";
+    String plan = "PLAN=ArrowToEnumerableConverter\n"
+        + "  ArrowProject(intField=[$0], stringField=[$1])\n"
+        + "    ArrowFilter(condition=[OR(AND(>($0, 1), =($1, '2')), =($0, 
0))])\n"
+        + "      ArrowTableScan(table=[[ARROW, ARROWDATA]], fields=[[0, 1, 2, 
3]])\n\n";
+    String result = "intField=0; stringField=0\n"
+        + "intField=2; stringField=2\n";
+
+    CalciteAssert.that()
+        .with(arrow)
+        .query(sql)
+        .returns(result)
+        .explainContains(plan);
+  }
+
   @Test void testArrowProjectFieldsWithInFilter() {
     String sql = "select \"intField\", \"stringField\"\n"
         + "from arrowdata\n"
         + "where \"intField\" in (0, 1, 2)";
-    String plan;
-    if (Bug.CALCITE_6294_FIXED) {
-      plan = "PLAN=ArrowToEnumerableConverter\n"
-          + "  ArrowProject(intField=[$0], stringField=[$1])\n"
-          + "    ArrowFilter(condition=[OR(=($0, 0), =($0, 1), =($0, 2))])\n"
-          + "      ArrowTableScan(table=[[ARROW, ARROWDATA]], fields=[[0, 1, 
2, 3]])\n\n";
-    } else {
-      plan = "PLAN=EnumerableCalc(expr#0..1=[{inputs}], expr#2=[Sarg[0, 1, 
2]], "
-          + "expr#3=[SEARCH($t0, $t2)], proj#0..1=[{exprs}], 
$condition=[$t3])\n"
-          + "  ArrowToEnumerableConverter\n"
-          + "    ArrowProject(intField=[$0], stringField=[$1])\n"
-          + "      ArrowTableScan(table=[[ARROW, ARROWDATA]], fields=[[0, 1, 
2, 3]])\n\n";
-    }
+    String plan = "PLAN=ArrowToEnumerableConverter\n"
+        + "  ArrowProject(intField=[$0], stringField=[$1])\n"
+        + "    ArrowFilter(condition=[SEARCH($0, Sarg[0, 1, 2])])\n"

Review Comment:
   Arrow supports search?



##########
arrow/src/main/java/org/apache/calcite/adapter/arrow/ArrowTable.java:
##########
@@ -184,6 +178,23 @@ private static RelDataType deduceRowType(Schema schema,
     return builder.build();
   }
 
+  private TreeNode parseSingleCondition(String condition) {

Review Comment:
   I don't really know the grammar for these conditions, so I cannot tell 
whether the spaces are where you expect them. Is this documented someplace?
   
   What happens if you have a comparison with a string containing a space, for 
example?



##########
core/src/main/java/org/apache/calcite/util/Bug.java:
##########
@@ -207,12 +207,12 @@ public abstract class Bug {
   /** Whether
    * <a 
href="https://issues.apache.org/jira/browse/CALCITE/issues/CALCITE-6293";>
    * [CALCITE-6293] Support OR condition in Arrow adapter</a> is fixed. */
-  public static final boolean CALCITE_6293_FIXED = false;
+  public static final boolean CALCITE_6293_FIXED = true;

Review Comment:
   Maybe these can be completely removed?



-- 
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]

Reply via email to