This is an automated email from the ASF dual-hosted git repository.
hongze pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/master by this push:
new 5ada462 [CALCITE-2787] Json aggregate calls with different null
clause get incorrectly merged during converting from SQL to relational algebra
5ada462 is described below
commit 5ada462e6f792deee7185ad2df8ec58380061d59
Author: Hongze Zhang <[email protected]>
AuthorDate: Fri Feb 22 22:32:24 2019 +0800
[CALCITE-2787] Json aggregate calls with different null clause get
incorrectly merged during converting from SQL to relational algebra
---
.../calcite/adapter/enumerable/RexImpTable.java | 13 +++-
.../org/apache/calcite/runtime/SqlFunctions.java | 3 +-
.../sql/fun/SqlJsonArrayAggAggFunction.java | 11 +---
.../sql/fun/SqlJsonObjectAggAggFunction.java | 11 +---
.../calcite/sql/fun/SqlStdOperatorTable.java | 6 +-
.../apache/calcite/test/SqlToRelConverterTest.xml | 10 +--
core/src/test/resources/sql/agg.iq | 77 +++++++++++++++++++++-
7 files changed, 100 insertions(+), 31 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
index a8112d2..e44af2b 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
@@ -44,6 +44,7 @@ import org.apache.calcite.schema.ImplementableFunction;
import org.apache.calcite.schema.impl.AggregateFunctionImpl;
import org.apache.calcite.sql.SqlAggFunction;
import org.apache.calcite.sql.SqlBinaryOperator;
+import org.apache.calcite.sql.SqlJsonConstructorNullClause;
import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.fun.SqlJsonArrayAggAggFunction;
import org.apache.calcite.sql.fun.SqlJsonObjectAggAggFunction;
@@ -452,13 +453,19 @@ public class RexImpTable {
defineMethod(JSON_VALUE_ANY, BuiltInMethod.JSON_VALUE_ANY.method,
NullPolicy.NONE);
defineMethod(JSON_QUERY, BuiltInMethod.JSON_QUERY.method, NullPolicy.NONE);
defineMethod(JSON_OBJECT, BuiltInMethod.JSON_OBJECT.method,
NullPolicy.NONE);
+ defineMethod(JSON_ARRAY, BuiltInMethod.JSON_ARRAY.method, NullPolicy.NONE);
defineMethod(JSON_TYPE, BuiltInMethod.JSON_TYPE.method, NullPolicy.NONE);
defineMethod(JSON_DEPTH, BuiltInMethod.JSON_DEPTH.method, NullPolicy.NONE);
- aggMap.put(JSON_OBJECTAGG,
+
aggMap.put(JSON_OBJECTAGG.with(SqlJsonConstructorNullClause.ABSENT_ON_NULL),
JsonObjectAggImplementor
.supplierFor(BuiltInMethod.JSON_OBJECTAGG_ADD.method));
- defineMethod(JSON_ARRAY, BuiltInMethod.JSON_ARRAY.method, NullPolicy.NONE);
- aggMap.put(JSON_ARRAYAGG,
+ aggMap.put(JSON_OBJECTAGG.with(SqlJsonConstructorNullClause.NULL_ON_NULL),
+ JsonObjectAggImplementor
+ .supplierFor(BuiltInMethod.JSON_OBJECTAGG_ADD.method));
+ aggMap.put(JSON_ARRAYAGG.with(SqlJsonConstructorNullClause.ABSENT_ON_NULL),
+ JsonArrayAggImplementor
+ .supplierFor(BuiltInMethod.JSON_ARRAYAGG_ADD.method));
+ aggMap.put(JSON_ARRAYAGG.with(SqlJsonConstructorNullClause.NULL_ON_NULL),
JsonArrayAggImplementor
.supplierFor(BuiltInMethod.JSON_ARRAYAGG_ADD.method));
defineImplementor(IS_JSON_VALUE, NullPolicy.NONE,
diff --git a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
index 2c35aba..fed1d97 100644
--- a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
+++ b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
@@ -73,7 +73,6 @@ import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
-import java.util.Objects;
import java.util.Queue;
import java.util.Set;
import java.util.TimeZone;
@@ -2487,7 +2486,7 @@ public class SqlFunctions {
errorBehavior.toString()).ex();
}
} else {
- return !Objects.isNull(context.pathReturned);
+ return context.pathReturned != null;
}
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/fun/SqlJsonArrayAggAggFunction.java
b/core/src/main/java/org/apache/calcite/sql/fun/SqlJsonArrayAggAggFunction.java
index 0569bea..6fcbc3f 100644
---
a/core/src/main/java/org/apache/calcite/sql/fun/SqlJsonArrayAggAggFunction.java
+++
b/core/src/main/java/org/apache/calcite/sql/fun/SqlJsonArrayAggAggFunction.java
@@ -34,7 +34,6 @@ import org.apache.calcite.sql.validate.SqlValidatorImpl;
import org.apache.calcite.sql.validate.SqlValidatorScope;
import org.apache.calcite.util.Optionality;
-import java.util.Locale;
import java.util.Objects;
/**
@@ -43,9 +42,9 @@ import java.util.Objects;
public class SqlJsonArrayAggAggFunction extends SqlAggFunction {
private final SqlJsonConstructorNullClause nullClause;
- public SqlJsonArrayAggAggFunction(String name,
+ public SqlJsonArrayAggAggFunction(SqlKind kind,
SqlJsonConstructorNullClause nullClause) {
- super(name, null, SqlKind.JSON_ARRAYAGG, ReturnTypes.VARCHAR_2000, null,
+ super(kind + "_" + nullClause.name(), null, kind,
ReturnTypes.VARCHAR_2000, null,
OperandTypes.family(SqlTypeFamily.ANY), SqlFunctionCategory.SYSTEM,
false, false, Optionality.OPTIONAL);
this.nullClause = Objects.requireNonNull(nullClause);
@@ -70,10 +69,6 @@ public class SqlJsonArrayAggAggFunction extends
SqlAggFunction {
return validateOperands(validator, scope, call);
}
- @Override public String toString() {
- return getName() + String.format(Locale.ROOT, "<%s>", nullClause);
- }
-
@Override public SqlCall createCall(SqlLiteral functionQualifier,
SqlParserPos pos, SqlNode... operands) {
assert operands.length == 1 || operands.length == 2;
@@ -95,7 +90,7 @@ public class SqlJsonArrayAggAggFunction extends
SqlAggFunction {
public SqlJsonArrayAggAggFunction with(SqlJsonConstructorNullClause
nullClause) {
return this.nullClause == nullClause ? this
- : new SqlJsonArrayAggAggFunction(getName(), nullClause);
+ : new SqlJsonArrayAggAggFunction(getKind(), nullClause);
}
public SqlJsonConstructorNullClause getNullClause() {
diff --git
a/core/src/main/java/org/apache/calcite/sql/fun/SqlJsonObjectAggAggFunction.java
b/core/src/main/java/org/apache/calcite/sql/fun/SqlJsonObjectAggAggFunction.java
index d8b85c7..3955e0d 100644
---
a/core/src/main/java/org/apache/calcite/sql/fun/SqlJsonObjectAggAggFunction.java
+++
b/core/src/main/java/org/apache/calcite/sql/fun/SqlJsonObjectAggAggFunction.java
@@ -32,7 +32,6 @@ import org.apache.calcite.sql.validate.SqlValidatorImpl;
import org.apache.calcite.sql.validate.SqlValidatorScope;
import org.apache.calcite.util.Optionality;
-import java.util.Locale;
import java.util.Objects;
/**
@@ -42,9 +41,9 @@ public class SqlJsonObjectAggAggFunction extends
SqlAggFunction {
private final SqlJsonConstructorNullClause nullClause;
/** Creates a SqlJsonObjectAggAggFunction. */
- public SqlJsonObjectAggAggFunction(String name,
+ public SqlJsonObjectAggAggFunction(SqlKind kind,
SqlJsonConstructorNullClause nullClause) {
- super(name, null, SqlKind.JSON_OBJECTAGG, ReturnTypes.VARCHAR_2000, null,
+ super(kind + "_" + nullClause.name(), null, kind,
ReturnTypes.VARCHAR_2000, null,
OperandTypes.family(SqlTypeFamily.CHARACTER, SqlTypeFamily.ANY),
SqlFunctionCategory.SYSTEM, false, false, Optionality.FORBIDDEN);
this.nullClause = Objects.requireNonNull(nullClause);
@@ -72,13 +71,9 @@ public class SqlJsonObjectAggAggFunction extends
SqlAggFunction {
return validateOperands(validator, scope, call);
}
- @Override public String toString() {
- return getName() + String.format(Locale.ROOT, "<%s>", nullClause);
- }
-
public SqlJsonObjectAggAggFunction with(SqlJsonConstructorNullClause
nullClause) {
return this.nullClause == nullClause ? this
- : new SqlJsonObjectAggAggFunction(getName(), nullClause);
+ : new SqlJsonObjectAggAggFunction(getKind(), nullClause);
}
public SqlJsonConstructorNullClause getNullClause() {
diff --git
a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
index 2e5b66b..5843835 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
@@ -1309,14 +1309,14 @@ public class SqlStdOperatorTable extends
ReflectiveSqlOperatorTable {
public static final SqlFunction JSON_DEPTH = new SqlJsonDepthFunction();
public static final SqlJsonObjectAggAggFunction JSON_OBJECTAGG =
- new SqlJsonObjectAggAggFunction("JSON_OBJECTAGG",
+ new SqlJsonObjectAggAggFunction(SqlKind.JSON_OBJECTAGG,
SqlJsonConstructorNullClause.NULL_ON_NULL);
public static final SqlFunction JSON_ARRAY = new SqlJsonArrayFunction();
public static final SqlJsonArrayAggAggFunction JSON_ARRAYAGG =
- new SqlJsonArrayAggAggFunction("JSON_ARRAYAGG",
- SqlJsonConstructorNullClause.NULL_ON_NULL);
+ new SqlJsonArrayAggAggFunction(SqlKind.JSON_ARRAYAGG,
+ SqlJsonConstructorNullClause.ABSENT_ON_NULL);
public static final SqlBetweenOperator BETWEEN =
new SqlBetweenOperator(
diff --git
a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
index 5c670bf..23367d9 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -5369,7 +5369,7 @@ from emp]]>
</Resource>
<Resource name="plan">
<![CDATA[
-LogicalAggregate(group=[{}], EXPR$0=[JSON_ARRAYAGG<ABSENT_ON_NULL>($0)])
+LogicalAggregate(group=[{}], EXPR$0=[JSON_ARRAYAGG_ABSENT_ON_NULL($0)])
LogicalProject($f0=[JSON_STRUCTURED_VALUE_EXPRESSION($1)])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
@@ -5382,7 +5382,7 @@ from emp]]>
</Resource>
<Resource name="plan">
<![CDATA[
-LogicalAggregate(group=[{}], EXPR$0=[JSON_ARRAYAGG<ABSENT_ON_NULL>($0) WITHIN
GROUP ([1])])
+LogicalAggregate(group=[{}], EXPR$0=[JSON_ARRAYAGG_ABSENT_ON_NULL($0) WITHIN
GROUP ([1])])
LogicalProject($f0=[JSON_STRUCTURED_VALUE_EXPRESSION($1)], ENAME=[$1])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
@@ -5395,7 +5395,7 @@ from emp]]>
</Resource>
<Resource name="plan">
<![CDATA[
-LogicalAggregate(group=[{}], EXPR$0=[JSON_ARRAYAGG<NULL_ON_NULL>($0) WITHIN
GROUP ([1])])
+LogicalAggregate(group=[{}], EXPR$0=[JSON_ARRAYAGG_NULL_ON_NULL($0) WITHIN
GROUP ([1])])
LogicalProject($f0=[JSON_STRUCTURED_VALUE_EXPRESSION($1)], ENAME=[$1])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
@@ -5408,7 +5408,7 @@ from emp]]>
</Resource>
<Resource name="plan">
<![CDATA[
-LogicalAggregate(group=[{}], EXPR$0=[JSON_ARRAYAGG<NULL_ON_NULL>($0) WITHIN
GROUP ([1])])
+LogicalAggregate(group=[{}], EXPR$0=[JSON_ARRAYAGG_NULL_ON_NULL($0) WITHIN
GROUP ([1])])
LogicalProject($f0=[JSON_STRUCTURED_VALUE_EXPRESSION($1)], ENAME=[$1])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
@@ -5433,7 +5433,7 @@ from emp]]>
</Resource>
<Resource name="plan">
<![CDATA[
-LogicalAggregate(group=[{}], EXPR$0=[JSON_OBJECTAGG<NULL_ON_NULL>($0, $1)])
+LogicalAggregate(group=[{}], EXPR$0=[JSON_OBJECTAGG_NULL_ON_NULL($0, $1)])
LogicalProject(ENAME=[$1], $f1=[JSON_STRUCTURED_VALUE_EXPRESSION($7)])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
diff --git a/core/src/test/resources/sql/agg.iq
b/core/src/test/resources/sql/agg.iq
index a1b52cd..83dcbca 100644
--- a/core/src/test/resources/sql/agg.iq
+++ b/core/src/test/resources/sql/agg.iq
@@ -2528,7 +2528,6 @@ select deptno, bit_and(empno), bit_or(empno) from
"scott".emp group by deptno;
!ok
# [CALCITE-2266] JSON_OBJECTAGG, JSON_ARRAYAGG
-
!use post
select gender, json_objectagg(ename: deptno absent on null) from emp group by
gender;
@@ -2582,7 +2581,7 @@ from emp group by gender;
(2 rows)
!ok
-EnumerableAggregate(group=[{0}], EXPR$1=[JSON_ARRAYAGG<ABSENT_ON_NULL>($1)
WITHIN GROUP ([2])], EXPR$2=[JSON_ARRAYAGG<ABSENT_ON_NULL>($1) WITHIN GROUP ([2
DESC])])
+EnumerableAggregate(group=[{0}], EXPR$1=[JSON_ARRAYAGG_ABSENT_ON_NULL($1)
WITHIN GROUP ([2])], EXPR$2=[JSON_ARRAYAGG_ABSENT_ON_NULL($1) WITHIN GROUP ([2
DESC])])
EnumerableCalc(expr#0..1=[{inputs}],
expr#2=[JSON_STRUCTURED_VALUE_EXPRESSION($t0)], GENDER=[$t1], $f1=[$t2],
DEPTNO=[$t0])
EnumerableUnion(all=[true])
EnumerableCalc(expr#0=[{inputs}], expr#1=[10], expr#2=['F'],
EXPR$1=[$t1], EXPR$2=[$t2])
@@ -2605,4 +2604,78 @@ EnumerableAggregate(group=[{0}],
EXPR$1=[JSON_ARRAYAGG<ABSENT_ON_NULL>($1) WITHI
EnumerableValues(tuples=[[{ 0 }]])
!plan
+# [CALCITE-2787] Json aggregate calls with different null clause get
incorrectly merged
+# during converting from SQL to relational algebra
+select gender,
+json_arrayagg(deptno),
+json_arrayagg(deptno null on null)
+from emp group by gender;
++--------+------------------+-----------------------+
+| GENDER | EXPR$1 | EXPR$2 |
++--------+------------------+-----------------------+
+| F | [10,30,30,50,60] | [10,30,30,50,60,null] |
+| M | [10,20,50] | [10,20,50] |
++--------+------------------+-----------------------+
+(2 rows)
+
+!ok
+EnumerableAggregate(group=[{0}], EXPR$1=[JSON_ARRAYAGG_ABSENT_ON_NULL($1)],
EXPR$2=[JSON_ARRAYAGG_NULL_ON_NULL($1)])
+ EnumerableCalc(expr#0..1=[{inputs}],
expr#2=[JSON_STRUCTURED_VALUE_EXPRESSION($t0)], GENDER=[$t1], $f1=[$t2])
+ EnumerableUnion(all=[true])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=[10], expr#2=['F'],
EXPR$1=[$t1], EXPR$2=[$t2])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=[10], expr#2=['M'],
EXPR$1=[$t1], EXPR$2=[$t2])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=[20], expr#2=['M'],
EXPR$1=[$t1], EXPR$2=[$t2])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=[30], expr#2=['F'],
EXPR$1=[$t1], EXPR$2=[$t2])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=[30], expr#2=['F'],
EXPR$1=[$t1], EXPR$2=[$t2])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=[50], expr#2=['M'],
EXPR$1=[$t1], EXPR$2=[$t2])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=[50], expr#2=['F'],
EXPR$1=[$t1], EXPR$2=[$t2])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=[60], expr#2=['F'],
EXPR$1=[$t1], EXPR$2=[$t2])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=[null:INTEGER], expr#2=['F'],
EXPR$1=[$t1], EXPR$2=[$t2])
+ EnumerableValues(tuples=[[{ 0 }]])
+!plan
+
+select gender,
+json_objectagg(ename: deptno),
+json_objectagg(ename: deptno absent on null)
+from emp group by gender;
++--------+--------------------------------------------------------------------+-------------------------------------------------------+
+| GENDER | EXPR$1
| EXPR$2 |
++--------+--------------------------------------------------------------------+-------------------------------------------------------+
+| F | {"Eve":50,"Grace":60,"Wilma":null,"Susan":30,"Alice":30,"Jane":10}
| {"Eve":50,"Grace":60,"Susan":30,"Alice":30,"Jane":10} |
+| M | {"Adam":50,"Bob":10,"Eric":20}
| {"Adam":50,"Bob":10,"Eric":20} |
++--------+--------------------------------------------------------------------+-------------------------------------------------------+
+(2 rows)
+
+!ok
+EnumerableAggregate(group=[{0}], EXPR$1=[JSON_OBJECTAGG_NULL_ON_NULL($1, $2)],
EXPR$2=[JSON_OBJECTAGG_ABSENT_ON_NULL($1, $2)])
+ EnumerableCalc(expr#0..2=[{inputs}],
expr#3=[JSON_STRUCTURED_VALUE_EXPRESSION($t1)], GENDER=[$t2], ENAME=[$t0],
$f2=[$t3])
+ EnumerableUnion(all=[true])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=['Jane'], expr#2=[10],
expr#3=['F'], EXPR$0=[$t1], EXPR$1=[$t2], EXPR$2=[$t3])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=['Bob'], expr#2=[10],
expr#3=['M'], EXPR$0=[$t1], EXPR$1=[$t2], EXPR$2=[$t3])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=['Eric'], expr#2=[20],
expr#3=['M'], EXPR$0=[$t1], EXPR$1=[$t2], EXPR$2=[$t3])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=['Susan'], expr#2=[30],
expr#3=['F'], EXPR$0=[$t1], EXPR$1=[$t2], EXPR$2=[$t3])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=['Alice'], expr#2=[30],
expr#3=['F'], EXPR$0=[$t1], EXPR$1=[$t2], EXPR$2=[$t3])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=['Adam'], expr#2=[50],
expr#3=['M'], EXPR$0=[$t1], EXPR$1=[$t2], EXPR$2=[$t3])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=['Eve'], expr#2=[50],
expr#3=['F'], EXPR$0=[$t1], EXPR$1=[$t2], EXPR$2=[$t3])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=['Grace'], expr#2=[60],
expr#3=['F'], EXPR$0=[$t1], EXPR$1=[$t2], EXPR$2=[$t3])
+ EnumerableValues(tuples=[[{ 0 }]])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=['Wilma'],
expr#2=[null:INTEGER], expr#3=['F'], EXPR$0=[$t1], EXPR$1=[$t2], EXPR$2=[$t3])
+ EnumerableValues(tuples=[[{ 0 }]])
+!plan
+
# End agg.iq
