Skip to content

Commit

Permalink
[CALCITE-4989] Nested JSON_OBJECT creation does not produce proper json
Browse files Browse the repository at this point in the history
fix checkstyle

make jdk11 checker happy
  • Loading branch information
libenchao authored and XuQianJin-Stars committed Mar 26, 2022
1 parent a6a1e2c commit 8844146
Show file tree
Hide file tree
Showing 11 changed files with 353 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@
import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_OBJECT;
import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_OBJECTAGG;
import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_QUERY;
import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_TYPE_OPERATOR;
import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_VALUE;
import static org.apache.calcite.sql.fun.SqlStdOperatorTable.JSON_VALUE_EXPRESSION;
import static org.apache.calcite.sql.fun.SqlStdOperatorTable.LAG;
Expand Down Expand Up @@ -577,6 +578,8 @@ public class RexImpTable {
// Json Operators
defineMethod(JSON_VALUE_EXPRESSION,
BuiltInMethod.JSON_VALUE_EXPRESSION.method, NullPolicy.STRICT);
defineMethod(JSON_TYPE_OPERATOR,
BuiltInMethod.JSON_VALUE_EXPRESSION.method, NullPolicy.STRICT);
defineMethod(JSON_EXISTS, BuiltInMethod.JSON_EXISTS.method, NullPolicy.ARG0);
map.put(JSON_VALUE,
new JsonValueImplementor(BuiltInMethod.JSON_VALUE.method));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -803,7 +803,7 @@ public boolean hasException() {
}

@Override public String toString() {
return Objects.toString(obj);
return jsonize(obj);
}
}

