julianhyde commented on a change in pull request #2709:
URL: https://github.com/apache/calcite/pull/2709#discussion_r798930442
##########
File path: core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java
##########
@@ -776,4 +905,22 @@ private void addRexFieldCollationList(
map.put("syntax", operator.getSyntax().toString());
return map;
}
+
+ /**
Review comment:
I wouldn't mention the 'functional interface'. Document its purpose,
which is to translate.
Rename `apply` to something like `translate`. `apply` only made sense when
this was a `BiFunction`.
In the method javadoc change `Lambda that defines how to transform` to
`Transforms`. Less is more.
Rename `stringObjectMap` to `map`.
##########
File path: core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java
##########
@@ -99,7 +102,143 @@
"org.apache.calcite.adapter.jdbc.JdbcRules$");
public RelJson(@Nullable JsonBuilder jsonBuilder) {
+ this(jsonBuilder, RelJson::inputTranslatorImpl);
+ }
+
+ private static RexNode inputTranslatorImpl(
+ Map<String, Object> stringObjectMap,
+ RexBuilder rexBuilder,
+ List<RelNode> relNodes) {
+ final Integer input = (Integer) stringObjectMap.get("input");
+ if (input != null) {
+ int i = input;
+ for (RelNode inputNode : relNodes) {
+ final RelDataType rowType = inputNode.getRowType();
+ if (i < rowType.getFieldCount()) {
+ final RelDataTypeField field = rowType.getFieldList().get(i);
+ return rexBuilder.makeInputRef(field.getType(), input);
+ }
+ i -= rowType.getFieldCount();
+ }
+ throw new RuntimeException("input field " + input + " is out of range");
+ } else {
+ throw new RuntimeException("input not defined");
+ }
+ }
+
+ public RelJson(@Nullable JsonBuilder jsonBuilder, InputTranslator
inputTranslator) {
this.jsonBuilder = jsonBuilder;
+ this.inputTranslator = inputTranslator;
+ }
+
+ /**
+ * Transforms a RexNode tree defined in a map (from a JSON) into a RexNode,
+ * applying a special method to inputs instead of transforming them into
inputRef.
+ * @param cluster The optimization environment
+ * @param apply is a InputTranslator lambda that transforms the map
representing input
Review comment:
rename `apply` parameter to `translator`
##########
File path: core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java
##########
@@ -99,7 +102,143 @@
"org.apache.calcite.adapter.jdbc.JdbcRules$");
public RelJson(@Nullable JsonBuilder jsonBuilder) {
+ this(jsonBuilder, RelJson::inputTranslatorImpl);
+ }
+
+ private static RexNode inputTranslatorImpl(
+ Map<String, Object> stringObjectMap,
+ RexBuilder rexBuilder,
+ List<RelNode> relNodes) {
+ final Integer input = (Integer) stringObjectMap.get("input");
+ if (input != null) {
+ int i = input;
+ for (RelNode inputNode : relNodes) {
+ final RelDataType rowType = inputNode.getRowType();
+ if (i < rowType.getFieldCount()) {
+ final RelDataTypeField field = rowType.getFieldList().get(i);
+ return rexBuilder.makeInputRef(field.getType(), input);
+ }
+ i -= rowType.getFieldCount();
+ }
+ throw new RuntimeException("input field " + input + " is out of range");
+ } else {
+ throw new RuntimeException("input not defined");
+ }
+ }
+
+ public RelJson(@Nullable JsonBuilder jsonBuilder, InputTranslator
inputTranslator) {
this.jsonBuilder = jsonBuilder;
+ this.inputTranslator = inputTranslator;
+ }
+
+ /**
+ * Transforms a RexNode tree defined in a map (from a JSON) into a RexNode,
+ * applying a special method to inputs instead of transforming them into
inputRef.
+ * @param cluster The optimization environment
+ * @param apply is a InputTranslator lambda that transforms the map
representing input
+ * references into a RexNode
+ * @param o the map derived from a RexNode transformed into a JSON
+ * @return the transformed RexNode
+ */
+ public static RexNode readExpression(
Review comment:
Let's move this method a bit further down the file. It's not the most
important method in this class.
The description 'Transforms a RexNode tree' is a bit misleading, given that
the input is JSON not RexNode. Maybe something like 'Converts a JSON
expression...'?
Convert the anonymous inner class that implements RelInput into a private
static inner class. That gives us an opportunity to document it. Also it will
make it easier to refactor in future.
##########
File path: core/src/test/java/org/apache/calcite/plan/RelWriterTest.java
##########
@@ -706,6 +717,51 @@
+ " LogicalTableScan(table=[[hr, emps]])\n"));
}
+ @Test void testJsonToRex() throws JsonProcessingException {
+ // Test simple literal without inputs
+ final String jsonString1 = "{\n"
+ + " \"literal\": 10,\n"
+ + " \"type\": {\n"
+ + " \"type\": \"INTEGER\",\n"
+ + " \"nullable\": false\n"
+ + " }\n"
+ + " }\n";
+
+ assertReadRex(jsonString1, "10");
+
+ // Test Binary with an input
+ final String jsonString2 = "{ \"op\": \n"
+ + " { \"name\": \"+\",\n"
+ + " \"kind\": \"PLUS\",\n"
+ + " \"syntax\": \"BINARY\"\n"
+ + " },\n"
+ + " \"operands\": [\n"
+ + " {\n"
+ + " \"input\": 1,\n"
+ + " \"sql\": \"column + 1\"\n"
+ + " },\n"
+ + " {\n"
+ + " \"literal\": 1,\n"
+ + " \"type\": { \"type\": \"INTEGER\", \"nullable\": false
}\n"
+ + " }\n"
+ + " ]\n"
+ + " }";
+ assertReadRex(jsonString2, "+(1, 1)");
+ }
+
+ private void assertReadRex(String jsonString1, String expected)
Review comment:
better name would be `assertThatReadExpressionResult`
change parameter to from `String` to `Matcher<String>`. then there will be
an `is` at the call site, and the code will read somewhat like English (per
junit best practices, e.g. `assertThat(1 + 2, is(3));`
##########
File path: core/src/test/java/org/apache/calcite/plan/RelWriterTest.java
##########
@@ -429,6 +435,11 @@
return Stream.of(SqlExplainFormat.TEXT, SqlExplainFormat.DOT);
}
+ private static RexNode apply(Map<String, Object> map, RexBuilder rexBuilder,
List<RelNode> inputs) {
Review comment:
need better name than `apply`. Also a bit of javadoc describing intend.
In the javadoc I'd mention that it is intended to implement the translator
interface.
--
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]
