diff --git a/core/src/main/java/org/apache/calcite/prepare/Prepare.java b/core/src/main/java/org/apache/calcite/prepare/Prepare.java index 04088231a48c..e56281e823c5 100644 --- a/core/src/main/java/org/apache/calcite/prepare/Prepare.java +++ b/core/src/main/java/org/apache/calcite/prepare/Prepare.java @@ -78,6 +78,7 @@ import java.util.List; import static org.apache.calcite.linq4j.Nullness.castNonNull; +import static org.apache.calcite.sql2rel.SqlToRelConverter.DEFAULT_IN_SUB_QUERY_THRESHOLD; import static java.util.Objects.requireNonNull; @@ -112,6 +113,10 @@ public abstract class Prepare { public static final TryThreadLocal<@Nullable Boolean> THREAD_EXPAND = TryThreadLocal.of(false); + // temporary. for testing. + public static final TryThreadLocal<@Nullable Integer> THREAD_INSUBQUERY_THRESHOLD = + TryThreadLocal.of(DEFAULT_IN_SUB_QUERY_THRESHOLD); + protected Prepare(CalcitePrepare.Context context, CatalogReader catalogReader, Convention resultConvention) { assert context != null; @@ -232,6 +237,7 @@ public PreparedResult prepareSql( SqlToRelConverter.config() .withTrimUnusedFields(true) .withExpand(castNonNull(THREAD_EXPAND.get())) + .withInSubQueryThreshold(castNonNull(THREAD_INSUBQUERY_THRESHOLD.get())) .withExplain(sqlQuery.getKind() == SqlKind.EXPLAIN); final Holder configHolder = Holder.of(config); Hook.SQL2REL_CONVERTER_CONFIG_BUILDER.run(configHolder); @@ -369,7 +375,8 @@ protected abstract RelNode decorrelate(SqlToRelConverter sqlToRelConverter, protected RelRoot trimUnusedFields(RelRoot root) { final SqlToRelConverter.Config config = SqlToRelConverter.config() .withTrimUnusedFields(shouldTrim(root.rel)) - .withExpand(castNonNull(THREAD_EXPAND.get())); + .withExpand(castNonNull(THREAD_EXPAND.get())) + .withInSubQueryThreshold(castNonNull(THREAD_INSUBQUERY_THRESHOLD.get())); final SqlToRelConverter converter = getSqlToRelConverter(getSqlValidator(), catalogReader, config); final boolean ordered = !root.collation.getFieldCollations().isEmpty(); 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 edf9636a606c..d28f21858c04 100644 --- a/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java +++ b/core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java @@ -1141,8 +1141,9 @@ private void substituteSubQuery(Blackboard bb, SubQuery subQuery) { if (query instanceof SqlNodeList) { SqlNodeList valueList = (SqlNodeList) query; - if (valueList.size() < config.getInSubQueryThreshold()) { - // We're under the threshold, so convert to OR. + // When the list size under the threshold or the list references columns, we convert to OR. + if (valueList.size() < config.getInSubQueryThreshold() + || valueList.accept(new SqlIdentifierFinder())) { subQuery.expr = convertInToOr( bb, @@ -6114,6 +6115,43 @@ private SubQuery(SqlNode node, RelOptUtil.Logic logic) { } } + /** + * Visitor that looks for an SqlIdentifier inside a tree of + * {@link SqlNode} objects and return {@link Boolean#TRUE} when it finds + * one. + */ + public static class SqlIdentifierFinder implements SqlVisitor { + + @Override public Boolean visit(SqlCall sqlCall) { + return sqlCall.getOperandList().stream().anyMatch(sqlNode -> sqlNode.accept(this)); + } + + @Override public Boolean visit(SqlNodeList nodeList) { + return nodeList.stream().anyMatch(sqlNode -> sqlNode.accept(this)); + } + + @Override public Boolean visit(SqlIdentifier identifier) { + return true; + } + + @Override public Boolean visit(SqlLiteral literal) { + return false; + } + + @Override public Boolean visit(SqlDataTypeSpec type) { + return false; + } + + @Override public Boolean visit(SqlDynamicParam param) { + return false; + } + + @Override public Boolean visit(SqlIntervalQualifier intervalQualifier) { + return false; + } + + } + /** * Visitor that collects all aggregate functions in a {@link SqlNode} tree. */ diff --git a/core/src/test/resources/sql/sub-query.iq b/core/src/test/resources/sql/sub-query.iq index 84084b604c95..be9e9ddccffd 100644 --- a/core/src/test/resources/sql/sub-query.iq +++ b/core/src/test/resources/sql/sub-query.iq @@ -3257,4 +3257,53 @@ EnumerableCalc(expr#0..7=[{inputs}], expr#8=[null:BOOLEAN], proj#0..8=[{exprs}]) EnumerableTableScan(table=[[scott, EMP]]) !plan +# [CALCITE-4844] IN-list that references columns is wrongly converted to Values, and gives incorrect results + +!set insubquerythreshold 0 + +SELECT empno, ename, mgr FROM "scott".emp WHERE 7782 IN (empno, mgr); ++-------+--------+------+ +| EMPNO | ENAME | MGR | ++-------+--------+------+ +| 7782 | CLARK | 7839 | +| 7934 | MILLER | 7782 | ++-------+--------+------+ +(2 rows) + +!ok + +EnumerableCalc(expr#0..7=[{inputs}], expr#8=[7782], expr#9=[CAST($t0):INTEGER NOT NULL], expr#10=[=($t8, $t9)], expr#11=[CAST($t3):INTEGER], expr#12=[=($t8, $t11)], expr#13=[OR($t10, $t12)], proj#0..1=[{exprs}], MGR=[$t3], $condition=[$t13]) + EnumerableTableScan(table=[[scott, EMP]]) +!plan + +SELECT empno, ename, mgr FROM "scott".emp WHERE (7782, 7839) IN ((empno, mgr), (mgr, empno)); ++-------+-------+------+ +| EMPNO | ENAME | MGR | ++-------+-------+------+ +| 7782 | CLARK | 7839 | ++-------+-------+------+ +(1 row) + +!ok + +EnumerableCalc(expr#0..7=[{inputs}], expr#8=[7782], expr#9=[CAST($t0):INTEGER NOT NULL], expr#10=[=($t8, $t9)], expr#11=[7839], expr#12=[CAST($t3):INTEGER], expr#13=[=($t11, $t12)], expr#14=[AND($t10, $t13)], expr#15=[=($t8, $t12)], expr#16=[=($t11, $t9)], expr#17=[AND($t15, $t16)], expr#18=[OR($t14, $t17)], proj#0..1=[{exprs}], MGR=[$t3], $condition=[$t18]) + EnumerableTableScan(table=[[scott, EMP]]) +!plan + +SELECT empno, ename, mgr FROM "scott".emp WHERE (7782, 7839) IN ((empno, 7839), (7782, mgr)); ++-------+-------+------+ +| EMPNO | ENAME | MGR | ++-------+-------+------+ +| 7566 | JONES | 7839 | +| 7698 | BLAKE | 7839 | +| 7782 | CLARK | 7839 | ++-------+-------+------+ +(3 rows) + +!ok + +EnumerableCalc(expr#0..7=[{inputs}], expr#8=[7782], expr#9=[CAST($t0):INTEGER NOT NULL], expr#10=[=($t8, $t9)], expr#11=[7839], expr#12=[CAST($t3):INTEGER], expr#13=[=($t11, $t12)], expr#14=[OR($t10, $t13)], proj#0..1=[{exprs}], MGR=[$t3], $condition=[$t14]) + EnumerableTableScan(table=[[scott, EMP]]) +!plan + # End sub-query.iq diff --git a/testkit/src/main/java/org/apache/calcite/test/QuidemTest.java b/testkit/src/main/java/org/apache/calcite/test/QuidemTest.java index 86b2eeac6516..2edcf52da323 100644 --- a/testkit/src/main/java/org/apache/calcite/test/QuidemTest.java +++ b/testkit/src/main/java/org/apache/calcite/test/QuidemTest.java @@ -48,6 +48,7 @@ import java.io.Writer; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.math.BigDecimal; import java.net.URL; import java.sql.Connection; import java.sql.DriverManager; @@ -150,6 +151,10 @@ protected void checkRun(String path) throws Exception { && (Boolean) value; closer.add(Prepare.THREAD_EXPAND.push(b)); } + if (propertyName.equals("insubquerythreshold")) { + int thresholdValue = ((BigDecimal) value).intValue(); + closer.add(Prepare.THREAD_INSUBQUERY_THRESHOLD.push(thresholdValue)); + } }) .withEnv(QuidemTest::getEnv) .build();