Expand Down
3 changes: 3 additions & 0 deletions core/src/main/java/org/apache/calcite/sql/SqlKind.java
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,9 @@ public enum SqlKind {
/** {@code JSON_OBJECTAGG} aggregate function. */
JSON_OBJECTAGG,

/** {@code JSON} type function. */
JSON_TYPE,

/** {@code UNNEST} operator. */
UNNEST,

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to you under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.calcite.sql.fun;

import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlInternalOperator;
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlWriter;
import org.apache.calcite.sql.type.OperandTypes;
import org.apache.calcite.sql.type.ReturnTypes;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.sql.type.SqlTypeTransforms;

/** Operator that indicates that the result is a json string. */
class SqlJsonTypeOperator extends SqlInternalOperator {
SqlJsonTypeOperator() {
super("JSON_TYPE", SqlKind.JSON_TYPE, MDX_PRECEDENCE, true,
ReturnTypes.explicit(SqlTypeName.ANY).andThen(SqlTypeTransforms.TO_NULLABLE),
null, OperandTypes.CHARACTER);
}

// This is an internal operator, which should not be unparsed to sql.
@Override public void unparse(SqlWriter writer, SqlCall call, int leftPrec, int rightPrec) {
call.operand(0).unparse(writer, leftPrec, rightPrec);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,9 @@ public class SqlStdOperatorTable extends ReflectiveSqlOperatorTable {
public static final SqlPostfixOperator JSON_VALUE_EXPRESSION =
new SqlJsonValueExpressionOperator();

public static final SqlJsonTypeOperator JSON_TYPE_OPERATOR =
new SqlJsonTypeOperator();


//-------------------------------------------------------------
// PREFIX OPERATORS
Expand Down
138 changes: 138 additions & 0 deletions core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.apache.calcite.rel.RelRoot;
import org.apache.calcite.rel.RelShuttleImpl;
import org.apache.calcite.rel.SingleRel;
import org.apache.calcite.rel.core.Aggregate;
import org.apache.calcite.rel.core.AggregateCall;
import org.apache.calcite.rel.core.Collect;
import org.apache.calcite.rel.core.CorrelationId;
Expand Down Expand Up @@ -615,6 +616,11 @@ public RelRoot convertQuery(
hints = SqlUtil.getRelHint(hintStrategies, select.getHints());
}
}

if (config.isAddJsonTypeOperatorEnabled()) {
result = result.accept(new NestedJsonFunctionRelRewriter());
}

// propagate the hints.
result = RelOptUtil.propagateRelHints(result, false);
return RelRoot.of(result, validatedRowType, query.getKind())
Expand Down Expand Up @@ -6416,6 +6422,138 @@ default Config addRelBuilderConfigTransform(

/** Sets {@link #getHintStrategyTable()}. */
Config withHintStrategyTable(HintStrategyTable hintStrategyTable);

/**
* Whether add {@link SqlStdOperatorTable#JSON_TYPE_OPERATOR} for between json functions.
*/
@Value.Default default boolean isAddJsonTypeOperatorEnabled() {
return true;
}

/** Sets {@link #isAddJsonTypeOperatorEnabled()}. */
Config withAddJsonTypeOperatorEnabled(boolean addJsonTypeOperatorEnabled);
}

/**
* Used to find nested json functions, and add {@link SqlStdOperatorTable#JSON_TYPE_OPERATOR}
* to nested json output.
*/
private class NestedJsonFunctionRelRewriter extends RelShuttleImpl {

@Override public RelNode visit(LogicalProject project) {
final Set<Integer> jsonInputFields = findJsonInputs(project.getInput());
final Set<Integer> requiredJsonFieldsFromParent = stack.size() > 0
? requiredJsonOutputFromParent(stack.getLast()) : Collections.emptySet();

final List<RexNode> originalProjections = project.getProjects();
final ImmutableList.Builder<RexNode> newProjections = ImmutableList.builder();
JsonFunctionRexRewriter rexRewriter = new JsonFunctionRexRewriter(jsonInputFields);
for (int i = 0; i < originalProjections.size(); ++i) {
if (requiredJsonFieldsFromParent.contains(i)) {
newProjections.add(rexRewriter.forceChildJsonType(originalProjections.get(i)));
} else {
newProjections.add(originalProjections.get(i).accept(rexRewriter));
}
}

RelNode newInput = project.getInput().accept(this);
return LogicalProject.create(
newInput,
project.getHints(),
newProjections.build(),
project.getRowType().getFieldNames());
}

private Set<Integer> requiredJsonOutputFromParent(RelNode relNode) {
if (!(relNode instanceof Aggregate)) {
return Collections.emptySet();
}
final Aggregate aggregate = (Aggregate) relNode;
final List<AggregateCall> aggregateCalls = aggregate.getAggCallList();
final ImmutableSet.Builder<Integer> result = ImmutableSet.builder();
for (final AggregateCall call : aggregateCalls) {
if (call.getAggregation() == SqlStdOperatorTable.JSON_OBJECTAGG) {
result.add(call.getArgList().get(1));
} else if (call.getAggregation() == SqlStdOperatorTable.JSON_ARRAYAGG) {
result.add(call.getArgList().get(0));
}
}
return result.build();
}

private Set<Integer> findJsonInputs(RelNode relNode) {
if (!(relNode instanceof Aggregate)) {
return Collections.emptySet();
}
final Aggregate aggregate = (Aggregate) relNode;
final List<AggregateCall> aggregateCalls = aggregate.getAggCallList();
final ImmutableSet.Builder<Integer> result = ImmutableSet.builder();
for (int i = 0; i < aggregateCalls.size(); ++i) {
final AggregateCall call = aggregateCalls.get(i);
if (call.getAggregation() == SqlStdOperatorTable.JSON_OBJECTAGG
|| call.getAggregation() == SqlStdOperatorTable.JSON_ARRAYAGG) {
result.add(aggregate.getGroupCount() + i);
}
}
return result.build();
}
}

/**
* Used to rewrite json functions which is nested.
*/
private class JsonFunctionRexRewriter extends RexShuttle {

private final Set<Integer> jsonInputFields;

JsonFunctionRexRewriter(Set<Integer> jsonInputFields) {
this.jsonInputFields = jsonInputFields;
}

@Override public RexNode visitCall(RexCall call) {
if (call.getOperator() == SqlStdOperatorTable.JSON_OBJECT) {
final ImmutableList.Builder<RexNode> builder = ImmutableList.builder();
for (int i = 0; i < call.operands.size(); ++i) {
if ((i & 1) == 0 && i != 0) {
builder.add(forceChildJsonType(call.operands.get(i)));
} else {
builder.add(call.operands.get(i));
}
}
return rexBuilder.makeCall(SqlStdOperatorTable.JSON_OBJECT, builder.build());
}
if (call.getOperator() == SqlStdOperatorTable.JSON_ARRAY) {
final ImmutableList.Builder<RexNode> builder = ImmutableList.builder();
builder.add(call.operands.get(0));
for (int i = 1; i < call.operands.size(); ++i) {
builder.add(forceChildJsonType(call.operands.get(i)));
}
return rexBuilder.makeCall(SqlStdOperatorTable.JSON_ARRAY, builder.build());
}
return super.visitCall(call);
}

private RexNode forceChildJsonType(RexNode rexNode) {
final RexNode childResult = rexNode.accept(this);
if (isJsonResult(rexNode)) {
return rexBuilder.makeCall(SqlStdOperatorTable.JSON_TYPE_OPERATOR, childResult);
}
return childResult;
}

private boolean isJsonResult(RexNode rexNode) {
if (rexNode instanceof RexCall) {
final RexCall call = (RexCall) rexNode;
final SqlOperator operator = call.getOperator();
return operator == SqlStdOperatorTable.JSON_OBJECT
|| operator == SqlStdOperatorTable.JSON_ARRAY
|| operator == SqlStdOperatorTable.JSON_VALUE;
} else if (rexNode instanceof RexInputRef) {
final RexInputRef inputRef = (RexInputRef) rexNode;
return jsonInputFields.contains(inputRef.getIndex());
}
return false;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -425,14 +425,6 @@ class SqlJsonFunctionsTest {
JsonFunctions.JsonValueContext.withJavaObj(new HashMap<>()), is("{ }"));
assertJsonPretty(
JsonFunctions.JsonValueContext.withJavaObj(Longs.asList(1, 2)), is("[ 1, 2 ]"));

Object input = new Object() {
private final Object self = this;
};
CalciteException expected = new CalciteException(
"Cannot serialize object to JSON: '" + input + "'", null);
assertJsonPrettyFailed(
JsonFunctions.JsonValueContext.withJavaObj(input), errorMatches(expected));
}

@Test void testDejsonize() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4019,6 +4019,58 @@ void checkCorrelatedMapSubQuery(boolean expand) {
sql(sql).ok();
}

@Test void testJsonNestedJsonObjectConstructor() {
final String sql = "select\n"
+ "json_object(\n"
+ " 'key1' :\n"
+ " json_object(\n"
+ " 'key2' :\n"
+ " ename)),\n"
+ " json_object(\n"
+ " 'key3' :\n"
+ " json_array(12, 'hello', deptno))\n"
+ "from emp";
sql(sql).ok();
}

@Test void testJsonNestedJsonArrayConstructor() {
final String sql = "select\n"
+ "json_array(\n"
+ " json_object(\n"
+ " 'key1' :\n"
+ " json_object(\n"
+ " 'key2' :\n"
+ " ename)),\n"
+ " json_array(12, 'hello', deptno))\n"
+ "from emp";
sql(sql).ok();
}

@Test void testJsonNestedJsonObjectAggConstructor() {
final String sql = "select\n"
+ "json_object(\n"
+ " 'k2' :\n"
+ " json_objectagg(\n"
+ " ename :\n"
+ " json_object(\n"
+ " 'k1' :\n"
+ " deptno)))\n"
+ "from emp";
sql(sql).ok();
}

@Test void testJsonNestedJsonArrayAggConstructor() {
final String sql = "select\n"
+ "json_object(\n"
+ " 'k2' :\n"
+ " json_arrayagg(\n"
+ " json_object(\n"
+ " ename :\n"
+ " deptno)))\n"
+ "from emp";
sql(sql).ok();
}

@Test void testWithinGroup1() {
final String sql = "select deptno,\n"
+ " collect(empno) within group (order by deptno, hiredate desc)\n"
Expand Down
Loading

0 comments on commit 8844146

Please sign in to comment.