This is an automated email from the ASF dual-hosted git repository. alexpl pushed a commit to branch sql-calcite in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/sql-calcite by this push: new 9bd99d2 IGNITE-14597 ANY_VALUE aggregate - Fixes #9159. 9bd99d2 is described below commit 9bd99d2d4cc73dcbfaff0e3344ded0e681acf370 Author: Aleksey Plekhanov <plehanov.a...@gmail.com> AuthorDate: Fri Aug 20 16:31:04 2021 +0300 IGNITE-14597 ANY_VALUE aggregate - Fixes #9159. Signed-off-by: Aleksey Plekhanov <plehanov.a...@gmail.com> --- .../query/calcite/exec/exp/agg/Accumulators.java | 43 +++++++++++---- .../query/calcite/prepare/IgniteSqlValidator.java | 1 + .../integration/AbstractBasicIntegrationTest.java | 5 +- .../integration/AggregatesIntegrationTest.java | 61 +++++++++++++++++++++- .../aggregates/test_aggregate_types_scalar.test | 11 ++++ .../test_aggregate_types_scalar.test_ignored | 7 ++- .../sql/aggregate/aggregates/test_scalar_aggr.test | 16 +++++- .../aggregates/test_scalar_aggr.test_ignore | 4 +- .../test/sql/aggregate/group/test_group_by.test | 7 +++ .../sql/aggregate/group/test_group_by.test_ignore | 5 +- .../test/sql/types/decimal/decimal_aggregates.test | 15 ++++++ .../types/decimal/decimal_aggregates.test_ignored | 13 +++-- 12 files changed, 159 insertions(+), 29 deletions(-) diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/agg/Accumulators.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/agg/Accumulators.java index de3dc6b1..3d69449 100644 --- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/agg/Accumulators.java +++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/agg/Accumulators.java @@ -68,6 +68,8 @@ public class Accumulators { return maxFactory(call); case "SINGLE_VALUE": return SingleVal.FACTORY; + case "ANY_VALUE": + return AnyVal.FACTORY; default: throw new AssertionError(call.getAggregation().getName()); } @@ -163,10 +165,7 @@ public class Accumulators { } /** */ - private static class SingleVal implements Accumulator { - /** */ - private Object holder; - + private static class SingleVal extends AnyVal { /** */ private boolean touched; @@ -175,21 +174,47 @@ public class Accumulators { /** */ @Override public void add(Object... args) { - assert args.length == 1 : args.length; - if (touched) throw new IllegalArgumentException("Subquery returned more than 1 value."); touched = true; - holder = args[0]; + super.add(args); } /** */ @Override public void apply(Accumulator other) { - assert holder == null : "sudden apply for: " + other + " on SingleVal"; + if (((SingleVal)other).touched) { + if (touched) + throw new IllegalArgumentException("Subquery returned more than 1 value."); + else + touched = true; + } - holder = ((SingleVal)other).holder; + super.apply(other); + } + } + + /** */ + private static class AnyVal implements Accumulator { + /** */ + private Object holder; + + /** */ + public static final Supplier<Accumulator> FACTORY = AnyVal::new; + + /** */ + @Override public void add(Object... args) { + assert args.length == 1 : args.length; + + if (holder == null) + holder = args[0]; + } + + /** */ + @Override public void apply(Accumulator other) { + if (holder == null) + holder = ((AnyVal)other).holder; } /** */ diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java index e85dc53..cfe49b2 100644 --- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java +++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java @@ -284,6 +284,7 @@ public class IgniteSqlValidator extends SqlValidatorImpl { case AVG: case MIN: case MAX: + case ANY_VALUE: return; default: diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AbstractBasicIntegrationTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AbstractBasicIntegrationTest.java index 97a536e..8004e0d 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AbstractBasicIntegrationTest.java +++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AbstractBasicIntegrationTest.java @@ -40,9 +40,12 @@ public class AbstractBasicIntegrationTest extends GridCommonAbstractTest { /** */ protected static IgniteEx client; + /** */ + protected static final int GRID_CNT = 3; + /** {@inheritDoc} */ @Override protected void beforeTestsStarted() throws Exception { - startGrids(3); + startGrids(GRID_CNT); client = startClientGrid("client"); } diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AggregatesIntegrationTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AggregatesIntegrationTest.java index aa5d982..83e50c2 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AggregatesIntegrationTest.java +++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AggregatesIntegrationTest.java @@ -17,6 +17,11 @@ package org.apache.ignite.internal.processors.query.calcite.integration; +import java.util.List; +import org.apache.ignite.IgniteCache; +import org.apache.ignite.cache.query.FieldsQueryCursor; +import org.apache.ignite.internal.processors.query.QueryEngine; +import org.apache.ignite.internal.processors.query.calcite.util.Commons; import org.apache.ignite.testframework.GridTestUtils; import org.junit.Test; @@ -100,10 +105,14 @@ public class AggregatesIntegrationTest extends AbstractBasicIntegrationTest { } /** */ + @SuppressWarnings("ThrowableNotThrown") @Test - public void testMultipleRowsFromSingleAggr() { + public void testMultipleRowsFromSingleAggr() throws Exception { createAndPopulateTable(); + GridTestUtils.assertThrowsWithCause(() -> assertQuery("SELECT (SELECT name FROM person)").check(), + IllegalArgumentException.class); + GridTestUtils.assertThrowsWithCause(() -> assertQuery("SELECT t._key, (SELECT x FROM " + "TABLE(system_range(1, 5))) FROM person t").check(), IllegalArgumentException.class); @@ -111,5 +120,55 @@ public class AggregatesIntegrationTest extends AbstractBasicIntegrationTest { "TABLE(system_range(t._key, t._key + 1))) FROM person t").check(), IllegalArgumentException.class); assertQuery("SELECT t._key, (SELECT x FROM TABLE(system_range(t._key, t._key))) FROM person t").check(); + + // Check exception on reduce phase. + String cacheName = "person"; + + IgniteCache<Integer, Employer> person = client.cache(cacheName); + + person.clear(); + + for (int gridIdx = 0; gridIdx < GRID_CNT; gridIdx++) + person.put(primaryKey(grid(gridIdx).cache(cacheName)), new Employer(gridIdx == 0 ? "Emp" : null, 0.0d)); + + GridTestUtils.assertThrowsWithCause(() -> assertQuery("SELECT (SELECT name FROM person)").check(), + IllegalArgumentException.class); + + assertQuery("SELECT (SELECT name FROM person WHERE name is not null)").returns("Emp").check(); + } + + /** */ + @Test + public void testAnyValAggr() { + createAndPopulateTable(); + + List<List<?>> res = execute("select any_value(name) from person"); + + assertEquals(1, res.size()); + + Object val = res.get(0).get(0); + + assertTrue("Unexpected value: " + val, "Igor".equals(val) || "Roma".equals(val) || "Ilya".equals(val)); + + // Test with grouping. + res = execute("select any_value(name), salary from person group by salary order by salary"); + + assertEquals(2, res.size()); + + val = res.get(0).get(0); + + assertTrue("Unexpected value: " + val, "Igor".equals(val) || "Roma".equals(val)); + + val = res.get(1).get(0); + + assertEquals("Ilya", val); + } + + /** */ + private List<List<?>> execute(String sql) { + List<FieldsQueryCursor<List<?>>> cursors = Commons.lookupComponent(client.context(), QueryEngine.class) + .query(null, "PUBLIC", sql); + + return cursors.get(0).getAll(); } } diff --git a/modules/calcite/src/test/sql/aggregate/aggregates/test_aggregate_types_scalar.test b/modules/calcite/src/test/sql/aggregate/aggregates/test_aggregate_types_scalar.test index 89e5910..23875e7 100644 --- a/modules/calcite/src/test/sql/aggregate/aggregates/test_aggregate_types_scalar.test +++ b/modules/calcite/src/test/sql/aggregate/aggregates/test_aggregate_types_scalar.test @@ -61,6 +61,17 @@ SELECT MAX() statement error SELECT MAX(1, 2) +query IIRTTTT +SELECT ANY_VALUE(1), ANY_VALUE(NULL), ANY_VALUE(33.3), ANY_VALUE('hello'), ANY_VALUE(True), ANY_VALUE(DATE '1992-02-02'), ANY_VALUE(TIMESTAMP '2008-01-01 00:00:01') +---- +1 NULL 33.300000 hello true 1992-02-02 2008-01-01 00:00:01.0 + +statement error +SELECT ANY_VALUE() + +statement error +SELECT ANY_VALUE(1, 2) + query RRR SELECT AVG(1), AVG(NULL), AVG(33.3) ---- diff --git a/modules/calcite/src/test/sql/aggregate/aggregates/test_aggregate_types_scalar.test_ignored b/modules/calcite/src/test/sql/aggregate/aggregates/test_aggregate_types_scalar.test_ignored index 0c5156f..1e4fe03 100644 --- a/modules/calcite/src/test/sql/aggregate/aggregates/test_aggregate_types_scalar.test_ignored +++ b/modules/calcite/src/test/sql/aggregate/aggregates/test_aggregate_types_scalar.test_ignored @@ -1,7 +1,6 @@ # name: test/sql/aggregate/aggregates/test_aggregate_types_scalar.test # description: Test scalar aggregates with many different types # group: [aggregates] -# Ignore: https://issues.apache.org/jira/browse/IGNITE-14597 # Ignore: https://issues.apache.org/jira/browse/IGNITE-14636 query IIIII @@ -64,15 +63,15 @@ statement error SELECT MAX(1, 2) query IIRTTTT -SELECT FIRST(1), FIRST(NULL), FIRST(33.3), FIRST('hello'), FIRST(True), FIRST(DATE '1992-02-02'), FIRST(TIMESTAMP '2008-01-01 00:00:01') +SELECT ANY_VALUE(1), ANY_VALUE(NULL), ANY_VALUE(33.3), ANY_VALUE('hello'), ANY_VALUE(True), ANY_VALUE(DATE '1992-02-02'), ANY_VALUE(TIMESTAMP '2008-01-01 00:00:01') ---- 1 NULL 33.300000 hello true 1992-02-02 2008-01-01 00:00:01.0 statement error -SELECT FIRST() +SELECT ANY_VALUE() statement error -SELECT FIRST(1, 2) +SELECT ANY_VALUE(1, 2) query RRR SELECT AVG(1), AVG(NULL), AVG(33.3) diff --git a/modules/calcite/src/test/sql/aggregate/aggregates/test_scalar_aggr.test b/modules/calcite/src/test/sql/aggregate/aggregates/test_scalar_aggr.test index e6827e8..81d09b8 100644 --- a/modules/calcite/src/test/sql/aggregate/aggregates/test_scalar_aggr.test +++ b/modules/calcite/src/test/sql/aggregate/aggregates/test_scalar_aggr.test @@ -16,10 +16,22 @@ CREATE TABLE integers(i INTEGER); statement ok INSERT INTO integers VALUES (1), (2), (NULL) -query IIII -SELECT COUNT(1), MIN(1), MAX(1), SUM(1) FROM integers +query IIIII +SELECT COUNT(1), MIN(1), ANY_VALUE(1), MAX(1), SUM(1) FROM integers ---- 3 1 1 +1 3 + +# test aggregates on a set of values with scalar NULL values as inputs +query IIIII +SELECT COUNT(NULL), MIN(NULL), ANY_VALUE(NULL), MAX(NULL), SUM(NULL) FROM integers +---- +0 +NULL +NULL +NULL +NULL + diff --git a/modules/calcite/src/test/sql/aggregate/aggregates/test_scalar_aggr.test_ignore b/modules/calcite/src/test/sql/aggregate/aggregates/test_scalar_aggr.test_ignore index 808c224..7c2a8a4 100644 --- a/modules/calcite/src/test/sql/aggregate/aggregates/test_scalar_aggr.test_ignore +++ b/modules/calcite/src/test/sql/aggregate/aggregates/test_scalar_aggr.test_ignore @@ -37,7 +37,7 @@ statement ok INSERT INTO integers VALUES (1), (2), (NULL) query IIIIIT -SELECT COUNT(1), MIN(1), FIRST(1), MAX(1), SUM(1), STRING_AGG('hello', ',') FROM integers +SELECT COUNT(1), MIN(1), ANY_VALUE(1), MAX(1), SUM(1), STRING_AGG('hello', ',') FROM integers ---- 3 1 @@ -48,7 +48,7 @@ hello,hello,hello # test aggregates on a set of values with scalar NULL values as inputs query IIIIIT -SELECT COUNT(NULL), MIN(NULL), FIRST(NULL), MAX(NULL), SUM(NULL), STRING_AGG(NULL, NULL) FROM integers +SELECT COUNT(NULL), MIN(NULL), ANY_VALUE(NULL), MAX(NULL), SUM(NULL), STRING_AGG(NULL, NULL) FROM integers ---- 0 NULL diff --git a/modules/calcite/src/test/sql/aggregate/group/test_group_by.test b/modules/calcite/src/test/sql/aggregate/group/test_group_by.test index 539e525..335c7aa 100644 --- a/modules/calcite/src/test/sql/aggregate/group/test_group_by.test +++ b/modules/calcite/src/test/sql/aggregate/group/test_group_by.test @@ -106,6 +106,13 @@ SELECT i, i + 10 FROM integers GROUP BY i ORDER BY i statement error SELECT i, SUM(j), j FROM integers GROUP BY i ORDER BY i +# but it works if we wrap it in ANY_VALUE() +query IRI +SELECT i, SUM(j), ANY_VALUE(j) FROM integers GROUP BY i ORDER BY i +---- +2 4.000000 4 +3 8.000000 4 + # use an alias that is identical to a column name (should prioritize column name) query IR SELECT 1 AS i, SUM(i) FROM integers GROUP BY i ORDER BY 2; diff --git a/modules/calcite/src/test/sql/aggregate/group/test_group_by.test_ignore b/modules/calcite/src/test/sql/aggregate/group/test_group_by.test_ignore index cf53871..c9d7179 100644 --- a/modules/calcite/src/test/sql/aggregate/group/test_group_by.test_ignore +++ b/modules/calcite/src/test/sql/aggregate/group/test_group_by.test_ignore @@ -2,7 +2,6 @@ # description: Test aggregation/group by statements # group: [group] # Ignored: https://issues.apache.org/jira/browse/IGNITE-14885 -# Ignored: https://issues.apache.org/jira/browse/IGNITE-14597 statement ok PRAGMA enable_verification @@ -115,9 +114,9 @@ SELECT i, i + 10 FROM integers GROUP BY i ORDER BY i statement error SELECT i, SUM(j), j FROM integers GROUP BY i ORDER BY i -# but it works if we wrap it in FIRST() +# but it works if we wrap it in ANY_VALUE() query IRI -SELECT i, SUM(j), FIRST(j) FROM integers GROUP BY i ORDER BY i +SELECT i, SUM(j), ANY_VALUE(j) FROM integers GROUP BY i ORDER BY i ---- 2 4.000000 4 3 8.000000 4 diff --git a/modules/calcite/src/test/sql/types/decimal/decimal_aggregates.test b/modules/calcite/src/test/sql/types/decimal/decimal_aggregates.test index b4945d0..2681cef 100644 --- a/modules/calcite/src/test/sql/types/decimal/decimal_aggregates.test +++ b/modules/calcite/src/test/sql/types/decimal/decimal_aggregates.test @@ -5,6 +5,21 @@ statement ok PRAGMA enable_verification +# scalar aggregates +# first +query IIIII +SELECT ANY_VALUE(NULL::DECIMAL), + ANY_VALUE('0.1'::DECIMAL(4,1))::VARCHAR, + ANY_VALUE('4938245.1'::DECIMAL(9,1))::VARCHAR, + ANY_VALUE('45672564564938245.1'::DECIMAL(18,1))::VARCHAR, + ANY_VALUE('4567645908450368043562342564564938245.1'::DECIMAL(38,1))::VARCHAR +---- +NULL +.1 +4938245.1 +45672564564938245.1 +4567645908450368043562342564564938245.1 + # min query IIIII SELECT MIN(NULL::DECIMAL), diff --git a/modules/calcite/src/test/sql/types/decimal/decimal_aggregates.test_ignored b/modules/calcite/src/test/sql/types/decimal/decimal_aggregates.test_ignored index ef2f395..af4b860 100644 --- a/modules/calcite/src/test/sql/types/decimal/decimal_aggregates.test_ignored +++ b/modules/calcite/src/test/sql/types/decimal/decimal_aggregates.test_ignored @@ -3,24 +3,23 @@ # group: [decimal] # Ignore: https://issues.apache.org/jira/browse/IGNITE-14555 # Ignore: https://issues.apache.org/jira/browse/IGNITE-14596 -# Ignore: https://issues.apache.org/jira/browse/IGNITE-14597 statement ok PRAGMA enable_verification query I -SELECT typeof(FIRST('0.1'::DECIMAL(4,1))) +SELECT typeof(ANY_VALUE('0.1'::DECIMAL(4,1))) ---- DECIMAL(4,1) # scalar aggregates # first query IIIII -SELECT FIRST(NULL::DECIMAL), - FIRST('0.1'::DECIMAL(4,1))::VARCHAR, - FIRST('4938245.1'::DECIMAL(9,1))::VARCHAR, - FIRST('45672564564938245.1'::DECIMAL(18,1))::VARCHAR, - FIRST('4567645908450368043562342564564938245.1'::DECIMAL(38,1))::VARCHAR +SELECT ANY_VALUE(NULL::DECIMAL), + ANY_VALUE('0.1'::DECIMAL(4,1))::VARCHAR, + ANY_VALUE('4938245.1'::DECIMAL(9,1))::VARCHAR, + ANY_VALUE('45672564564938245.1'::DECIMAL(18,1))::VARCHAR, + ANY_VALUE('4567645908450368043562342564564938245.1'::DECIMAL(38,1))::VARCHAR ---- NULL 0.1