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


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -4741,6 +4743,47 @@ private static class IsNullImplementor extends 
AbstractRexCallImplementor {
     }
   }
 
+  /** Implementor for the {@code IS NOT DISTINCT FROM} SQL operator. */
+  private static class IsNotDistinctFromImplementor extends 
AbstractRexCallImplementor {
+    IsNotDistinctFromImplementor() {
+      super("is_not_distinct_from", NullPolicy.NONE, false);
+    }
+
+    @Override public RexToLixTranslator.Result implement(final 
RexToLixTranslator translator,
+        final RexCall call, final List<RexToLixTranslator.Result> arguments) {
+      final RexToLixTranslator.Result left = arguments.get(0);

Review Comment:
   can you add a comment showing the generated java code?



##########
core/src/main/java/org/apache/calcite/rel/core/JoinInfo.java:
##########
@@ -62,10 +62,9 @@ protected JoinInfo(ImmutableIntList leftKeys, 
ImmutableIntList rightKeys,
   public static JoinInfo of(RelNode left, RelNode right, RexNode condition) {
     final List<Integer> leftKeys = new ArrayList<>();
     final List<Integer> rightKeys = new ArrayList<>();
-    final List<Boolean> filterNulls = new ArrayList<>();
     final List<RexNode> nonEquiList = new ArrayList<>();
     RelOptUtil.splitJoinCondition(left, right, condition, leftKeys, rightKeys,
-        filterNulls, nonEquiList);
+        null, nonEquiList);

Review Comment:
   is this change needed?



##########
core/src/test/java/org/apache/calcite/test/JdbcTest.java:
##########
@@ -3950,6 +3950,24 @@ public void checkOrderBy(final boolean desc,
         .returnsUnordered("empid=150; name=Sebastian");
   }
 
+  /** Test case for
+   * <a 
href="https://issues.apache.org/jira/browse/CALCITE-6904";>[CALCITE-6904]
+   * IS_NOT_DISTINCT_FROM is converted error in EnumerableJoinRule</a>. */
+  @Test void testIsNotDistinctFrom() {

Review Comment:
   Can you also add a quidem test in one .iq file?
   



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -425,6 +425,7 @@
 import static org.apache.calcite.sql.fun.SqlStdOperatorTable.IS_JSON_SCALAR;
 import static org.apache.calcite.sql.fun.SqlStdOperatorTable.IS_JSON_VALUE;
 import static org.apache.calcite.sql.fun.SqlStdOperatorTable.IS_NOT_A_SET;
+import static 
org.apache.calcite.sql.fun.SqlStdOperatorTable.IS_NOT_DISTINCT_FROM;

Review Comment:
   what about IS_DISTINCT_FROM?
   Does that need an implementor, or is it converted to IS_NOT_DISTINCT_FROM?



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -4741,6 +4743,47 @@ private static class IsNullImplementor extends 
AbstractRexCallImplementor {
     }
   }
 
+  /** Implementor for the {@code IS NOT DISTINCT FROM} SQL operator. */
+  private static class IsNotDistinctFromImplementor extends 
AbstractRexCallImplementor {
+    IsNotDistinctFromImplementor() {
+      super("is_not_distinct_from", NullPolicy.NONE, false);
+    }
+
+    @Override public RexToLixTranslator.Result implement(final 
RexToLixTranslator translator,
+        final RexCall call, final List<RexToLixTranslator.Result> arguments) {
+      final RexToLixTranslator.Result left = arguments.get(0);
+      final RexToLixTranslator.Result right = arguments.get(1);
+
+      final Expression valueExpression =
+          Expressions.condition(left.isNullVariable,
+          Expressions.condition(right.isNullVariable, BOXED_TRUE_EXPR, 
BOXED_FALSE_EXPR),
+          Expressions.condition(right.isNullVariable, BOXED_FALSE_EXPR,
+              Expressions.equal(left.valueVariable, right.valueVariable)));
+
+      BlockBuilder builder = translator.getBlockBuilder();
+      final ParameterExpression valueVariable =
+          Expressions.parameter(valueExpression.getType(),
+              builder.newName(variableName + "_value"));
+      final Expression isNullExpression = FALSE_EXPR;
+      final ParameterExpression isNullVariable =
+          Expressions.parameter(Boolean.TYPE,
+              builder.newName(variableName + "_isNull"));
+
+      builder.add(
+          Expressions.declare(Modifier.FINAL, valueVariable, valueExpression));
+      builder.add(
+          Expressions.declare(Modifier.FINAL, isNullVariable, 
isNullExpression));
+
+      return new RexToLixTranslator.Result(isNullVariable, valueVariable);
+    }
+
+    @Override Expression implementSafe(final RexToLixTranslator translator,
+        final RexCall call, final List<Expression> argValueList) {
+      throw new IllegalStateException("This implementSafe should not be 
called,"
+          + " please call implement(...)");

Review Comment:
   I am not sure this error message is helpful - who is the "please" for?
   



##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -4741,6 +4743,47 @@ private static class IsNullImplementor extends 
AbstractRexCallImplementor {
     }
   }
 
+  /** Implementor for the {@code IS NOT DISTINCT FROM} SQL operator. */
+  private static class IsNotDistinctFromImplementor extends 
AbstractRexCallImplementor {
+    IsNotDistinctFromImplementor() {
+      super("is_not_distinct_from", NullPolicy.NONE, false);
+    }
+
+    @Override public RexToLixTranslator.Result implement(final 
RexToLixTranslator translator,
+        final RexCall call, final List<RexToLixTranslator.Result> arguments) {
+      final RexToLixTranslator.Result left = arguments.get(0);
+      final RexToLixTranslator.Result right = arguments.get(1);
+
+      final Expression valueExpression =
+          Expressions.condition(left.isNullVariable,
+          Expressions.condition(right.isNullVariable, BOXED_TRUE_EXPR, 
BOXED_FALSE_EXPR),
+          Expressions.condition(right.isNullVariable, BOXED_FALSE_EXPR,
+              Expressions.equal(left.valueVariable, right.valueVariable)));
+
+      BlockBuilder builder = translator.getBlockBuilder();
+      final ParameterExpression valueVariable =
+          Expressions.parameter(valueExpression.getType(),
+              builder.newName(variableName + "_value"));
+      final Expression isNullExpression = FALSE_EXPR;
+      final ParameterExpression isNullVariable =

Review Comment:
   is this declaration necessary or can you just use FALSE_EXPR below?



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