Skip to content

Commit

Permalink
[CALCITE-4758] When SOME sub-query is SqlNodeList and converted to VA…
Browse files Browse the repository at this point in the history 
…LUES, Calcite returns incorrect result
  • Loading branch information
@NobiGo
NobiGo committed Dec 6, 2024
1 parent 0eb83b1 commit 84ea4be
Show file tree
Hide file tree
Showing 5 changed files with 220 additions and 106 deletions.
76 changes: 53 additions & 23 deletions core/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1207,9 +1207,7 @@ private void substituteSubQuery(Blackboard bb, SubQuery subQuery) {
case ALL:
call = (SqlBasicCall) subQuery.node;
query = call.operand(1);
if (!config.isExpand() && !(query instanceof SqlNodeList)) {
return;
}

final SqlNode leftKeyNode = call.operand(0);

final List<SqlNode> leftSqlKeys;
Expand Down Expand Up @@ -1241,6 +1239,31 @@ private void substituteSubQuery(Blackboard bb, SubQuery subQuery) {
// reference to Q below.
}

final RelDataType targetRowType =
promoteToRowType(typeFactory,
validator().getValidatedNodeType(leftKeyNode), null);

if (!config.isExpand()) {
if (query instanceof SqlNodeList) {
// convert
// select * from "scott".emp where sal > some (4000, 2000)
// to
// select * from "scott".emp where sal > some (VALUES (4000), (2000))
// The SqlNodeList become a RexSubQuery then optimized by SubQueryRemoveRule.
RelNode relNode = convertRowValues(bb, query, (SqlNodeList) query, false, targetRowType);
final ImmutableList.Builder<RexNode> builder =
ImmutableList.builder();
for (SqlNode node : leftSqlKeys) {
builder.add(bb.convertExpression(node));
}
final ImmutableList<RexNode> list = builder.build();
assert relNode != null;
subQuery.expr = createSubquery(subQuery.node.getKind(), relNode, list, call);
return;
}
return;
}

final List<RexNode> leftKeys = leftSqlKeys.stream()
.map(bb::convertExpression)
.collect(toImmutableList());
Expand Down Expand Up @@ -1278,9 +1301,7 @@ private void substituteSubQuery(Blackboard bb, SubQuery subQuery) {
if (bb.root == null) {
return;
}
final RelDataType targetRowType =
promoteToRowType(typeFactory,
validator().getValidatedNodeType(leftKeyNode), null);

final boolean notIn = call.getOperator().kind == SqlKind.NOT_IN;
converted =
convertExists(query, RelOptUtil.SubQueryType.IN, subQuery.logic,
Expand Down Expand Up @@ -5625,23 +5646,7 @@ ImmutableList<RelNode> retrieveCursors() {
if (correlationUse != null) {
rel = correlationUse.r;
}

switch (kind) {
case IN:
return RexSubQuery.in(rel, list);
case NOT_IN:
return rexBuilder.makeCall(SqlStdOperatorTable.NOT,
RexSubQuery.in(rel, list));
case SOME:
return RexSubQuery.some(rel, list,
(SqlQuantifyOperator) call.getOperator());
case ALL:
return rexBuilder.makeCall(SqlStdOperatorTable.NOT,
RexSubQuery.some(rel, list,
negate((SqlQuantifyOperator) call.getOperator())));
default:
throw new AssertionError(kind);
}
return createSubquery(kind, rel, list, call);
}
break;

Expand Down Expand Up @@ -5994,6 +5999,31 @@ public List<SqlMonotonicity> getColumnMonotonicities() {
}
}

/**
* Creates an sub-query of type {@code IN, NOT IN, SOME, or ALL}.
*/
private RexNode createSubquery(SqlKind kind,
RelNode rel, ImmutableList<RexNode> list, @Nullable SqlCall call) {
switch (kind) {
case IN:
return RexSubQuery.in(rel, list);
case NOT_IN:
return rexBuilder.makeCall(SqlStdOperatorTable.NOT,
RexSubQuery.in(rel, list));
case SOME:
assert call != null;
return RexSubQuery.some(rel, list,
(SqlQuantifyOperator) call.getOperator());
case ALL:
assert call != null;
return rexBuilder.makeCall(SqlStdOperatorTable.NOT,
RexSubQuery.some(rel, list,
negate((SqlQuantifyOperator) call.getOperator())));
default:
throw new AssertionError(kind);
}
}

private static SqlQuantifyOperator negate(SqlQuantifyOperator operator) {
assert operator.kind == SqlKind.ALL;
return SqlStdOperatorTable.some(operator.comparisonKind.negateNullSafe());
Expand Down
44 changes: 14 additions & 30 deletions core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4751,49 +4751,33 @@ private void checkLiteral2(String expression, String expected) {
@Test void convertInListToValues1() {
String query = "select \"product_id\" from \"product\"\n"
+ "where \"product_id\" in (12, null)";
String expected = "SELECT \"product\".\"product_id\"\n"
String expected = "SELECT \"product_id\"\n"
+ "FROM \"foodmart\".\"product\"\n"
+ "INNER JOIN (SELECT \"ROW_VALUE\"\n"
+ "FROM (VALUES (12),\n(NULL)) AS \"t\" (\"ROW_VALUE\")\n"
+ "GROUP BY \"ROW_VALUE\") AS \"t0\" ON \"product\".\"product_id\" = \"t0\".\"ROW_VALUE\"";
+ "WHERE \"product_id\" IN (SELECT *\n"
+ "FROM (VALUES (12),\n"
+ "(NULL)) AS \"t\" (\"ROW_VALUE\"))";
sql(query).withConfig(c -> c.withInSubQueryThreshold(1)).ok(expected);
}

@Test void convertInListToValues2() {
String query = "select \"brand_name\" from \"product\"\n"
+ "where cast(\"brand_name\" as char) in ('n', null)";
String expected = "SELECT \"t\".\"brand_name\"\n"
+ "FROM (SELECT \"product_class_id\", \"product_id\","
+ " \"brand_name\", \"product_name\","
+ " \"SKU\", \"SRP\", \"gross_weight\","
+ " \"net_weight\", \"recyclable_package\","
+ " \"low_fat\", \"units_per_case\","
+ " \"cases_per_pallet\", \"shelf_width\","
+ " \"shelf_height\", \"shelf_depth\","
+ " CAST(\"brand_name\" AS CHAR(1) CHARACTER SET \"ISO-8859-1\") AS \"brand_name0\"\n"
+ "FROM \"foodmart\".\"product\") AS \"t\"\n"
+ "INNER JOIN (SELECT \"ROW_VALUE\"\n"
+ "FROM (VALUES ('n'),\n(NULL)) AS \"t0\" (\"ROW_VALUE\")\n"
+ "GROUP BY \"ROW_VALUE\") AS \"t1\" ON \"t\".\"brand_name0\" = \"t1\".\"ROW_VALUE\"";
String expected = "SELECT \"brand_name\"\n"
+ "FROM \"foodmart\".\"product\"\n"
+ "WHERE CAST(\"brand_name\" AS CHAR(1) CHARACTER SET \"ISO-8859-1\") IN (SELECT *\n"
+ "FROM (VALUES ('n'),\n"
+ "(NULL)) AS \"t\" (\"ROW_VALUE\"))";
sql(query).withConfig(c -> c.withInSubQueryThreshold(1)).ok(expected);
}

@Test void convertInListToValues3() {
String query = "select \"brand_name\" from \"product\"\n"
+ "where (\"brand_name\" = \"product_name\") in (false, null)";
String expected = "SELECT \"t\".\"brand_name\"\n"
+ "FROM (SELECT \"product_class_id\", \"product_id\","
+ " \"brand_name\", \"product_name\","
+ " \"SKU\", \"SRP\", \"gross_weight\","
+ " \"net_weight\", \"recyclable_package\","
+ " \"low_fat\", \"units_per_case\","
+ " \"cases_per_pallet\", \"shelf_width\","
+ " \"shelf_height\", \"shelf_depth\","
+ " \"brand_name\" = \"product_name\" AS \"$f15\"\n"
+ "FROM \"foodmart\".\"product\") AS \"t\"\n"
+ "INNER JOIN (SELECT \"ROW_VALUE\"\n"
+ "FROM (VALUES (FALSE),\n(NULL)) AS \"t0\" (\"ROW_VALUE\")\n"
+ "GROUP BY \"ROW_VALUE\") AS \"t1\" ON \"t\".\"$f15\" = \"t1\".\"ROW_VALUE\"";
String expected = "SELECT \"brand_name\"\n"
+ "FROM \"foodmart\".\"product\"\n"
+ "WHERE (\"brand_name\" = \"product_name\") IN (SELECT *\n"
+ "FROM (VALUES (FALSE),\n"
+ "(NULL)) AS \"t\" (\"ROW_VALUE\"))";
sql(query).withConfig(c -> c.withInSubQueryThreshold(1)).ok(expected);
}

Expand Down
5 changes: 5 additions & 0 deletions core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3148,6 +3148,7 @@ private void checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
+ "from t1\n"
+ "where (t1.a, t1.y) in ((1, 2), (3, null), (7369, null), (7499, 30), (null, 20), (null, 5))";
sql(sql)
.withExpand(true)
.withRelBuilderConfig(b -> b.withSimplifyValues(false))
.withRule(CoreRules.PROJECT_VALUES_MERGE,
CoreRules.UNION_TO_VALUES)
Expand All @@ -3163,6 +3164,7 @@ private void checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
+ "from t1\n"
+ "where (t1.a, t1.y) in ((cast(1.1 as int), 2), (3, null), (7369, null), (7499, 30), (null, cast(20.2 as int)), (null, 5))";
sql(sql)
.withExpand(true)
.withRelBuilderConfig(b -> b.withSimplifyValues(false))
.withRule(CoreRules.PROJECT_VALUES_MERGE,
CoreRules.UNION_TO_VALUES)
Expand All @@ -3173,6 +3175,7 @@ private void checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
@Test void testUnionToValuesByInList3() {
final String sql = "select * from dept where deptno in (12, 34, cast(56.4 as int))";
sql(sql)
.withExpand(true)
.withRelBuilderConfig(b -> b.withSimplifyValues(false))
.withRule(CoreRules.PROJECT_VALUES_MERGE,
CoreRules.UNION_TO_VALUES)
Expand All @@ -3183,6 +3186,7 @@ private void checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
@Test void testUnionToValuesByInList4() {
final String sql = "select * from dept where deptno in (12, 34, cast(56.4 as double))";
sql(sql)
.withExpand(true)
.withRelBuilderConfig(b -> b.withSimplifyValues(false))
.withRule(CoreRules.PROJECT_VALUES_MERGE,
CoreRules.UNION_TO_VALUES)
Expand All @@ -3193,6 +3197,7 @@ private void checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
@Test void testUnionToValuesByInList5() {
final String sql = "select deptno in (12, 34, cast(56.4 as double)) from dept";
sql(sql)
.withExpand(true)
.withRelBuilderConfig(b -> b.withSimplifyValues(false))
.withRule(CoreRules.PROJECT_VALUES_MERGE,
CoreRules.UNION_TO_VALUES)
Expand Down
86 changes: 86 additions & 0 deletions core/src/test/resources/sql/some.iq
View file
Original file line number Diff line number Diff line change
Expand Up @@ -847,5 +847,91 @@ where sal > all (select comm from "scott".emp where comm is not null);

!ok

# [CALCITE-4758] When SOME sub-query is SqlNodeList and converted to VALUES, Calcite returns incorrect result

# make sure the SqlNodeList converted to VALUES not OR condition
!set insubquerythreshold 0

select * from "scott".emp
where sal > all(500, 2000);
+-------+-------+-----------+------+------------+---------+------+--------+
| EMPNO | ENAME | JOB | MGR | HIREDATE | SAL | COMM | DEPTNO |
+-------+-------+-----------+------+------------+---------+------+--------+
| 7566 | JONES | MANAGER | 7839 | 1981-02-04 | 2975.00 | | 20 |
| 7698 | BLAKE | MANAGER | 7839 | 1981-01-05 | 2850.00 | | 30 |
| 7782 | CLARK | MANAGER | 7839 | 1981-06-09 | 2450.00 | | 10 |
| 7788 | SCOTT | ANALYST | 7566 | 1987-04-19 | 3000.00 | | 20 |
| 7839 | KING | PRESIDENT | | 1981-11-17 | 5000.00 | | 10 |
| 7902 | FORD | ANALYST | 7566 | 1981-12-03 | 3000.00 | | 20 |
+-------+-------+-----------+------+------------+---------+------+--------+
(6 rows)

!ok

select * from "scott".emp
where sal > all (4000, 2000);
+-------+-------+-----------+-----+------------+---------+------+--------+
| EMPNO | ENAME | JOB | MGR | HIREDATE | SAL | COMM | DEPTNO |
+-------+-------+-----------+-----+------------+---------+------+--------+
| 7839 | KING | PRESIDENT | | 1981-11-17 | 5000.00 | | 10 |
+-------+-------+-----------+-----+------------+---------+------+--------+
(1 row)

!ok

select * from "scott".emp
where sal > some (4000, 2000);
+-------+-------+-----------+------+------------+---------+------+--------+
| EMPNO | ENAME | JOB | MGR | HIREDATE | SAL | COMM | DEPTNO |
+-------+-------+-----------+------+------------+---------+------+--------+
| 7566 | JONES | MANAGER | 7839 | 1981-02-04 | 2975.00 | | 20 |
| 7698 | BLAKE | MANAGER | 7839 | 1981-01-05 | 2850.00 | | 30 |
| 7782 | CLARK | MANAGER | 7839 | 1981-06-09 | 2450.00 | | 10 |
| 7788 | SCOTT | ANALYST | 7566 | 1987-04-19 | 3000.00 | | 20 |
| 7839 | KING | PRESIDENT | | 1981-11-17 | 5000.00 | | 10 |
| 7902 | FORD | ANALYST | 7566 | 1981-12-03 | 3000.00 | | 20 |
+-------+-------+-----------+------+------------+---------+------+--------+
(6 rows)

!ok

select sal, sal > some (4000, 2000, null) from "scott".emp;
+---------+--------+
| SAL | EXPR$1 |
+---------+--------+
| 1100.00 | |
| 1250.00 | |
| 1250.00 | |
| 1300.00 | |
| 1500.00 | |
| 1600.00 | |
| 2450.00 | true |
| 2850.00 | true |
| 2975.00 | true |
| 3000.00 | true |
| 3000.00 | true |
| 5000.00 | true |
| 800.00 | |
| 950.00 | |
+---------+--------+
(14 rows)

!ok

select * from "scott".emp
where sal > any (4000, 2000);
+-------+-------+-----------+------+------------+---------+------+--------+
| EMPNO | ENAME | JOB | MGR | HIREDATE | SAL | COMM | DEPTNO |
+-------+-------+-----------+------+------------+---------+------+--------+
| 7566 | JONES | MANAGER | 7839 | 1981-02-04 | 2975.00 | | 20 |
| 7698 | BLAKE | MANAGER | 7839 | 1981-01-05 | 2850.00 | | 30 |
| 7782 | CLARK | MANAGER | 7839 | 1981-06-09 | 2450.00 | | 10 |
| 7788 | SCOTT | ANALYST | 7566 | 1987-04-19 | 3000.00 | | 20 |
| 7839 | KING | PRESIDENT | | 1981-11-17 | 5000.00 | | 10 |
| 7902 | FORD | ANALYST | 7566 | 1981-12-03 | 3000.00 | | 20 |
+-------+-------+-----------+------+------------+---------+------+--------+
(6 rows)

!ok
# End some.iq

Loading

0 comments on commit 84ea4be

Please sign in to comment.