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")))
                         ))

Reply via email to