This is an automated email from the ASF dual-hosted git repository.
xiong pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new 86b34f7ba8 [CALCITE-5138] Join on condition generates wrong plan when
the condition is sub-query
86b34f7ba8 is described below
commit 86b34f7ba81cc85aec4589f8fa4dcbf503ad8677
Author: NobiGo <[email protected]>
AuthorDate: Sun May 8 10:45:01 2022 +0800
[CALCITE-5138] Join on condition generates wrong plan when the condition is
sub-query
---
.../calcite/sql/validate/SqlValidatorImpl.java | 13 +-
.../apache/calcite/sql2rel/SqlToRelConverter.java | 23 +++-
.../apache/calcite/test/SqlToRelConverterTest.java | 9 ++
.../apache/calcite/test/SqlToRelConverterTest.xml | 21 +++
core/src/test/resources/sql/conditions.iq | 153 +++++++++++++++++++++
5 files changed, 216 insertions(+), 3 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
index 01c13c94c0..2978f1a700 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
@@ -4341,11 +4341,22 @@ public class SqlValidatorImpl implements
SqlValidatorWithHints {
condition.validate(this, scope);
final RelDataType type = deriveType(scope, condition);
- if (!SqlTypeUtil.inBooleanFamily(type)) {
+ if (!isReturnBooleanType(type)) {
throw newValidationError(condition, RESOURCE.condMustBeBoolean(clause));
}
}
+ private boolean isReturnBooleanType(RelDataType relDataType) {
+ if (relDataType instanceof RelRecordType) {
+ RelRecordType recordType = (RelRecordType) relDataType;
+ Preconditions.checkState(recordType.getFieldList().size() == 1,
+ "sub-query as condition must return only one column");
+ RelDataTypeField recordField = recordType.getFieldList().get(0);
+ return SqlTypeUtil.inBooleanFamily(recordField.getType());
+ }
+ return SqlTypeUtil.inBooleanFamily(relDataType);
+ }
+
protected void validateHavingClause(SqlSelect select) {
// HAVING is validated in the scope after groups have been created.
// For example, in "SELECT empno FROM emp WHERE empno = 10 GROUP BY
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 c22ecc5a2d..91d223c62b 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
@@ -3049,7 +3049,9 @@ public class SqlToRelConverter {
rightRel,
condition,
convertJoinType(join.getJoinType()));
- bb.setRoot(joinRel, false);
+ relBuilder.push(joinRel);
+ final RelNode newProjectRel =
relBuilder.project(relBuilder.fields()).build();
+ bb.setRoot(newProjectRel, false);
}
private RexNode convertNaturalCondition(
@@ -3113,6 +3115,13 @@ public class SqlToRelConverter {
: bb.reRegister(rightRel);
bb.setRoot(ImmutableList.of(leftRel, newRightRel));
RexNode conditionExp = bb.convertExpression(condition);
+ if (conditionExp instanceof RexInputRef && newRightRel != rightRel) {
+ int leftFieldCount = leftRel.getRowType().getFieldCount();
+ List<RelDataTypeField> rightFieldList =
newRightRel.getRowType().getFieldList();
+ int rightFieldCount = newRightRel.getRowType().getFieldCount();
+ conditionExp =
rexBuilder.makeInputRef(rightFieldList.get(rightFieldCount - 1).getType(),
+ leftFieldCount + rightFieldCount - 1);
+ }
return Pair.of(conditionExp, newRightRel);
}
@@ -4763,7 +4772,17 @@ public class SqlToRelConverter {
List<RegisterArgs> registerCopy = registered;
registered = new ArrayList<>();
for (RegisterArgs reg: registerCopy) {
- register(reg.rel, reg.joinType, reg.leftKeys);
+ RelNode relNode = reg.rel;
+ relBuilder.push(relNode);
+ final RelMetadataQuery mq = relBuilder.getCluster().getMetadataQuery();
+ final Boolean unique = mq.areColumnsUnique(relBuilder.peek(),
+ ImmutableBitSet.of());
+ if (unique == null || !unique) {
+ relBuilder.aggregate(relBuilder.groupKey(),
+ relBuilder.aggregateCall(SqlStdOperatorTable.SINGLE_VALUE,
+ relBuilder.field(0)));
+ }
+ register(relBuilder.build(), reg.joinType, reg.leftKeys);
}
return requireNonNull(this.root, "root");
}
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 a7e974fa9a..880846127c 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -4419,6 +4419,15 @@ class SqlToRelConverterTest extends SqlToRelTestBase {
sql(sql).withTrim(true).ok();
}
+ @Test void testJoinWithOnConditionQuery() {
+ String sql = ""
+ + "SELECT emp.deptno, emp.sal\n"
+ + "FROM dept\n"
+ + "JOIN emp\n"
+ + "ON (SELECT AVG(emp.sal) > 0 FROM emp)";
+ sql(sql).ok();
+ }
+
@Test void testJoinExpandAndDecorrelation() {
String sql = ""
+ "SELECT emp.deptno, emp.sal\n"
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 407f03ab4f..3cf56aa333 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -3400,6 +3400,27 @@ LogicalProject(EXPR$0=[COALESCE(ROW($4, $5, $6),
ROW($14, $15, $16)).X])
LogicalTableScan(table=[[CATALOG, CUSTOMER, CONTACT_PEEK]])
LogicalProject(CONTACTNO=[$0], FNAME=[$1], LNAME=[$2], EMAIL=[$3],
X=[$4.X], Y=[$4.Y], unit=[$4.unit], M=[$5.M], A=[$5.SUB.A], B=[$5.SUB.B])
LogicalTableScan(table=[[CATALOG, CUSTOMER, CONTACT_PEEK]])
+]]>
+ </Resource>
+ </TestCase>
+ <TestCase name="testJoinWithOnConditionQuery">
+ <Resource name="sql">
+ <![CDATA[SELECT emp.deptno, emp.sal
+FROM dept
+ LEFT JOIN emp ON (SELECT AVG(emp.sal) > 0 FROM emp)]]>
+ </Resource>
+ <Resource name="plan">
+ <![CDATA[
+LogicalProject(DEPTNO=[$9], SAL=[$7])
+ LogicalJoin(condition=[$11], joinType=[inner])
+ LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
+ LogicalJoin(condition=[true], joinType=[left])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalAggregate(group=[{}], agg#0=[SINGLE_VALUE($0)])
+ LogicalProject(EXPR$0=[>($0, 0)])
+ LogicalAggregate(group=[{}], agg#0=[AVG($0)])
+ LogicalProject(SAL=[$5])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
</TestCase>
diff --git a/core/src/test/resources/sql/conditions.iq
b/core/src/test/resources/sql/conditions.iq
index f83ea1e1d7..96a6bf4ebe 100644
--- a/core/src/test/resources/sql/conditions.iq
+++ b/core/src/test/resources/sql/conditions.iq
@@ -357,4 +357,157 @@ EnumerableCalc(expr#0..7=[{inputs}],
expr#8=[CAST($t7):INTEGER], expr#9=[25], ex
EnumerableTableScan(table=[[scott, EMP]])
!plan
+# [CALCITE-5138] Join on condition generates wrong plan when the condition is
sub-query
+
+# Bad: more than one value returned by sub-query
+select empno
+from emp as r
+ left join dept as s on (select empno > 0 from emp);
+more than one value in agg SINGLE_VALUE
+!error
+
+# Bad: more than one value returned by sub-query
+select empno
+from emp as r
+ left join dept as s on (select true from emp);
+more than one value in agg SINGLE_VALUE
+!error
+
+# Bad: more than one column returned by sub-query
+select empno
+from emp as r
+ left join dept as s on (select true,true from emp);
+sub-query as condition must return only one column
+!error
+
+# Bad: column returned by sub-query is not a conditon
+select empno
+from emp as r
+ left join dept as s on (select empno from emp);
+ON clause must be a condition
+!error
+
+# sub-query return true
+select empno
+from emp as r
+ left join dept as s on (select count(true) > 0 from emp)
+where empno = 7369;
++-------+
+| EMPNO |
++-------+
+| 7369 |
+| 7369 |
+| 7369 |
+| 7369 |
++-------+
+(4 rows)
+
+!ok
+
+EnumerableCalc(expr#0..2=[{inputs}], EMPNO=[$t0])
+ EnumerableNestedLoopJoin(condition=[true], joinType=[left])
+ EnumerableCalc(expr#0..7=[{inputs}], expr#8=[CAST($t0):INTEGER NOT NULL],
expr#9=[7369], expr#10=[=($t8, $t9)], EMPNO=[$t0], $condition=[$t10])
+ EnumerableTableScan(table=[[scott, EMP]])
+ EnumerableCalc(expr#0..1=[{inputs}], proj#0..1=[{exprs}], $condition=[$t1])
+ EnumerableNestedLoopJoin(condition=[true], joinType=[left])
+ EnumerableCalc(expr#0..2=[{inputs}], DEPTNO=[$t0])
+ EnumerableTableScan(table=[[scott, DEPT]])
+ EnumerableAggregate(group=[{}], agg#0=[SINGLE_VALUE($0)])
+ EnumerableCalc(expr#0=[{inputs}], expr#1=[0], expr#2=[>($t0, $t1)],
EXPR$0=[$t2])
+ EnumerableAggregate(group=[{}], agg#0=[COUNT()])
+ EnumerableTableScan(table=[[scott, EMP]])
+!plan
+
+# sub-query return true with Equal condition
+select r.empno, s.deptno
+from emp as r
+ left join dept as s on (select count(true) > 0 from emp) and s.deptno
= r.deptno;
+
++-------+--------+
+| EMPNO | DEPTNO |
++-------+--------+
+| 7369 | 20 |
+| 7499 | 30 |
+| 7521 | 30 |
+| 7566 | 20 |
+| 7654 | 30 |
+| 7698 | 30 |
+| 7782 | 10 |
+| 7788 | 20 |
+| 7839 | 10 |
+| 7844 | 30 |
+| 7876 | 20 |
+| 7900 | 30 |
+| 7902 | 20 |
+| 7934 | 10 |
++-------+--------+
+(14 rows)
+
+!ok
+
+# sub-query return false
+select r.empno, s.deptno
+from emp as r
+ left join dept as s on (select count(job) > 0 from bonus);
+
++-------+--------+
+| EMPNO | DEPTNO |
++-------+--------+
+| 7369 | |
+| 7499 | |
+| 7521 | |
+| 7566 | |
+| 7654 | |
+| 7698 | |
+| 7782 | |
+| 7788 | |
+| 7839 | |
+| 7844 | |
+| 7876 | |
+| 7900 | |
+| 7902 | |
+| 7934 | |
++-------+--------+
+(14 rows)
+
+!ok
+
+# ON condition is a column
+select *
+from dept
+ LEFT JOIN (SELECT COUNT(*) > 0 AS "EXPR$0"
+ FROM emp) AS "t0" ON "EXPR$0";
++--------+------------+----------+--------+
+| DEPTNO | DNAME | LOC | EXPR$0 |
++--------+------------+----------+--------+
+| 10 | ACCOUNTING | NEW YORK | true |
+| 20 | RESEARCH | DALLAS | true |
+| 30 | SALES | CHICAGO | true |
+| 40 | OPERATIONS | BOSTON | true |
++--------+------------+----------+--------+
+(4 rows)
+
+!ok
+
+# As above, but complicated
+SELECT emp.empno
+FROM emp
+ LEFT JOIN (select *
+ from dept
+ LEFT JOIN (SELECT COUNT(*) > 0 AS "EXPR$0"
+ FROM emp) AS "t0" ON TRUE) as "dt0*"
ON "EXPR$0"
+where emp.empno = 7902;
++-------+
+| EMPNO |
++-------+
+| 7902 |
+| 7902 |
+| 7902 |
+| 7902 |
++-------+
+(4 rows)
+
+!ok
+
+
# End conditions.iq
