Repository: calcite Updated Branches: refs/heads/master 999021115 -> f55d10c14
[CALCITE-864] Correlation variable has incorrect row type if it is populated by right side of a Join Also fixes [CALCITE-559], apparently a duplicate issue. Project: http://git-wip-us.apache.org/repos/asf/calcite/repo Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/d80e26c0 Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/d80e26c0 Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/d80e26c0 Branch: refs/heads/master Commit: d80e26c016babcb250e41b0abe94b8b0cfadbbe7 Parents: 9990211 Author: Julian Hyde <[email protected]> Authored: Fri Jan 22 18:00:40 2016 -0800 Committer: Julian Hyde <[email protected]> Committed: Tue Jan 26 11:53:24 2016 -0800 ---------------------------------------------------------------------- .../calcite/rel/type/RelDataTypeFactory.java | 13 ++++ .../calcite/sql/validate/SqlQualified.java | 4 ++ .../apache/calcite/sql2rel/RelDecorrelator.java | 7 +-- .../calcite/sql2rel/SqlToRelConverter.java | 64 ++++++++++++++----- .../java/org/apache/calcite/test/JdbcTest.java | 22 ++++--- .../calcite/test/SqlToRelConverterTest.java | 15 +++++ .../calcite/test/SqlToRelConverterTest.xml | 27 +++++++- core/src/test/resources/sql/subquery.iq | 66 ++++++++++++++++++++ 8 files changed, 186 insertions(+), 32 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/calcite/blob/d80e26c0/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java b/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java index 35272c8..ab9b5bb 100644 --- a/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java +++ b/core/src/main/java/org/apache/calcite/rel/type/RelDataTypeFactory.java @@ -19,6 +19,7 @@ package org.apache.calcite.rel.type; import org.apache.calcite.sql.SqlCollation; import org.apache.calcite.sql.SqlIntervalQualifier; import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.calcite.sql.validate.SqlValidatorUtil; import java.nio.charset.Charset; import java.util.ArrayList; @@ -418,6 +419,18 @@ public interface RelDataTypeFactory { } /** + * Makes sure that field names are unique. + */ + public FieldInfoBuilder uniquify() { + final List<String> uniqueNames = SqlValidatorUtil.uniquify(names); + if (uniqueNames != names) { + names.clear(); + names.addAll(uniqueNames); + } + return this; + } + + /** * Creates a struct type with the current contents of this builder. */ public RelDataType build() { http://git-wip-us.apache.org/repos/asf/calcite/blob/d80e26c0/core/src/main/java/org/apache/calcite/sql/validate/SqlQualified.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql/validate/SqlQualified.java b/core/src/main/java/org/apache/calcite/sql/validate/SqlQualified.java index 4b99845..067c324 100644 --- a/core/src/main/java/org/apache/calcite/sql/validate/SqlQualified.java +++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlQualified.java @@ -47,6 +47,10 @@ public class SqlQualified { this.identifier = identifier; } + @Override public String toString() { + return "{id: " + identifier.toString() + ", prefix: " + prefixLength + "}"; + } + public static SqlQualified create(SqlValidatorScope scope, int prefixLength, SqlValidatorNamespace namespace, SqlIdentifier identifier) { return new SqlQualified(scope, prefixLength, namespace, identifier); http://git-wip-us.apache.org/repos/asf/calcite/blob/d80e26c0/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java index ba196db..099bb9a 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java @@ -35,7 +35,6 @@ import org.apache.calcite.rel.core.Aggregate; import org.apache.calcite.rel.core.AggregateCall; import org.apache.calcite.rel.core.Correlate; import org.apache.calcite.rel.core.CorrelationId; -import org.apache.calcite.rel.core.Join; import org.apache.calcite.rel.core.JoinRelType; import org.apache.calcite.rel.core.Project; import org.apache.calcite.rel.core.RelFactories; @@ -806,11 +805,7 @@ public class RelDecorrelator implements ReflectiveVisitor { private RelNode getCorRel(Correlation corVar) { final RelNode r = cm.mapCorVarToCorRel.get(corVar.corr); - RelNode r2 = r.getInput(0); - if (r2 instanceof Join) { - r2 = r2.getInput(0); - } - return r2; + return r.getInput(0); } private void decorrelateInputWithValueGenerator(RelNode rel) { http://git-wip-us.apache.org/repos/asf/calcite/blob/d80e26c0/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java index d9189a5..a85c79a 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java @@ -160,6 +160,7 @@ import org.apache.calcite.util.trace.CalciteTrace; import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; @@ -2246,8 +2247,8 @@ public class SqlToRelConverter { } } - RelDataTypeField field = - catalogReader.field(foundNs.getRowType(), originalFieldName); + final RelDataTypeField field = foundNs.getRowType().getFieldList() + .get(fieldAccess.getField().getIndex() - namespaceOffset); int pos = namespaceOffset + field.getIndex(); assert field.getType() @@ -3275,21 +3276,26 @@ public class SqlToRelConverter { } else { qualified = SqlQualified.create(null, 1, null, identifier); } - final RexNode e0 = bb.lookupExp(qualified); - RexNode e = e0; + final Pair<RexNode, Map<String, Integer>> e0 = bb.lookupExp(qualified); + RexNode e = e0.left; for (String name : qualified.suffixTranslated()) { - final boolean caseSensitive = true; // name already fully-qualified - e = rexBuilder.makeFieldAccess(e, name, caseSensitive); + if (e == e0.left && e0.right != null) { + int i = e0.right.get(name); + e = rexBuilder.makeFieldAccess(e, i); + } else { + final boolean caseSensitive = true; // name already fully-qualified + e = rexBuilder.makeFieldAccess(e, name, caseSensitive); + } } if (e instanceof RexInputRef) { // adjust the type to account for nulls introduced by outer joins e = adjustInputRef(bb, (RexInputRef) e); } - if (e0 instanceof RexCorrelVariable) { + if (e0.left instanceof RexCorrelVariable) { assert e instanceof RexFieldAccess; final RexNode prev = - bb.mapCorrelateToRex.put(((RexCorrelVariable) e0).id, + bb.mapCorrelateToRex.put(((RexCorrelVariable) e0.left).id, (RexFieldAccess) e); assert prev == null; } @@ -3891,14 +3897,14 @@ public class SqlToRelConverter { * @return a {@link RexFieldAccess} or {@link RexRangeRef}, or null if * not found */ - RexNode lookupExp(SqlQualified qualified) { + Pair<RexNode, Map<String, Integer>> lookupExp(SqlQualified qualified) { if (nameToNodeMap != null && qualified.prefixLength == 1) { RexNode node = nameToNodeMap.get(qualified.identifier.names.get(0)); if (node == null) { throw Util.newInternal("Unknown identifier '" + qualified.identifier + "' encountered while expanding expression"); } - return node; + return Pair.of(node, null); } int[] offsets = {-1}; final SqlValidatorScope[] ancestorScopes = {null}; @@ -3917,7 +3923,12 @@ public class SqlToRelConverter { int offset = offsets[0]; final LookupContext rels = new LookupContext(this, inputs, systemFieldList.size()); - return lookup(offset, rels); + final RexNode node = lookup(offset, rels); + if (node == null) { + return null; + } else { + return Pair.of(node, null); + } } else { // We're referencing a relational expression which has not been // converted yet. This occurs when from items are correlated, @@ -3926,10 +3937,33 @@ public class SqlToRelConverter { assert isParent; DeferredLookup lookup = new DeferredLookup(this, qualified.identifier.names.get(0)); - final CorrelationId correlName = cluster.createCorrel(); - mapCorrelToDeferred.put(correlName, lookup); - final RelDataType rowType = foundNs.getRowType(); - return rexBuilder.makeCorrel(rowType, correlName); + final CorrelationId correlId = cluster.createCorrel(); + mapCorrelToDeferred.put(correlId, lookup); + if (offsets[0] < 0) { + return Pair.of(rexBuilder.makeCorrel(foundNs.getRowType(), correlId), + null); + } else { + final RelDataTypeFactory.FieldInfoBuilder builder = + typeFactory.builder(); + final ListScope ancestorScope1 = (ListScope) ancestorScopes[0]; + final ImmutableMap.Builder<String, Integer> fields = + ImmutableMap.builder(); + int i = 0; + int offset = 0; + for (SqlValidatorNamespace c : ancestorScope1.getChildren()) { + builder.addAll(c.getRowType().getFieldList()); + if (i == offsets[0]) { + for (RelDataTypeField field : c.getRowType().getFieldList()) { + fields.put(field.getName(), field.getIndex() + offset); + } + } + ++i; + offset += c.getRowType().getFieldCount(); + } + final RexNode c = + rexBuilder.makeCorrel(builder.uniquify().build(), correlId); + return Pair.<RexNode, Map<String, Integer>>of(c, fields.build()); + } } } http://git-wip-us.apache.org/repos/asf/calcite/blob/d80e26c0/core/src/test/java/org/apache/calcite/test/JdbcTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java b/core/src/test/java/org/apache/calcite/test/JdbcTest.java index 0e10877..0cc70ba 100644 --- a/core/src/test/java/org/apache/calcite/test/JdbcTest.java +++ b/core/src/test/java/org/apache/calcite/test/JdbcTest.java @@ -4581,19 +4581,21 @@ public class JdbcTest { } } - @Ignore("CALCITE-559 Correlated subquery will hit exception in Calcite") - @Test public void testJoinCorreScalarSubQ() - throws ClassNotFoundException, SQLException { + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-559";>[CALCITE-559] + * Correlated scalar subquery in WHERE gives error</a>. */ + @Test public void testJoinCorrelatedScalarSubquery() throws SQLException { + final String sql = "select e.employee_id, d.department_id " + + " from employee e, department d " + + " where e.department_id = d.department_id " + + " and e.salary > (select avg(e2.salary) " + + " from employee e2 " + + " where e2.store_id = e.store_id)"; CalciteAssert.that() .with(CalciteAssert.Config.FOODMART_CLONE) .with(Lex.JAVA) - .query("select e.employee_id, d.department_id " - + " from employee e, department d " - + " where e.department_id = d.department_id and " - + " e.salary > (select avg(e2.salary) " - + " from employee e2 " - + " where e2.store_id = e.store_id)") - .returnsCount(0); + .query(sql) + .returnsCount(599); } @Test public void testLeftJoin() { http://git-wip-us.apache.org/repos/asf/calcite/blob/d80e26c0/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java index 2e18dc7..f3cdf68 100644 --- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java +++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java @@ -834,6 +834,21 @@ public class SqlToRelConverterTest extends SqlToRelTestBase { sql(sql).expand(false).convertsTo("${plan}"); } + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-864";>[CALCITE-864] + * Correlation variable has incorrect row type if it is populated by right + * side of a Join</a>. */ + @Test public void testCorrelatedSubQueryInJoin() { + final String sql = "select *\n" + + "from emp as e\n" + + "join dept as d using (deptno)\n" + + "where d.name = (\n" + + " select max(name)\n" + + " from dept as d2\n" + + " where d2.deptno = d.deptno)"; + sql(sql).expand(false).convertsTo("${plan}"); + } + @Test public void testExists() { check( "select*from emp where exists (select 1 from dept where deptno=55)", http://git-wip-us.apache.org/repos/asf/calcite/blob/d80e26c0/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml ---------------------------------------------------------------------- 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 a02eb60..84e1afe 100644 --- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml +++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml @@ -117,6 +117,31 @@ LogicalAggregate(group=[{0}], EXPR$1=[SUM($1)], EXPR$2=[SUM(DISTINCT $1)], EXPR$ <![CDATA[select deptno, sum(sal), sum(distinct sal), count(*) from emp group by deptno]]> </Resource> </TestCase> + <TestCase name="testCorrelatedSubQueryInJoin"> + <Resource name="sql"> + <![CDATA[select * +from emp as e +join dept as d using (deptno) +where d.name = ( + select max(name) + from dept as d2 + where d2.deptno = d.deptno)]]> + </Resource> + <Resource name="plan"> + <![CDATA[ +LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8], DEPTNO0=[$9], NAME=[$10]) + LogicalFilter(condition=[=($10, $SCALAR_QUERY({ +LogicalAggregate(group=[{}], EXPR$0=[MAX($0)]) + LogicalProject(NAME=[$1]) + LogicalFilter(condition=[=($0, $cor0.DEPTNO0)]) + LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +}))], variablesSet=[[$cor0]]) + LogicalJoin(condition=[=($7, $9)], joinType=[inner]) + LogicalTableScan(table=[[CATALOG, SALES, EMP]]) + LogicalTableScan(table=[[CATALOG, SALES, DEPT]]) +]]> + </Resource> + </TestCase> <TestCase name="testUnnest"> <Resource name="plan"> <![CDATA[ @@ -2859,7 +2884,7 @@ or exists (select deptno from emp where empno > dept.deptno + 5)]]> <![CDATA[ LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8], DEPTNO0=[$9], NAME=[$10]) LogicalJoin(condition=[OR(=($0, 1), EXISTS({ -LogicalFilter(condition=[>($0, +($cor0.DEPTNO, 5))]) +LogicalFilter(condition=[>($0, +($cor0.DEPTNO0, 5))]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) }))], joinType=[left]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) http://git-wip-us.apache.org/repos/asf/calcite/blob/d80e26c0/core/src/test/resources/sql/subquery.iq ---------------------------------------------------------------------- diff --git a/core/src/test/resources/sql/subquery.iq b/core/src/test/resources/sql/subquery.iq index cfeb6e5..8f0eb70 100644 --- a/core/src/test/resources/sql/subquery.iq +++ b/core/src/test/resources/sql/subquery.iq @@ -319,4 +319,70 @@ where e.job not in ( !plan !} +# [CALCITE-864] Correlation variable has incorrect row type if it is populated +# by right side of a Join +select * +from "scott".emp as e +join "scott".dept as d using (deptno) +where sal = ( + select max(sal) + from "scott".emp as e2 + join "scott".dept as d2 using (deptno) + where d2.deptno = d.deptno); + EMPNO | ENAME | JOB | MGR | HIREDATE | SAL | COMM | DEPTNO | DEPTNO0 | DNAME | LOC +-------+-------+-----------+------+------------+---------+------+--------+---------+------------+---------- + 7698 | BLAKE | MANAGER | 7839 | 1981-01-05 | 2850.00 | | 30 | 30 | SALES | CHICAGO + 7788 | SCOTT | ANALYST | 7566 | 1987-04-19 | 3000.00 | | 20 | 20 | RESEARCH | DALLAS + 7839 | KING | PRESIDENT | | 1981-11-17 | 5000.00 | | 10 | 10 | ACCOUNTING | NEW YORK + 7902 | FORD | ANALYST | 7566 | 1981-12-03 | 3000.00 | | 20 | 20 | RESEARCH | DALLAS +(4 rows) + +!ok + +# Simpler test case for [CALCITE-864] +select empno, ename, sal, e.deptno, loc +from "scott".emp as e +join "scott".dept as d using (deptno) +where e.sal = ( + select max(sal) + from "scott".emp as e2 + where e2.deptno = e.deptno); + EMPNO | ENAME | SAL | DEPTNO | LOC +-------+-------+---------+--------+---------- + 7698 | BLAKE | 2850.00 | 30 | CHICAGO + 7788 | SCOTT | 3000.00 | 20 | DALLAS + 7839 | KING | 5000.00 | 10 | NEW YORK + 7902 | FORD | 3000.00 | 20 | DALLAS +(4 rows) + +!ok + +# Simpler test case for [CALCITE-864] +select * +from "scott".emp as e +join "scott".dept as d using (deptno) +where d.dname = ( + select max(dname) + from "scott".dept as d2 + where d2.deptno = d.deptno); + EMPNO | ENAME | JOB | MGR | HIREDATE | SAL | COMM | DEPTNO | DEPTNO0 | DNAME | LOC +-------+--------+-----------+------+------------+---------+---------+--------+---------+------------+---------- + 7369 | SMITH | CLERK | 7902 | 1980-12-17 | 800.00 | | 20 | 20 | RESEARCH | DALLAS + 7499 | ALLEN | SALESMAN | 7698 | 1981-02-20 | 1600.00 | 300.00 | 30 | 30 | SALES | CHICAGO + 7521 | WARD | SALESMAN | 7698 | 1981-02-22 | 1250.00 | 500.00 | 30 | 30 | SALES | CHICAGO + 7566 | JONES | MANAGER | 7839 | 1981-02-04 | 2975.00 | | 20 | 20 | RESEARCH | DALLAS + 7654 | MARTIN | SALESMAN | 7698 | 1981-09-28 | 1250.00 | 1400.00 | 30 | 30 | SALES | CHICAGO + 7698 | BLAKE | MANAGER | 7839 | 1981-01-05 | 2850.00 | | 30 | 30 | SALES | CHICAGO + 7782 | CLARK | MANAGER | 7839 | 1981-06-09 | 2450.00 | | 10 | 10 | ACCOUNTING | NEW YORK + 7788 | SCOTT | ANALYST | 7566 | 1987-04-19 | 3000.00 | | 20 | 20 | RESEARCH | DALLAS + 7839 | KING | PRESIDENT | | 1981-11-17 | 5000.00 | | 10 | 10 | ACCOUNTING | NEW YORK + 7844 | TURNER | SALESMAN | 7698 | 1981-09-08 | 1500.00 | 0.00 | 30 | 30 | SALES | CHICAGO + 7876 | ADAMS | CLERK | 7788 | 1987-05-23 | 1100.00 | | 20 | 20 | RESEARCH | DALLAS + 7900 | JAMES | CLERK | 7698 | 1981-12-03 | 950.00 | | 30 | 30 | SALES | CHICAGO + 7902 | FORD | ANALYST | 7566 | 1981-12-03 | 3000.00 | | 20 | 20 | RESEARCH | DALLAS + 7934 | MILLER | CLERK | 7782 | 1982-01-23 | 1300.00 | | 10 | 10 | ACCOUNTING | NEW YORK +(14 rows) + +!ok + # End subquery.iq
