This is an automated email from the ASF dual-hosted git repository.
ppa pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 295a4f22eb IGNITE-20835 Sql. Fixed missing SINGLE_VALUE aggregate for
scalar sub-query (#4370)
295a4f22eb is described below
commit 295a4f22eb621a116c4c67774889db3703f86398
Author: Pavel Pereslegin <[email protected]>
AuthorDate: Wed Sep 18 15:25:48 2024 +0400
IGNITE-20835 Sql. Fixed missing SINGLE_VALUE aggregate for scalar sub-query
(#4370)
---
.../ignite/internal/sql/engine/ItDmlTest.java | 93 ++++++++++++++++++++++
.../sql/engine/prepare/IgniteSqlValidator.java | 11 ++-
.../planner/AbstractAggregatePlannerTest.java | 37 +++++++++
.../sql/engine/planner/AggregatePlannerTest.java | 17 +++-
.../planner/ColocatedHashAggregatePlannerTest.java | 17 +++-
.../planner/ColocatedSortAggregatePlannerTest.java | 17 +++-
.../sql/engine/planner/DynamicParametersTest.java | 2 +-
.../planner/MapReduceHashAggregatePlannerTest.java | 17 +++-
.../planner/MapReduceSortAggregatePlannerTest.java | 17 +++-
9 files changed, 220 insertions(+), 8 deletions(-)
diff --git
a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDmlTest.java
b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDmlTest.java
index 951a344d2e..9afe4cb15f 100644
---
a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDmlTest.java
+++
b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDmlTest.java
@@ -486,6 +486,39 @@ public class ItDmlTest extends BaseSqlIntegrationTest {
.check();
}
+ @Test
+ public void testMergeWithSubqueryExpression() {
+ sql("CREATE TABLE t0(ID INT PRIMARY KEY, VAL INT)");
+ sql("CREATE TABLE t1(ID INT PRIMARY KEY, VAL INT)");
+
+ String sql = "MERGE INTO t0 USING t1 ON t0.id = t1.id "
+ + "WHEN MATCHED THEN UPDATE SET val = (SELECT val FROM t1
WHERE id > ?)";
+
+ sql("INSERT INTO t0 VALUES (1, 0), (2, 0)");
+ sql("INSERT INTO t1 VALUES (1, -100), (3, 3)");
+
+ // sub-query returns no rows.
+ sql(sql, 3);
+ assertQuery("SELECT * FROM t0 ORDER BY id")
+ .returns(1, null)
+ .returns(2, 0)
+ .check();
+
+ // sub-query returns single row.
+ sql(sql, 1);
+ assertQuery("SELECT * FROM t0 ORDER BY id")
+ .returns(1, 3)
+ .returns(2, 0)
+ .check();
+
+ // sub-query returns more than one row.
+ assertThrowsSqlException(
+ Sql.RUNTIME_ERR,
+ "Subquery returned more than 1 value",
+ () -> sql(sql, 0)
+ );
+ }
+
/**
* Test verifies that scan is executed within provided transaction.
*/
@@ -685,6 +718,37 @@ public class ItDmlTest extends BaseSqlIntegrationTest {
}
}
+ @Test
+ public void testUpdateWithSubqueryExpression() {
+ sql("CREATE TABLE t0(ID INT PRIMARY KEY, VAL INT)");
+ sql("CREATE TABLE t1(ID INT PRIMARY KEY, VAL INT)");
+
+ sql("INSERT INTO t0 VALUES (1, 1), (2, 2)");
+ sql("INSERT INTO t1 VALUES (1, 1), (2, 2)");
+
+ // Sub-query returns no rows.
+ sql("UPDATE t0 SET val = (SELECT val FROM t1 WHERE id = -42)");
+ assertQuery("SELECT * FROM t0")
+ .returns(1, null)
+ .returns(2, null)
+ .check();
+
+ // Sub-query returns single row.
+ sql("UPDATE t0 SET val = (SELECT val FROM t1 WHERE id = 2)");
+ assertQuery("SELECT * FROM t0")
+ .returns(1, 2)
+ .returns(2, 2)
+ .check();
+
+ // Sub-query returns more than one row.
+ //noinspection ThrowableNotThrown
+ assertThrowsSqlException(
+ Sql.RUNTIME_ERR,
+ "Subquery returned more than 1 value",
+ () -> sql("UPDATE t0 SET val = (SELECT val FROM t1)")
+ );
+ }
+
@Test
public void testDropDefault() {
// SQL Standard 2016 feature F221 - Explicit defaults
@@ -848,6 +912,35 @@ public class ItDmlTest extends BaseSqlIntegrationTest {
}
}
+ @Test
+ public void testInsertValueWithSubqueryExpression() {
+ sql("CREATE TABLE t0(ID INT PRIMARY KEY, VAL INT)");
+ sql("CREATE TABLE t1(ID INT PRIMARY KEY, VAL INT)");
+
+ sql("INSERT INTO t1 VALUES (1, 1), (2, 2)");
+
+ // Sub-query returns no rows.
+ sql("INSERT INTO t0 VALUES (1, (SELECT val FROM t1 WHERE id = -42))");
+ assertQuery("SELECT * FROM t0")
+ .returns(1, null)
+ .check();
+
+ // Sub-query returns single row.
+ sql("INSERT INTO t0 VALUES (2, (SELECT val FROM t1 WHERE id = 2))");
+ assertQuery("SELECT * FROM t0")
+ .returns(1, null)
+ .returns(2, 2)
+ .check();
+
+ // Sub-query returns more than one row.
+ //noinspection ThrowableNotThrown
+ assertThrowsSqlException(
+ Sql.RUNTIME_ERR,
+ "Subquery returned more than 1 value",
+ () -> sql("INSERT INTO t0 VALUES (2, (SELECT val FROM t1))")
+ );
+ }
+
@ParameterizedTest
@CsvSource(value = {
"id1,id2; id1",
diff --git
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteSqlValidator.java
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteSqlValidator.java
index b39385b555..1afa9cef13 100644
---
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteSqlValidator.java
+++
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteSqlValidator.java
@@ -394,9 +394,16 @@ public class IgniteSqlValidator extends SqlValidatorImpl {
int startPosition = selectList.size() - sourceExprListSize;
for (var i = 0; i < sourceExprListSize; i++) {
- SqlNode sourceExpr = sourceExpressionList.get(i);
+ SqlNode replacement = sourceExpressionList.get(i);
int position = startPosition + i;
- selectList.set(position, sourceExpr);
+
+ // This method was introduced to replace an expression with an
expression that has the
+ // required type cast. Therefore, this only applies when the
replacement contains SqlBasicCall.
+ // For example a call with SCALAR_QUERY is only present in
sourceSelect, keeping original
+ // SqlSelect in sourceExpressionList, and we should not make a
replacement in this case.
+ if (replacement instanceof SqlBasicCall) {
+ selectList.set(position, replacement);
+ }
}
}
diff --git
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AbstractAggregatePlannerTest.java
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AbstractAggregatePlannerTest.java
index 46fd8f68ab..2d459de929 100644
---
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AbstractAggregatePlannerTest.java
+++
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AbstractAggregatePlannerTest.java
@@ -31,6 +31,7 @@ import java.util.function.Predicate;
import java.util.function.UnaryOperator;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.core.AggregateCall;
+import org.apache.calcite.sql.fun.SqlSingleValueAggFunction;
import
org.apache.ignite.internal.sql.engine.framework.TestBuilders.TableBuilder;
import org.apache.ignite.internal.sql.engine.rel.IgniteAggregate;
import org.apache.ignite.internal.sql.engine.rel.agg.IgniteReduceAggregateBase;
@@ -917,6 +918,36 @@ public abstract class AbstractAggregatePlannerTest extends
AbstractPlannerTest {
*/
CASE_26A("SELECT val0, val1, COUNT(*) cnt FROM test GROUP BY val0,
val1 ORDER BY val1 DESC",
schema(hash(0))),
+
+ /**
+ * Query: SELECT val0 FROM test WHERE val0 = (SELECT val1 FROM test).
+ *
+ * <p>Distribution single()
+ */
+ CASE_27("SELECT val0 FROM test WHERE val0 = (SELECT val1 FROM test)",
schema(single())),
+
+ /**
+ * Query: INSERT INTO test (id, val0) VALUES (1, (SELECT val1 FROM
test)).
+ *
+ * <p>Distribution single()
+ */
+ CASE_27A("INSERT INTO test (id, val0) VALUES (1, (SELECT val1 FROM
test))", schema(single())),
+
+ /**
+ * Query: UPDATE test set val0 = (SELECT val1 FROM test).
+ *
+ * <p>Distribution single()
+ */
+ CASE_27B("UPDATE test set val0 = (SELECT val1 FROM test)",
schema(single())),
+
+ /**
+ * Query: MERGE INTO test as t0 USING test as t1 ON t0.id = t1.id
+ * WHEN MATCHED THEN UPDATE SET val1 = (SELECT val0 FROM test)
+ *
+ * <p>Distribution single()
+ */
+ CASE_27C("MERGE INTO test as t0 USING test as t1 ON t0.id = t1.id "
+ + "WHEN MATCHED THEN UPDATE SET val1 = (SELECT val0 FROM
test)", schema(single())),
;
final String query;
@@ -1030,6 +1061,12 @@ public abstract class AbstractAggregatePlannerTest
extends AbstractPlannerTest {
return mapNode.or(reduceNode);
}
+ <T extends RelNode> Predicate<T> hasSingleValueAggregate() {
+ return (Predicate<T>) isInstanceOf(IgniteAggregate.class)
+ .and(n -> n.getAggCallList().stream()
+ .anyMatch(agg -> agg.getAggregation() instanceof
SqlSingleValueAggFunction));
+ }
+
<T extends RelNode> Predicate<T> hasDistinctAggregate() {
Predicate<T> mapNode = (Predicate<T>)
isInstanceOf(IgniteAggregate.class)
.and(n ->
n.getAggCallList().stream().anyMatch(NON_NULL_PREDICATE.and(AggregateCall::isDistinct)));
diff --git
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AggregatePlannerTest.java
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AggregatePlannerTest.java
index 5a53898104..b8bbcb140c 100644
---
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AggregatePlannerTest.java
+++
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/AggregatePlannerTest.java
@@ -254,6 +254,17 @@ public class AggregatePlannerTest extends
AbstractAggregatePlannerTest {
checkSimpleAggHash(TestCase.CASE_14B);
}
+ /**
+ * Validates that the SINGLE_VALUE aggregate is added for a sub-query
where a single value is expected.
+ */
+ @Test
+ public void subqueryWithSingleValueAggregate() throws Exception {
+ checkSimpleAggSingle(TestCase.CASE_27, hasSingleValueAggregate());
+ checkSimpleAggSingle(TestCase.CASE_27A, hasSingleValueAggregate());
+ checkSimpleAggSingle(TestCase.CASE_27B, hasSingleValueAggregate());
+ checkSimpleAggSingle(TestCase.CASE_27C, hasSingleValueAggregate());
+ }
+
/**
* Validates a plan for a query with DISTINCT aggregate in WHERE clause.
*/
@@ -569,9 +580,13 @@ public class AggregatePlannerTest extends
AbstractAggregatePlannerTest {
}
private void checkSimpleAggSingle(TestCase testCase) throws Exception {
+ checkSimpleAggSingle(testCase, hasAggregate());
+ }
+
+ private void checkSimpleAggSingle(TestCase testCase,
Predicate<IgniteColocatedHashAggregate> aggPredicate) throws Exception {
assertPlan(testCase,
nodeOrAnyChild(isInstanceOf(IgniteColocatedHashAggregate.class)
- .and(hasAggregate())
+ .and(aggPredicate)
.and(input(isTableScan("TEST")))
)
);
diff --git
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/ColocatedHashAggregatePlannerTest.java
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/ColocatedHashAggregatePlannerTest.java
index 2d7e800b30..85d3b8af08 100644
---
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/ColocatedHashAggregatePlannerTest.java
+++
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/ColocatedHashAggregatePlannerTest.java
@@ -236,6 +236,17 @@ public class ColocatedHashAggregatePlannerTest extends
AbstractAggregatePlannerT
checkSimpleAggHash(TestCase.CASE_14B);
}
+ /**
+ * Validates that the SINGLE_VALUE aggregate is added for a sub-query
where a single value is expected.
+ */
+ @Test
+ public void subqueryWithSingleValueAggregate() throws Exception {
+ checkSimpleAggSingle(TestCase.CASE_27, hasSingleValueAggregate());
+ checkSimpleAggSingle(TestCase.CASE_27A, hasSingleValueAggregate());
+ checkSimpleAggSingle(TestCase.CASE_27B, hasSingleValueAggregate());
+ checkSimpleAggSingle(TestCase.CASE_27C, hasSingleValueAggregate());
+ }
+
/**
* Validates a plan for a query with DISTINCT aggregate in WHERE clause.
*/
@@ -519,9 +530,13 @@ public class ColocatedHashAggregatePlannerTest extends
AbstractAggregatePlannerT
}
private void checkSimpleAggSingle(TestCase testCase) throws Exception {
+ checkSimpleAggSingle(testCase, hasAggregate());
+ }
+
+ private void checkSimpleAggSingle(TestCase testCase,
Predicate<IgniteColocatedHashAggregate> aggPredicate) throws Exception {
assertPlan(testCase,
nodeOrAnyChild(isInstanceOf(IgniteColocatedHashAggregate.class)
- .and(hasAggregate())
+ .and(aggPredicate)
.and(input(isTableScan("TEST")))
),
disableRules
diff --git
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/ColocatedSortAggregatePlannerTest.java
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/ColocatedSortAggregatePlannerTest.java
index eb6c58922d..b8591746fd 100644
---
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/ColocatedSortAggregatePlannerTest.java
+++
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/ColocatedSortAggregatePlannerTest.java
@@ -243,6 +243,17 @@ public class ColocatedSortAggregatePlannerTest extends
AbstractAggregatePlannerT
checkSimpleAggHash(TestCase.CASE_14B);
}
+ /**
+ * Validates that the SINGLE_VALUE aggregate is added for a sub-query
where a single value is expected.
+ */
+ @Test
+ public void subqueryWithSingleValueAggregate() throws Exception {
+ checkSimpleAggSingle(TestCase.CASE_27, hasSingleValueAggregate());
+ checkSimpleAggSingle(TestCase.CASE_27A, hasSingleValueAggregate());
+ checkSimpleAggSingle(TestCase.CASE_27B, hasSingleValueAggregate());
+ checkSimpleAggSingle(TestCase.CASE_27C, hasSingleValueAggregate());
+ }
+
/**
* Validates a plan for a query with DISTINCT aggregate in WHERE clause.
*/
@@ -513,9 +524,13 @@ public class ColocatedSortAggregatePlannerTest extends
AbstractAggregatePlannerT
}
private void checkSimpleAggSingle(TestCase testCase) throws Exception {
+ checkSimpleAggSingle(testCase, hasAggregate());
+ }
+
+ private void checkSimpleAggSingle(TestCase testCase,
Predicate<IgniteColocatedSortAggregate> aggPredicate) throws Exception {
assertPlan(testCase,
nodeOrAnyChild(isInstanceOf(IgniteColocatedSortAggregate.class)
- .and(hasAggregate())
+ .and(aggPredicate)
// Without GROUP BY sort can be omitted.
.and(input(isTableScan("TEST")))
),
diff --git
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/DynamicParametersTest.java
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/DynamicParametersTest.java
index 1649609dfb..6af36ffbd8 100644
---
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/DynamicParametersTest.java
+++
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/DynamicParametersTest.java
@@ -351,7 +351,7 @@ public class DynamicParametersTest extends
AbstractPlannerTest {
.sql("UPDATE t1 SET c1 = (SELECT ?)", 1)
.ok()
- // TODO https://issues.apache.org/jira/browse/IGNITE-20835
+ // TODO https://issues.apache.org/jira/browse/IGNITE-23183
// Assertion is caused by incorrect subquery handling
// checkStatement()
// .table("t1", "c1", NativeTypes.INT32, "c2",
NativeTypes.INT64)
diff --git
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/MapReduceHashAggregatePlannerTest.java
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/MapReduceHashAggregatePlannerTest.java
index 0a4c52d1de..6c2ed5bef9 100644
---
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/MapReduceHashAggregatePlannerTest.java
+++
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/MapReduceHashAggregatePlannerTest.java
@@ -243,6 +243,17 @@ public class MapReduceHashAggregatePlannerTest extends
AbstractAggregatePlannerT
checkSimpleAggHash(TestCase.CASE_14B);
}
+ /**
+ * Validates that the SINGLE_VALUE aggregate is added for a sub-query
where a single value is expected.
+ */
+ @Test
+ public void subqueryWithSingleValueAggregate() throws Exception {
+ checkSimpleAggSingle(TestCase.CASE_27, hasSingleValueAggregate());
+ checkSimpleAggSingle(TestCase.CASE_27A, hasSingleValueAggregate());
+ checkSimpleAggSingle(TestCase.CASE_27B, hasSingleValueAggregate());
+ checkSimpleAggSingle(TestCase.CASE_27C, hasSingleValueAggregate());
+ }
+
/**
* Validates a plan for a query with DISTINCT aggregate in WHERE clause.
*/
@@ -565,10 +576,14 @@ public class MapReduceHashAggregatePlannerTest extends
AbstractAggregatePlannerT
}
private void checkSimpleAggSingle(TestCase testCase) throws Exception {
+ checkSimpleAggSingle(testCase, hasAggregate());
+ }
+
+ private void checkSimpleAggSingle(TestCase testCase,
Predicate<IgniteMapHashAggregate> aggPredicate) throws Exception {
assertPlan(testCase,
nodeOrAnyChild(isInstanceOf(IgniteReduceHashAggregate.class)
.and(input(isInstanceOf(IgniteMapHashAggregate.class)
- .and(hasAggregate())
+ .and(aggPredicate)
.and(input(isTableScan("TEST")))
))
),
diff --git
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/MapReduceSortAggregatePlannerTest.java
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/MapReduceSortAggregatePlannerTest.java
index 0ca318b131..89818a8a3c 100644
---
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/MapReduceSortAggregatePlannerTest.java
+++
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/planner/MapReduceSortAggregatePlannerTest.java
@@ -245,6 +245,17 @@ public class MapReduceSortAggregatePlannerTest extends
AbstractAggregatePlannerT
checkSimpleAggHash(TestCase.CASE_14B);
}
+ /**
+ * Validates that the SINGLE_VALUE aggregate is added for a sub-query
where a single value is expected.
+ */
+ @Test
+ public void subqueryWithSingleValueAggregate() throws Exception {
+ checkSimpleAggSingle(TestCase.CASE_27, hasSingleValueAggregate());
+ checkSimpleAggSingle(TestCase.CASE_27A, hasSingleValueAggregate());
+ checkSimpleAggSingle(TestCase.CASE_27B, hasSingleValueAggregate());
+ checkSimpleAggSingle(TestCase.CASE_27C, hasSingleValueAggregate());
+ }
+
/**
* Validates a plan for a query with DISTINCT aggregate in WHERE clause.
*/
@@ -573,10 +584,14 @@ public class MapReduceSortAggregatePlannerTest extends
AbstractAggregatePlannerT
}
private void checkSimpleAggSingle(TestCase testCase) throws Exception {
+ checkSimpleAggSingle(testCase, hasAggregate());
+ }
+
+ private void checkSimpleAggSingle(TestCase testCase,
Predicate<IgniteMapSortAggregate> aggPredicate) throws Exception {
assertPlan(testCase,
nodeOrAnyChild(isInstanceOf(IgniteReduceSortAggregate.class)
.and(input(isInstanceOf(IgniteMapSortAggregate.class)
- .and(hasAggregate())
+ .and(aggPredicate)
.and(hasGroups())
.and(input(isTableScan("TEST")))
))