From 0cc480e7857e828fcab06f89cfdc63ef9e2ddeb7 Mon Sep 17 00:00:00 2001 From: Vladimir Steshin Date: Thu, 19 Sep 2024 13:59:41 +0300 Subject: [PATCH 1/4] research --- .../calcite/prepare/IgniteSqlValidator.java | 52 +++++++++++- .../query/calcite/util/IgniteResource.java | 3 + .../AggregatesIntegrationTest.java | 48 +++++------ .../calcite/integration/DataTypesTest.java | 6 ++ .../DynamicParametersIntegrationTest.java | 85 +++++++++++++++++++ .../MemoryQuotasIntegrationTest.java | 18 ++++ .../hints/ForceIndexHintPlannerTest.java | 12 +++ .../testsuites/IgniteCalciteTestSuite.java | 4 +- .../ignite/testsuites/ScriptTestSuite.java | 3 +- .../aggregates/test_aggr_string.test | 40 ++++++++- 10 files changed, 241 insertions(+), 30 deletions(-) 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 2565a7bdfd1d1..7fcde0bce6a50 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 @@ -45,6 +45,7 @@ import org.apache.calcite.sql.SqlNodeList; import org.apache.calcite.sql.SqlNumericLiteral; import org.apache.calcite.sql.SqlOperatorTable; +import org.apache.calcite.sql.SqlOrderBy; import org.apache.calcite.sql.SqlSelect; import org.apache.calcite.sql.SqlUpdate; import org.apache.calcite.sql.SqlUtil; @@ -101,8 +102,11 @@ public class IgniteSqlValidator extends SqlValidatorImpl { HUMAN_READABLE_ALIASES_FOR = Collections.unmodifiableSet(kinds); } - /** Dynamic parameters. */ - Object[] parameters; + /** Passed query arguments. */ + private Object[] parameters; + + /** Number of query's dynamic parameters. */ + private int dynamicParamsCnt; /** * Creates a validator. @@ -522,8 +526,52 @@ private boolean isSystemFieldName(String alias) { || QueryUtils.VAL_FIELD_NAME.equalsIgnoreCase(alias); } + /** {@inheritDoc} */ + @Override public SqlNode validate(SqlNode topNode) { + extractDynamicParameters(topNode); + + SqlNode res = super.validate(topNode); + + if (dynamicParamsCnt != parameters.length) + throw newValidationError(res, IgniteResource.INSTANCE.unexpectedParameter(parameters.length, dynamicParamsCnt)); + + return res; + } + + /** */ + private void checkDynamicParametersNumber(SqlDynamicParam dynamicParam) { + if (dynamicParam.getIndex() >= dynamicParamsCnt) + dynamicParamsCnt = dynamicParam.getIndex() + 1; + } + + /** */ + private void extractDynamicParameters(SqlNode node) { + if (node instanceof SqlDynamicParam) + checkDynamicParametersNumber((SqlDynamicParam)node); + if (node instanceof SqlOrderBy) { + SqlOrderBy orderBy = (SqlOrderBy)node; + + if (orderBy.offset instanceof SqlDynamicParam) + checkDynamicParametersNumber((SqlDynamicParam)orderBy.offset); + + if (orderBy.fetch instanceof SqlDynamicParam) + checkDynamicParametersNumber((SqlDynamicParam)orderBy.fetch); + } + else if (node instanceof SqlSelect) { + SqlSelect select = (SqlSelect)node; + + if (select.getOffset() instanceof SqlDynamicParam) + checkDynamicParametersNumber((SqlDynamicParam)select.getOffset()); + + if (select.getFetch() instanceof SqlDynamicParam) + checkDynamicParametersNumber((SqlDynamicParam)select.getFetch()); + } + } + /** {@inheritDoc} */ @Override protected void inferUnknownTypes(RelDataType inferredType, SqlValidatorScope scope, SqlNode node) { + extractDynamicParameters(node); + if (node instanceof SqlDynamicParam && inferredType.equals(unknownType)) { if (parameters.length > ((SqlDynamicParam)node).getIndex()) { Object param = parameters[((SqlDynamicParam)node).getIndex()]; diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/IgniteResource.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/IgniteResource.java index fb71451894fbe..c09e59d878f6b 100644 --- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/IgniteResource.java +++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/IgniteResource.java @@ -72,4 +72,7 @@ public interface IgniteResource { /** */ @Resources.BaseMessage("Modify operation is not supported for table ''{0}''") Resources.ExInst modifyTableNotSupported(String table); + + @Resources.BaseMessage("Unexpected number of query parameters. Provided {0} but there is only {1} dynamic parameter(s).") + Resources.ExInst unexpectedParameter(int provided, int expected); } 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 4b910029caef1..7304c120d0784 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 @@ -113,31 +113,31 @@ public void testCountWithBackupsAndCacheModes() { public void testCountIndexedField() { createAndPopulateIndexedTable(1, CacheMode.PARTITIONED); - assertQuery("select count(salary) from person").returns(4L).check(); - assertQuery("select count(descVal) from person").returns(4L).check(); - assertQuery("select count(salary + 1) from person").returns(4L).check(); +// assertQuery("select count(salary) from person").returns(4L).check(); +// assertQuery("select count(descVal) from person").returns(4L).check(); +// assertQuery("select count(salary + 1) from person").returns(4L).check(); assertQuery("select count(distinct descVal) from person").returns(3L).check(); - assertQuery("select count(salary) from person where salary >= 5").returns(2L).check(); - assertQuery("select count(salary) filter (where salary >= 5) from person").returns(2L).check(); - assertQuery("select count(salary), descVal from person group by descVal") - .returns(1L, 1d) - .returns(1L, 9d) - .returns(1L, 15d) - .returns(1L, null) - .check(); - - // Check count with two columns index. - sql("CREATE TABLE tbl (a INT, b INT, c INT)"); - sql("CREATE INDEX idx_a ON tbl(a, c)"); - sql("CREATE INDEX idx_b ON tbl(b DESC, c)"); - - for (int i = 0; i < 100; i++) { - sql("INSERT INTO tbl VALUES (null, null, ?)", i % 2 == 0 ? i : null); - sql("INSERT INTO tbl VALUES (?, ?, ?)", i, i, i % 2 == 0 ? null : i); - } - - assertQuery("SELECT COUNT(a) FROM tbl").returns(100L).check(); - assertQuery("SELECT COUNT(b) FROM tbl").returns(100L).check(); +// assertQuery("select count(salary) from person where salary >= 5").returns(2L).check(); +// assertQuery("select count(salary) filter (where salary >= 5) from person").returns(2L).check(); +// assertQuery("select count(salary), descVal from person group by descVal") +// .returns(1L, 1d) +// .returns(1L, 9d) +// .returns(1L, 15d) +// .returns(1L, null) +// .check(); +// +// // Check count with two columns index. +// sql("CREATE TABLE tbl (a INT, b INT, c INT)"); +// sql("CREATE INDEX idx_a ON tbl(a, c)"); +// sql("CREATE INDEX idx_b ON tbl(b DESC, c)"); +// +// for (int i = 0; i < 100; i++) { +// sql("INSERT INTO tbl VALUES (null, null, ?)", i % 2 == 0 ? i : null); +// sql("INSERT INTO tbl VALUES (?, ?, ?)", i, i, i % 2 == 0 ? null : i); +// } +// +// assertQuery("SELECT COUNT(a) FROM tbl").returns(100L).check(); +// assertQuery("SELECT COUNT(b) FROM tbl").returns(100L).check(); } /** */ diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DataTypesTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DataTypesTest.java index 23474698fe533..9415e4425d625 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DataTypesTest.java +++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DataTypesTest.java @@ -415,6 +415,12 @@ public void testBinaryConcat() { } } + /** */ + @Test + public void test0(){ + assertQuery("SELECT COALESCE(?, ?)").withParams("a").returns("a").check(); + } + /** */ @Test public void testDecimalScale() { diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java index 89fba933b5bdc..ed5cd70fe909d 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java +++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java @@ -26,6 +26,7 @@ import java.time.Period; import java.util.List; import java.util.UUID; +import org.apache.calcite.runtime.CalciteContextException; import org.apache.ignite.internal.util.typedef.F; import org.junit.Test; @@ -33,6 +34,20 @@ * Dynamic parameters types inference test. */ public class DynamicParametersIntegrationTest extends AbstractBasicIntegrationTest { + /** {@inheritDoc} */ + @Override public void beforeTest() throws Exception { + super.beforeTest(); + + sql("CREATE TABLE t1 (id INTEGER PRIMARY KEY, val1 INTEGER NOT NULL, val2 INTEGER)"); + } + + /** {@inheritDoc} */ + @Override public void afterTest() throws Exception { + sql("DROP TABLE IF EXISTS t1"); + + super.afterTest(); + } + /** */ @Test public void testMetadataTypesForDynamicParameters() { @@ -109,4 +124,74 @@ public void testWithDifferentParametersTypes() { assertQuery("SELECT UPPER(TYPEOF(?))").withParams(1).returns("INTEGER").check(); assertQuery("SELECT UPPER(TYPEOF(?))").withParams(1d).returns("DOUBLE").check(); } + + /** */ + @Test + public void testWrongParametersNumberInSelectList() { + assertUnexpectedNumberOfParameters("SELECT 1", 1); + assertUnexpectedNumberOfParameters("SELECT ?", 1, 2); + assertUnexpectedNumberOfParameters("SELECT COALESCE(?)"); + assertUnexpectedNumberOfParameters("SELECT * FROM (VALUES(1, 2, ?)) t1"); + } + + /** */ + @Test + public void testDynamicParametersInExplain() { + sql("EXPLAIN PLAN FOR SELECT * FROM t1 WHERE id > ?", 1); + } + + /** */ + @Test + public void testWrongParametersNumberInDelete() { + assertUnexpectedNumberOfParameters("DELETE FROM t1 WHERE id = 1 AND val1=1", 1); + assertUnexpectedNumberOfParameters("DELETE FROM t1 WHERE id = ? AND val1=1", 1, 2); + assertUnexpectedNumberOfParameters("DELETE FROM t1 WHERE id = ? AND val1=1"); + } + + /** */ + @Test + public void testWrongParametersNumberInInsert() { + assertUnexpectedNumberOfParameters("INSERT INTO t1 VALUES(1, 2, 3)", 1); + assertUnexpectedNumberOfParameters("INSERT INTO t1 VALUES(1, 2, ?)", 1, 2); + assertUnexpectedNumberOfParameters("INSERT INTO t1 VALUES(1, 2, ?)"); + } + + /** */ + @Test + public void testWrongParametersNumberInExplain() { + assertUnexpectedNumberOfParameters("EXPLAIN PLAN FOR SELECT * FROM t1 WHERE id > ?"); + assertUnexpectedNumberOfParameters("EXPLAIN PLAN FOR SELECT * FROM t1 WHERE id > ?", 1, 2); + } + + /** */ + @Test + public void testWrongParametersNumberInLimitOffset() { + assertUnexpectedNumberOfParameters("SELECT * FROM t1 LIMIT 1", 1); + assertUnexpectedNumberOfParameters("SELECT * FROM t1 LIMIT ?", 1, 2); + assertUnexpectedNumberOfParameters("SELECT * FROM t1 LIMIT ?"); + + assertUnexpectedNumberOfParameters("SELECT * FROM t1 LIMIT 1 OFFSET 1", 1); + assertUnexpectedNumberOfParameters("SELECT * FROM t1 LIMIT ? OFFSET ?", 1, 2, 3); + assertUnexpectedNumberOfParameters("SELECT * FROM t1 LIMIT ? OFFSET ?", 1); + assertUnexpectedNumberOfParameters("SELECT * FROM t1 LIMIT 1 OFFSET ?", 1, 2); + assertUnexpectedNumberOfParameters("SELECT * FROM t1 LIMIT 1 OFFSET ?"); + + assertUnexpectedNumberOfParameters("SELECT * FROM t1 OFFSET 1", 1); + assertUnexpectedNumberOfParameters("SELECT * FROM t1 OFFSET ?", 1, 2); + assertUnexpectedNumberOfParameters("SELECT * FROM t1 OFFSET ?"); + } + + /** */ + @Test + public void testWrongParametersNumberInUpdate() { + assertUnexpectedNumberOfParameters("UPDATE t1 SET val1=? WHERE id = 1"); + assertUnexpectedNumberOfParameters("UPDATE t1 SET val1=? WHERE id = 1", 1, 2); + assertUnexpectedNumberOfParameters("UPDATE t1 SET val1=10 WHERE id = ?"); + assertUnexpectedNumberOfParameters("UPDATE t1 SET val1=10 WHERE id = ?", 1, 2); + } + + /** */ + private void assertUnexpectedNumberOfParameters(String qry, Object... params) { + assertThrows(qry, CalciteContextException.class, "Unexpected number of query parameters", params); + } } diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/MemoryQuotasIntegrationTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/MemoryQuotasIntegrationTest.java index 99cd26fd9c2fb..b630effdc1bf3 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/MemoryQuotasIntegrationTest.java +++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/MemoryQuotasIntegrationTest.java @@ -252,6 +252,24 @@ public void testNestedLoopJoinNode() { IgniteException.class, "Query quota exceeded"); } + /** */ + @Test + public void test0() { + // Colocated. + sql("CREATE TABLE tbl2 (id INT, b VARBINARY, v INTEGER) WITH TEMPLATE=REPLICATED"); + + sql("CREATE INDEX idx2 ON tbl2(id)"); + sql("CREATE INDEX idx3 ON tbl2(v)"); + + for (int i = 0; i < 100; i++) + sql("INSERT INTO tbl2 VALUES (?, ?, ?)", i, new byte[1000], i % 5); + + assertQuery("SELECT COUNT(DISTINCT v) FROM tbl2") +// .matches(QueryChecker.containsSubPlan("IgniteColocatedSortAggregate")) + .resultSize(1) + .check(); + } + /** */ @Test public void testSortAggregateNode() { diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/hints/ForceIndexHintPlannerTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/hints/ForceIndexHintPlannerTest.java index fa88732a1b611..dc91296a96a05 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/hints/ForceIndexHintPlannerTest.java +++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/hints/ForceIndexHintPlannerTest.java @@ -31,6 +31,9 @@ * Planner test for force index hint. */ public class ForceIndexHintPlannerTest extends AbstractPlannerTest { + + + /** */ private IgniteSchema schema; @@ -40,6 +43,10 @@ public class ForceIndexHintPlannerTest extends AbstractPlannerTest { /** */ private TestTable tbl2; + private static final String TBL1 = "TBL1"; + + private static final String TBL2 = "TBL2"; + /** {@inheritDoc} */ @Override public void setup() { super.setup(); @@ -70,6 +77,11 @@ public class ForceIndexHintPlannerTest extends AbstractPlannerTest { ((GridTestLog4jLogger)log).setLevel(Level.INFO); } + + private void assertNoCertainIndex(String sql, String tblName, String idxName) throws Exception { + assertPlan(sql, schema, nodeOrAnyChild(isIndexScan(tblName, idxName)).negate()); + } + /** */ @Test public void testBasicIndexSelection() throws Exception { diff --git a/modules/calcite/src/test/java/org/apache/ignite/testsuites/IgniteCalciteTestSuite.java b/modules/calcite/src/test/java/org/apache/ignite/testsuites/IgniteCalciteTestSuite.java index ed7ad797597d0..d9d9b93ddd893 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/testsuites/IgniteCalciteTestSuite.java +++ b/modules/calcite/src/test/java/org/apache/ignite/testsuites/IgniteCalciteTestSuite.java @@ -33,8 +33,8 @@ */ @RunWith(Suite.class) @Suite.SuiteClasses({ - PlannerTestSuite.class, - ExecutionTestSuite.class, +// PlannerTestSuite.class, +// ExecutionTestSuite.class, IntegrationTestSuite.class, ClosableIteratorsHolderTest.class, diff --git a/modules/calcite/src/test/java/org/apache/ignite/testsuites/ScriptTestSuite.java b/modules/calcite/src/test/java/org/apache/ignite/testsuites/ScriptTestSuite.java index fbed0087a3aeb..e71e2c76ed119 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/testsuites/ScriptTestSuite.java +++ b/modules/calcite/src/test/java/org/apache/ignite/testsuites/ScriptTestSuite.java @@ -72,6 +72,7 @@ * */ @RunWith(ScriptTestRunner.class) -@ScriptRunnerTestsEnvironment(scriptsRoot = "modules/calcite/src/test/sql", timeout = 180000) +//@ScriptRunnerTestsEnvironment(scriptsRoot = "modules/calcite/src/test/sql", timeout = 180000) +@ScriptRunnerTestsEnvironment(scriptsRoot = "modules/calcite/src/test/sql/aggregate/aggregates/", timeout = 180000) public class ScriptTestSuite { } diff --git a/modules/calcite/src/test/sql/aggregate/aggregates/test_aggr_string.test b/modules/calcite/src/test/sql/aggregate/aggregates/test_aggr_string.test index 29c005bc99f45..99a6c02238d65 100644 --- a/modules/calcite/src/test/sql/aggregate/aggregates/test_aggr_string.test +++ b/modules/calcite/src/test/sql/aggregate/aggregates/test_aggr_string.test @@ -39,25 +39,62 @@ NULL 11.000000 statement ok INSERT INTO test VALUES (11, 'hello'), (12, 'world') +statement ok +INSERT INTO test VALUES (10, 'ahello'), (13, 'world') + +query I +SELECT COUNT(*) FROM test; +---- +7 + +query II +SELECT COUNT(*), COUNT(s) FROM test; +---- +7 6 + +query I +SELECT AVG(a) FROM test; +---- +11 + +query II +SELECT AVG(a), COUNT(DISTINCT(s)) FROM test; +---- +11 3 + +query II +SELECT AVG(a), COUNT(a) FROM test; +---- +11 7 + +query II +SELECT COUNT(a), COUNT(DISTINCT s) FROM test; +---- +7 3 + # scalar distinct query III SELECT COUNT(*), COUNT(s), COUNT(DISTINCT s) FROM test; ---- -5 4 2 +7 6 3 # grouped distinct query IIII SELECT a, COUNT(*), COUNT(s), COUNT(DISTINCT s) FROM test GROUP BY a ORDER BY a; ---- +10 1 1 1 11 3 2 1 12 2 2 1 +13 1 1 1 # now with WHERE clause query IIII SELECT a, COUNT(*), COUNT(s), COUNT(DISTINCT s) FROM test WHERE s IS NOT NULL GROUP BY a ORDER BY a; ---- +10 1 1 1 11 2 2 1 12 2 2 1 +13 1 1 1 # string min/max with long strings statement ok @@ -80,3 +117,4 @@ SELECT MAX(s), MIN(s) FROM test_strings2; aa A + From aa61f13189667d44bea6cc6046564e13f11446b2 Mon Sep 17 00:00:00 2001 From: Vladimir Steshin Date: Sat, 21 Sep 2024 15:52:18 +0300 Subject: [PATCH 2/4] minor --- .../query/calcite/prepare/IgniteSqlValidator.java | 2 +- .../processors/query/calcite/prepare/PlanningContext.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) 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 995a10ac772d0..809bfab2681fa 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 @@ -104,7 +104,7 @@ public class IgniteSqlValidator extends SqlValidatorImpl { /** Passed query arguments. */ private final Object[] parameters; - /** If {@code true}, enables validation of {@link #parameters}'s' number against the query's dynamic parameters. */ + /** If {@code true}, enables validation of {@link #parameters}'s number against the query's dynamic parameters. */ private final boolean validateParamsNum; /** diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/PlanningContext.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/PlanningContext.java index 314a75869e11e..f9e9cc309cdf3 100644 --- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/PlanningContext.java +++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/PlanningContext.java @@ -52,7 +52,7 @@ public final class PlanningContext implements Context { /** * If not {@code null}, notifies to validate passed parameters number against number of the query's dynamic parameters. * Since several queries may share {@link #parameters()} while each query is validated by dedicated validator, - * the validator has to be aware of numbers total queries and current query. + * this validator has to be aware of numbers of total queries and current query. * The pair is: number of current query starting with 0 and total number of queries. */ @Nullable private final IgnitePair validateParamsNumberCfg; @@ -76,11 +76,11 @@ public final class PlanningContext implements Context { * Private constructor, used by a builder. * * @param parentCtx Parent context. - * @param qry The query. + * @param qry The query to process. * @param parameters Parameters to pass to the query dynamic parameters. * @param validateParamsNumberCfg If not {@code null}, notifies to validate passed parameters number against number * of the query's dynamic parameters. The pair is: number of current query starting with 0 - * and total number of queries. + * and total number of queries to process. * @param plannerTimeout Timeout on operation. * * @see #validateParamsNumberCfg From 7a9c8a7e2c238a1727b0c5397357a4d86be7855b Mon Sep 17 00:00:00 2001 From: Vladimir Steshin Date: Sat, 21 Sep 2024 16:27:56 +0300 Subject: [PATCH 3/4] minors --- .../query/calcite/CalciteQueryProcessor.java | 6 +- .../processors/query/calcite/RootQuery.java | 4 +- .../calcite/prepare/IgniteSqlValidator.java | 69 +++++++++---------- .../calcite/prepare/PlanningContext.java | 30 ++++---- .../DynamicParametersIntegrationTest.java | 2 + 5 files changed, 56 insertions(+), 55 deletions(-) diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessor.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessor.java index aa03dcb769af0..a7283a9cb5199 100644 --- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessor.java +++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/CalciteQueryProcessor.java @@ -484,7 +484,7 @@ private List parseAndProcessQuery( BiFunction, QueryPlan, T> action, @Nullable String schemaName, String sql, - boolean validatParamsCnt, + boolean validateParamsCnt, Object... params ) throws IgniteSQLException { SchemaPlus schema = schemaHolder.schema(schemaName); @@ -525,11 +525,11 @@ private List parseAndProcessQuery( plan0 = queryPlanCache().queryPlan( // Use source SQL to avoid redundant parsing next time. new CacheKey(schema.getName(), sql, contextKey(qryCtx), params), - () -> prepareSvc.prepareSingle(sqlNode, qry.planningContext(validatParamsCnt, 0, 1)) + () -> prepareSvc.prepareSingle(sqlNode, qry.planningContext(validateParamsCnt, 0, 1)) ); } else - plan0 = prepareSvc.prepareSingle(sqlNode, qry.planningContext(validatParamsCnt, qryIdx0, qryList.size())); + plan0 = prepareSvc.prepareSingle(sqlNode, qry.planningContext(validateParamsCnt, qryIdx0, qryList.size())); return action.apply(qry, plan0); }, schema.getName(), removeSensitive(sqlNode), qrys, params); diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/RootQuery.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/RootQuery.java index da86292151952..b5654ee671529 100644 --- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/RootQuery.java +++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/RootQuery.java @@ -306,9 +306,9 @@ public PlanningContext planningContext() { * * @param validateParamsCnt If {@code true}, enables validation of {@link #parameters()} number. * @param curQryNum Current query number in the queries sharing {@link #parameters()}. Ignored if - * {@code validatParamsCnt} is {@code false}. + * {@code validateParamsCnt} is {@code false}. * @param totalQueriesCnt Total count of th queries sharing {@link #parameters()}. Ignored if - * {@code validatParamsCnt} is {@code false}. + * {@code validateParamsCnt} is {@code false}. * @return Planning context. * @see PlanningContext#validateParamsNumber() */ 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 809bfab2681fa..00f93f0e144ff 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 @@ -140,8 +140,7 @@ public IgniteSqlValidator(SqlOperatorTable opTab, CalciteCatalogReader catalogRe qryNum = planningCtx.currentQueryNumber(); qryCnt = planningCtx.queriesCnt(); - assert qryNum >= 0 || qryCnt == 0; - assert qryCnt > qryNum || qryCnt == 0; + assert qryNum >= 0 && qryCnt > qryNum : "Wrong query number or total queries number."; } else { qryNum = 0; @@ -565,39 +564,6 @@ private boolean isSystemFieldName(String alias) { return res; } - /** */ - private void checkDynamicParametersNumber(SqlDynamicParam dynamicParam) { - if (dynamicParam.getIndex() >= dynParCnt) - dynParCnt = dynamicParam.getIndex() + 1; - } - - /** */ - private void extractDynamicParameters(SqlNode node) { - if (!validateParamsNum) - return; - - if (node instanceof SqlDynamicParam) - checkDynamicParametersNumber((SqlDynamicParam)node); - if (node instanceof SqlOrderBy) { - SqlOrderBy orderBy = (SqlOrderBy)node; - - if (orderBy.offset instanceof SqlDynamicParam) - checkDynamicParametersNumber((SqlDynamicParam)orderBy.offset); - - if (orderBy.fetch instanceof SqlDynamicParam) - checkDynamicParametersNumber((SqlDynamicParam)orderBy.fetch); - } - else if (node instanceof SqlSelect) { - SqlSelect select = (SqlSelect)node; - - if (select.getOffset() instanceof SqlDynamicParam) - checkDynamicParametersNumber((SqlDynamicParam)select.getOffset()); - - if (select.getFetch() instanceof SqlDynamicParam) - checkDynamicParametersNumber((SqlDynamicParam)select.getFetch()); - } - } - /** {@inheritDoc} */ @Override protected void inferUnknownTypes(RelDataType inferredType, SqlValidatorScope scope, SqlNode node) { extractDynamicParameters(node); @@ -656,6 +622,39 @@ else if (operandTypeChecker instanceof FamilyOperandTypeChecker) { super.inferUnknownTypes(inferredType, scope, node); } + /** */ + private void extractDynamicParameters(SqlNode node) { + if (!validateParamsNum) + return; + + if (node instanceof SqlDynamicParam) + findMaximalDynamicParameterNumber((SqlDynamicParam)node); + if (node instanceof SqlOrderBy) { + SqlOrderBy orderBy = (SqlOrderBy)node; + + if (orderBy.offset instanceof SqlDynamicParam) + findMaximalDynamicParameterNumber((SqlDynamicParam)orderBy.offset); + + if (orderBy.fetch instanceof SqlDynamicParam) + findMaximalDynamicParameterNumber((SqlDynamicParam)orderBy.fetch); + } + else if (node instanceof SqlSelect) { + SqlSelect select = (SqlSelect)node; + + if (select.getOffset() instanceof SqlDynamicParam) + findMaximalDynamicParameterNumber((SqlDynamicParam)select.getOffset()); + + if (select.getFetch() instanceof SqlDynamicParam) + findMaximalDynamicParameterNumber((SqlDynamicParam)select.getFetch()); + } + } + + /** */ + private void findMaximalDynamicParameterNumber(SqlDynamicParam dynamicParam) { + if (dynamicParam.getIndex() >= dynParCnt) + dynParCnt = dynamicParam.getIndex() + 1; + } + /** {@inheritDoc} */ @Override public SqlLiteral resolveLiteral(SqlLiteral literal) { if (literal instanceof SqlNumericLiteral && literal.createSqlType(typeFactory).getSqlTypeName() == SqlTypeName.BIGINT) { diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/PlanningContext.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/PlanningContext.java index f9e9cc309cdf3..10288d614e6b8 100644 --- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/PlanningContext.java +++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/PlanningContext.java @@ -52,10 +52,10 @@ public final class PlanningContext implements Context { /** * If not {@code null}, notifies to validate passed parameters number against number of the query's dynamic parameters. * Since several queries may share {@link #parameters()} while each query is validated by dedicated validator, - * this validator has to be aware of numbers of total queries and current query. + * this validator has to be aware of numbers of total queries and of current query number. * The pair is: number of current query starting with 0 and total number of queries. */ - @Nullable private final IgnitePair validateParamsNumberCfg; + @Nullable private final IgnitePair validateParamsNumCfg; /** */ private final CancelFlag cancelFlag = new CancelFlag(new AtomicBoolean()); @@ -77,24 +77,24 @@ public final class PlanningContext implements Context { * * @param parentCtx Parent context. * @param qry The query to process. - * @param parameters Parameters to pass to the query dynamic parameters. - * @param validateParamsNumberCfg If not {@code null}, notifies to validate passed parameters number against number + * @param parameters Values to pass to the query's dynamic parameters. + * @param validateParamsNumCfg If not {@code null}, notifies to validate passed parameters number against number * of the query's dynamic parameters. The pair is: number of current query starting with 0 - * and total number of queries to process. + * and total number of queries being processed with shared {@link PlanningContext#parameters()}. * @param plannerTimeout Timeout on operation. * - * @see #validateParamsNumberCfg + * @see #validateParamsNumCfg */ private PlanningContext( Context parentCtx, String qry, Object[] parameters, - @Nullable IgnitePair validateParamsNumberCfg, + @Nullable IgnitePair validateParamsNumCfg, long plannerTimeout ) { this.qry = qry; this.parameters = parameters; - this.validateParamsNumberCfg = validateParamsNumberCfg; + this.validateParamsNumCfg = validateParamsNumCfg; this.parentCtx = parentCtx; startTs = U.currentTimeMillis(); @@ -118,25 +118,25 @@ public Object[] parameters() { /** @return {@code True}, if the validation of number of {@link #parameters()} is required. */ public boolean validateParamsNumber() { - return validateParamsNumberCfg != null; + return validateParamsNumCfg != null; } /** - * @return Number of current query being processed with the same {@link #parameters()}. Starts with 0 and is always 0 + * @return Number of current query being processed with shared {@link #parameters()}. Starts with 0 and is always 0 * if {@link #validateParamsNumber()} is not set. * @see #validateParamsNumber() */ public int currentQueryNumber() { - return validateParamsNumberCfg == null ? 0 : validateParamsNumberCfg.get1(); + return validateParamsNumCfg == null ? 0 : validateParamsNumCfg.get1(); } /** - * @return Total number of queries to process with the same {@link #parameters()}. Is always 0 if {@link #validateParamsNumber()} - * is not set. + * @return Total number of queries to process with shared{@link #parameters()}. Is always 0 if {@link #validateParamsNumber()} + * is {@code false}. * @see #validateParamsNumber() */ public int queriesCnt() { - return validateParamsNumberCfg == null ? 0 : validateParamsNumberCfg.get2(); + return validateParamsNumCfg == null ? 0 : validateParamsNumCfg.get2(); } // Helper methods @@ -319,7 +319,7 @@ public Builder validateParametersQueryNumber(int curQryNum) { } /** - * @param totalQueriesCnt Total number of quries being processed with the same {@link #parameters(Object...)}. + * @param totalQueriesCnt Total number of quries being processed with shared {@link #parameters(Object...)}. * @return Builder for chaining. * @see PlanningContext#validateParamsNumber() * @see #validateParametersQueryNumber(int) diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java index b2ccc7055960c..139cf582a11d9 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java +++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java @@ -133,7 +133,9 @@ public void testWrongParametersNumberInSelectList() { assertUnexpectedNumberOfParameters("SELECT 1", 1); assertUnexpectedNumberOfParameters("SELECT ?", 1, 2); assertUnexpectedNumberOfParameters("SELECT COALESCE(?)"); + assertUnexpectedNumberOfParameters("SELECT COALESCE(?)", 1, 2); assertUnexpectedNumberOfParameters("SELECT * FROM (VALUES(1, 2, ?)) t1"); + assertUnexpectedNumberOfParameters("SELECT * FROM (VALUES(1, 2, ?)) t1", 1, 2); } /** */ From aea7bbb15b557e229eda31ce0900221491f0cc18 Mon Sep 17 00:00:00 2001 From: Vladimir Steshin Date: Sat, 21 Sep 2024 16:36:57 +0300 Subject: [PATCH 4/4] + joins test --- .../DynamicParametersIntegrationTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java index 139cf582a11d9..99ab1bc776bb0 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java +++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java @@ -190,6 +190,20 @@ public void testWrongParametersNumberInSubqueries() { "LIMIT ? OFFSET ?) LIMIT ?", 1, 2, 3); } + /** */ + @Test + public void testWrongParametersNumberInJoins() { + assertUnexpectedNumberOfParameters("SELECT val1, val3 FROM t1 JOIN t2 on val2=val4", 1); + assertUnexpectedNumberOfParameters("SELECT val1 * ?, val3 FROM t1 JOIN t2 on val2=val4*?"); + assertUnexpectedNumberOfParameters("SELECT val1 * ?, val3 FROM t1 JOIN t2 on val2=val4*?", 1); + assertUnexpectedNumberOfParameters("SELECT val1 * ?, val3 FROM t1 JOIN t2 on val2=val4*?", 1, 2, 3); + + assertUnexpectedNumberOfParameters("SELECT val1 * ?, val3 FROM t1 JOIN t2 on val2=val4*? WHERE val3>? ORDER BY " + + "val1 LIMIT ? OFFSET ?", 1, 2, 3, 4); + assertUnexpectedNumberOfParameters("SELECT val1 * ?, val3 FROM t1 JOIN t2 on val2=val4*? WHERE val3>? ORDER BY " + + "val1 LIMIT ? OFFSET ?", 1, 2, 3, 4, 5, 6); + } + /** */ @Test public void testWrongParametersNumberInUnion() {