This is an automated email from the ASF dual-hosted git repository.
jhyde 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 c1b042f [CALCITE-3297] PigToSqlAggregateRule should be applied on
multi-set projection to produce an optimal plan (Igor Guzenko)
c1b042f is described below
commit c1b042fec9a979bfa39a767f7b04da0a5134053a
Author: Igor Guzenko <[email protected]>
AuthorDate: Wed Aug 28 19:15:03 2019 +0300
[CALCITE-3297] PigToSqlAggregateRule should be applied on multi-set
projection to produce an optimal plan (Igor Guzenko)
Close apache/calcite#1420
---
.../org/apache/calcite/sql/type/SqlTypeUtil.java | 14 ------------
.../java/org/apache/calcite/tools/RelBuilder.java | 19 +---------------
.../apache/calcite/test/SqlToRelConverterTest.xml | 25 +++++++++-------------
.../java/org/apache/calcite/test/PigRelOpTest.java | 23 +++-----------------
4 files changed, 14 insertions(+), 67 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
index a859f33..bd0941d 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
@@ -21,7 +21,6 @@ import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rel.type.RelDataTypeFamily;
import org.apache.calcite.rel.type.RelDataTypeField;
import org.apache.calcite.rel.type.RelDataTypeFieldImpl;
-import org.apache.calcite.rex.RexNode;
import org.apache.calcite.rex.RexUtil;
import org.apache.calcite.sql.SqlBasicTypeNameSpec;
import org.apache.calcite.sql.SqlCall;
@@ -1255,19 +1254,6 @@ public abstract class SqlTypeUtil {
}
/**
- * Checks that there is no expression inside list which may
- * return struct type.
- *
- * @param exprList list of expressions to check
- * @return true if all expressions don't return struct type
- */
- public static boolean isFlat(List<RexNode> exprList) {
- return exprList.stream()
- .map(RexNode::getType)
- .noneMatch(RelDataType::isStruct);
- }
-
- /**
* Returns whether two types are comparable. They need to be scalar types of
* the same family, or struct types whose fields are pairwise comparable.
*
diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
index 0d14e46..a8b51a4 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -80,7 +80,6 @@ import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
import org.apache.calcite.sql.type.SqlReturnTypeInference;
import org.apache.calcite.sql.type.SqlTypeName;
-import org.apache.calcite.sql.type.SqlTypeUtil;
import org.apache.calcite.sql.type.TableFunctionReturnTypeInference;
import org.apache.calcite.sql.validate.SqlValidatorUtil;
import org.apache.calcite.util.Holder;
@@ -1310,8 +1309,7 @@ public class RelBuilder {
}
if (frame.rel instanceof Project
- && shouldMergeProject()
- && isNotRestructuringProjection(frame.rel.getRowType(), nodeList)) {
+ && shouldMergeProject()) {
final Project project = (Project) frame.rel;
// Populate field names. If the upper expression is an input ref and does
// not have a recommended name, use the name of the underlying field.
@@ -1420,21 +1418,6 @@ public class RelBuilder {
return this;
}
- /**
- * Restructuring projection is when inputProjection returns flat type
- * but new projection collects flatten types back into struct columns.
- * Given method negates the condition to know whether projects can be merged.
- *
- * @param inputProjectionType input projection result type
- * @param newProjects new projections
- * @return whether new projections don't do restructuring
- * (collection of flattened fields back into struct fields)
- */
- private boolean isNotRestructuringProjection(RelDataType inputProjectionType,
- List<RexNode> newProjects) {
- return SqlTypeUtil.isFlat(newProjects) ||
!SqlTypeUtil.isFlat(inputProjectionType);
- }
-
/** Whether to attempt to merge consecutive {@link Project} operators.
*
* <p>The default implementation returns {@code true};
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 f8cace1..5e26d5b 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -491,9 +491,8 @@ LogicalProject(EXPR$0=[ITEM(ITEM($3, 1).DETAIL.SKILLS, +(2,
3)).DESC])
</Resource>
<Resource name="plan">
<![CDATA[
-LogicalProject(EXPR$0=[ROW($0, $1, ROW($2))])
- LogicalProject(EXPR$0$0=[ITEM($3, 1).EMPNO], EXPR$0$1=[ITEM($3, 1).ENAME],
EXPR$0$2$0=[ITEM($3, 1).DETAIL.SKILLS])
- LogicalTableScan(table=[[CATALOG, SALES, DEPT_NESTED]])
+LogicalProject(EXPR$0=[ROW(ITEM($3, 1).EMPNO, ITEM($3, 1).ENAME, ROW(ITEM($3,
1).DETAIL.SKILLS))])
+ LogicalTableScan(table=[[CATALOG, SALES, DEPT_NESTED]])
]]>
</Resource>
</TestCase>
@@ -886,9 +885,8 @@ LogicalProject(ZIP=[$3])
<TestCase name="testNestedStructFieldAccess">
<Resource name="plan">
<![CDATA[
-LogicalProject(EXPR$0=[ROW($0, $1)])
- LogicalProject(EXPR$0=[$2.OTHERS.A], EXPR$01=[$2.OTHERS.B])
- LogicalTableScan(table=[[CATALOG, SALES, DEPT_NESTED]])
+LogicalProject(EXPR$0=[ROW($2.OTHERS.A, $2.OTHERS.B)])
+ LogicalTableScan(table=[[CATALOG, SALES, DEPT_NESTED]])
]]>
</Resource>
</TestCase>
@@ -927,18 +925,16 @@ LogicalProject(EXPR$0=[ITEM(ITEM(ITEM(ITEM(ITEM($3, 0),
'detail'), 'skills'), 0)
<TestCase name="testArrayElementDoublyNestedStruct">
<Resource name="plan">
<![CDATA[
-LogicalProject(EXPR$0=[ROW($0, $1, ROW($2, $3))])
- LogicalProject(EXPR$0$0=[ITEM(ITEM(ITEM(ITEM($3, 0), 'detail'), 'skills'),
0).TYPE], EXPR$0$1=[ITEM(ITEM(ITEM(ITEM($3, 0), 'detail'), 'skills'), 0).DESC],
EXPR$0$2$0=[ITEM(ITEM(ITEM(ITEM($3, 0), 'detail'), 'skills'), 0).OTHERS.A],
EXPR$0$2$1=[ITEM(ITEM(ITEM(ITEM($3, 0), 'detail'), 'skills'), 0).OTHERS.B])
- LogicalTableScan(table=[[CATALOG, SALES, DEPT_NESTED]])
+LogicalProject(EXPR$0=[ROW(ITEM(ITEM(ITEM(ITEM($3, 0), 'detail'), 'skills'),
0).TYPE, ITEM(ITEM(ITEM(ITEM($3, 0), 'detail'), 'skills'), 0).DESC,
ROW(ITEM(ITEM(ITEM(ITEM($3, 0), 'detail'), 'skills'), 0).OTHERS.A,
ITEM(ITEM(ITEM(ITEM($3, 0), 'detail'), 'skills'), 0).OTHERS.B))])
+ LogicalTableScan(table=[[CATALOG, SALES, DEPT_NESTED]])
]]>
</Resource>
</TestCase>
<TestCase name="testArrayElementThreeTimesNestedStruct">
<Resource name="plan">
<![CDATA[
-LogicalProject(EXPR$0=[ROW($0, $1)])
- LogicalProject(EXPR$0$0$2$0=[ITEM(ITEM(ITEM(ITEM($3, 0), 'detail'),
'skills'), 0).OTHERS.A], EXPR$0$0$2$1=[ITEM(ITEM(ITEM(ITEM($3, 0), 'detail'),
'skills'), 0).OTHERS.B])
- LogicalTableScan(table=[[CATALOG, SALES, DEPT_NESTED]])
+LogicalProject(EXPR$0=[ROW(ITEM(ITEM(ITEM(ITEM($3, 0), 'detail'), 'skills'),
0).OTHERS.A, ITEM(ITEM(ITEM(ITEM($3, 0), 'detail'), 'skills'), 0).OTHERS.B)])
+ LogicalTableScan(table=[[CATALOG, SALES, DEPT_NESTED]])
]]>
</Resource>
</TestCase>
@@ -1775,9 +1771,8 @@ from (select row(row(1)) r from dept) t]]>
</Resource>
<Resource name="plan">
<![CDATA[
-LogicalProject(MYROW=[ROW(ROW($0))])
- LogicalProject(MYROW$$0$$0=[1])
- LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
+LogicalProject(MYROW=[ROW(ROW(1))])
+ LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
]]>
</Resource>
</TestCase>
diff --git a/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
b/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
index f41729e..cc2223f 100644
--- a/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
+++ b/piglet/src/test/java/org/apache/calcite/test/PigRelOpTest.java
@@ -19,7 +19,6 @@ package org.apache.calcite.test;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.sql.SqlDialect;
import org.apache.calcite.sql.dialect.CalciteSqlDialect;
-import org.apache.calcite.util.Bug;
import org.apache.calcite.util.TestUtil;
import org.apache.calcite.util.Util;
@@ -1049,36 +1048,20 @@ public class PigRelOpTest extends PigRelTestBase {
+ " LogicalAggregate(group=[{0}], A=[COLLECT($1)])\n"
+ " LogicalProject(DEPTNO=[$0], $f1=[ROW($0, $1, $2)])\n"
+ " LogicalTableScan(table=[[scott, DEPT]])\n";
- String optimizedPlan = ""
+ final String optimizedPlan = ""
+ "LogicalProject($f0=[$1])\n"
+ " LogicalAggregate(group=[{0}], agg#0=[COLLECT($2)])\n"
- + " LogicalProject(DEPTNO=[$0], DNAME=[$1], $f2=[ROW"
- + "($0, $1)])\n"
+ + " LogicalProject(DEPTNO=[$0], DNAME=[$1], $f2=[ROW($0, $1)])\n"
+ " LogicalTableScan(table=[[scott, DEPT]])\n";
final String result = ""
+ "({(20,RESEARCH)})\n"
+ "({(40,OPERATIONS)})\n"
+ "({(10,ACCOUNTING)})\n"
+ "({(30,SALES)})\n";
- String sql = ""
+ final String sql = ""
+ "SELECT COLLECT(ROW(DEPTNO, DNAME)) AS $f0\n"
+ "FROM scott.DEPT\n"
+ "GROUP BY DEPTNO";
- // When
- // [CALCITE-3297] PigToSqlAggregateRule should be applied on multi-set
- // projection to produce an optimal plan
- // is fixed we can remove the following block.
- if (Bug.remark("[CALCITE-3297]") != null) {
- optimizedPlan = ""
- + "LogicalProject($f0=[MULTISET_PROJECTION($1, 0, 1)])\n"
- + " LogicalAggregate(group=[{0}], A=[COLLECT($1)])\n"
- + " LogicalProject(DEPTNO=[$0], $f1=[ROW($0, $1, $2)])\n"
- + " LogicalTableScan(table=[[scott, DEPT]])\n";
- sql = "SELECT MULTISET_PROJECTION(COLLECT(ROW(DEPTNO, DNAME, LOC)), "
- + "0, 1) AS $f0\n"
- + "FROM scott.DEPT\n"
- + "GROUP BY DEPTNO";
- }
pig(script).assertRel(hasTree(plan))
.assertOptimizedRel(hasTree(optimizedPlan))
.assertResult(is(result))
