[GitHub] [calcite] zhuwenzhuang commented on a diff in pull request #3404: [CALCITE-5968] Provide an interface for RexExecutable to decouple janino runtime binding

["zhuwenzhuang (via GitHub)" <gi...@apache.org>]

zhuwenzhuang commented on code in PR #3404:
URL: https://github.com/apache/calcite/pull/3404#discussion_r1322281984


##########
core/src/test/java/org/apache/calcite/rex/RexExecutorTest.java:
##########
@@ -378,4 +383,140 @@ public void run() {
   interface Action {
     void check(RexBuilder rexBuilder, RexExecutorImpl executor);
   }
+
+  /**
+   * ArrayList-based DataContext to check Rex execution.
+   */
+  public static class TestDataContext extends SingleValueDataContext {
+    private TestDataContext(Object[] values) {
+      super("inputRecord", values);
+    }
+  }
+
+  /**
+   * Context that holds a value for a particular context name.
+   */
+  static class SingleValueDataContext implements DataContext {
+    private final String name;
+    private final Object value;
+
+    SingleValueDataContext(String name, Object value) {
+      this.name = name;
+      this.value = value;
+    }
+
+    public SchemaPlus getRootSchema() {
+      throw new RuntimeException("Unsupported");
+    }
+
+    public JavaTypeFactory getTypeFactory() {
+      throw new RuntimeException("Unsupported");
+    }
+
+    public QueryProvider getQueryProvider() {
+      throw new RuntimeException("Unsupported");
+    }
+
+    public Object get(String name) {
+      if (this.name.equals(name)) {
+        return value;
+      } else {
+        throw new RuntimeException("Wrong DataContext access");
+      }
+    }
+  }
+
+  /**
+   * User defined executable.
+   */
+  static class UserExecutable implements RexExecutable {
+    final RexBuilder rexBuilder;
+    final List<RexNode> exps;
+    DataContext dataContext;
+    UserExecutable(final RexBuilder rexBuilder, final List<RexNode> exps) {
+      assert exps != null && exps.size() == 1;
+      this.rexBuilder = rexBuilder;
+      this.exps = exps;
+    }
+    @Override public void reduce(RexBuilder rexBuilder, List<RexNode> constExps,
+        List<RexNode> reducedValues) {
+      Object[] values;
+      try {
+        values = execute();
+        assert values.length == constExps.size();
+        final List<Object> valueList = Arrays.asList(values);
+        for (Pair<RexNode, Object> value : Pair.zip(constExps, valueList)) {
+          reducedValues.add(
+              rexBuilder.makeLiteral(value.right, value.left.getType(), true));
+        }
+      } catch (RuntimeException e) {
+        // One or more of the expressions failed.
+        // Don't reduce any of the expressions.
+        reducedValues.addAll(constExps);
+      }
+    }
+    @Override public void setDataContext(DataContext dataContext) {
+      this.dataContext = dataContext;
+    }
+    @Override public Object[] execute() {
+      final RexNode exp = exps.get(0);
+      assert exp instanceof RexCall;
+      RexCall call = (RexCall) exp;
+      // NOTE: this isn't a correct substring function implementation, only for testing
+      if (!call.op.getName().equals("SUBSTRING")) {
+        throw new RuntimeException("not supported");
+      }
+      assert call.getOperands().get(0) instanceof RexInputRef;
+      assert call.getOperands().get(1) instanceof RexLiteral;
+      String str = (String) ((Object[]) dataContext.get("inputRecord"))[0];
+      Integer startIndex = ((RexLiteral) call.getOperands().get(1)).getValueAs(Integer.class);
+      Object[] values = new Object[1];
+      values[0] = str.substring(startIndex - 1);
+      return values;
+    }
+  }
+  /**
+   * User defined executor.
+   */
+  static class UserExecutor implements RexExecutor {
+    @Override public void reduce(RexBuilder rexBuilder, List<RexNode> constExps,
+        List<RexNode> reducedValues) {
+      this.getExecutable(rexBuilder, constExps, null).reduce(rexBuilder, constExps, reducedValues);
+    }
+    @Override public RexExecutable getExecutable(RexBuilder rexBuilder, List<RexNode> exps,
+        RelDataType rowType) {
+      return new UserExecutable(rexBuilder, exps);
+    }
+  }
+  @Test public void userExecutorTest() throws Exception {
+    check((rexBuilder, executor) -> {
+      Object[] values = new Object[1];
+      final DataContext testContext = new TestDataContext(values);
+      final RelDataTypeFactory typeFactory = rexBuilder.getTypeFactory();
+      final RelDataType varchar =
+          typeFactory.createSqlType(SqlTypeName.VARCHAR);
+      final RelDataType integer =
+          typeFactory.createSqlType(SqlTypeName.INTEGER);
+      // Calcite is internally creating the input ref via a RexRangeRef
+      // which eventually leads to a RexInputRef. So we are good.
+      final RexInputRef input = rexBuilder.makeInputRef(varchar, 0);
+      final RexNode lengthArg = rexBuilder.makeLiteral(3, integer, true);
+      final RexNode substr =
+          rexBuilder.makeCall(SqlStdOperatorTable.SUBSTRING, input,
+              lengthArg);
+      ImmutableList<RexNode> constExps = ImmutableList.of(substr);
+      final RelDataType rowType = typeFactory.builder()
+          .add("someStr", varchar)
+          .build();
+      RexExecutor useExecutor = new UserExecutor();
+      final RexExecutable exec =

Review Comment:
   Hi @LakeShen. 
   The userExecutorTest covered a user-defined RexExecutable/RexExecutor scenario. So, maybe adding more mock functions will not increase the code coverage.
   Please let me know which part of code requires additional coverage with test cases. Thank you.



-- 
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: commits-unsubscribe@calcite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org