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 b033be5061 IGNITE-20764 Sql. Rework script parsing to return correct number of dynamic parameters for each statement (#2774) b033be5061 is described below commit b033be5061e495ae8d63c0f7730b8511fd38be96 Author: Pavel Pereslegin <xxt...@gmail.com> AuthorDate: Thu Nov 2 13:33:17 2023 +0300 IGNITE-20764 Sql. Rework script parsing to return correct number of dynamic parameters for each statement (#2774) --- .../internal/sql/engine/sql/ParseResult.java | 6 --- .../internal/sql/engine/sql/ParserService.java | 11 ++++- .../internal/sql/engine/sql/ParserServiceImpl.java | 27 +++++++++++ .../internal/sql/engine/sql/ScriptParseResult.java | 52 ++++++++++++++++++---- .../sql/engine/sql/StatementParseResult.java | 14 ++---- .../sql/engine/sql/IgniteSqlParserTest.java | 41 ++++++++++++++--- .../sql/engine/sql/ParserServiceImplTest.java | 52 ++++++++++++++++++++++ 7 files changed, 171 insertions(+), 32 deletions(-) diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ParseResult.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ParseResult.java index b36a984f96..88f5128d33 100644 --- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ParseResult.java +++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ParseResult.java @@ -17,9 +17,6 @@ package org.apache.ignite.internal.sql.engine.sql; -import java.util.List; -import org.apache.calcite.sql.SqlNode; - /** * Result of parsing SQL string. */ @@ -43,7 +40,4 @@ public abstract class ParseResult { public int dynamicParamsCount() { return dynamicParamsCount; } - - /** Returns a list of parsed statements. */ - public abstract List<SqlNode> statements(); } diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ParserService.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ParserService.java index d358ebf161..a597e80a6d 100644 --- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ParserService.java +++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ParserService.java @@ -17,12 +17,13 @@ package org.apache.ignite.internal.sql.engine.sql; +import java.util.List; + /** * A service whose sole purpose is to take a query string and convert it into a syntax tree according to the rules of grammar. * * @see ParsedResult */ -@SuppressWarnings("InterfaceMayBeAnnotatedFunctional") public interface ParserService { /** * Takes a query string and convert it into a syntax tree according to the rules of grammar. @@ -31,4 +32,12 @@ public interface ParserService { * @return Result of the parsing. */ ParsedResult parse(String query); + + /** + * Takes a query containing multiple statements and converts it into a list of syntax trees according to the rules of the grammar. + * + * @param query A query to convert. + * @return List of parsing results. + */ + List<ParsedResult> parseScript(String query); } diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ParserServiceImpl.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ParserServiceImpl.java index 66d00e0e70..6e298ff2e3 100644 --- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ParserServiceImpl.java +++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ParserServiceImpl.java @@ -17,6 +17,8 @@ package org.apache.ignite.internal.sql.engine.sql; +import java.util.ArrayList; +import java.util.List; import java.util.function.Supplier; import org.apache.calcite.sql.SqlNode; import org.apache.ignite.internal.sql.engine.SqlQueryType; @@ -72,6 +74,31 @@ public class ParserServiceImpl implements ParserService { return result; } + /** {@inheritDoc} */ + @Override + public List<ParsedResult> parseScript(String query) { + ScriptParseResult parsedStatement = IgniteSqlParser.parse(query, ScriptParseResult.MODE); + List<ParsedResult> results = new ArrayList<>(parsedStatement.results().size()); + + for (StatementParseResult result : parsedStatement.results()) { + SqlNode parsedTree = result.statement(); + SqlQueryType queryType = Commons.getQueryType(parsedTree); + String normalizedQuery = parsedTree.toString(); + + assert queryType != null : normalizedQuery; + + results.add(new ParsedResultImpl( + queryType, + normalizedQuery, + normalizedQuery, + result.dynamicParamsCount(), + () -> parsedTree + )); + } + + return results; + } + static class ParsedResultImpl implements ParsedResult { private final SqlQueryType queryType; private final String originalQuery; diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ScriptParseResult.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ScriptParseResult.java index 82c77d24fc..2d59946b14 100644 --- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ScriptParseResult.java +++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/ScriptParseResult.java @@ -18,8 +18,13 @@ package org.apache.ignite.internal.sql.engine.sql; import java.util.List; +import java.util.stream.Collectors; +import javax.annotation.concurrent.NotThreadSafe; +import org.apache.calcite.sql.SqlDynamicParam; import org.apache.calcite.sql.SqlNode; +import org.apache.calcite.sql.util.SqlBasicVisitor; import org.apache.ignite.internal.tostring.S; +import org.jetbrains.annotations.Nullable; /** * Result of parsing SQL string that multiple statements. @@ -32,27 +37,58 @@ public final class ScriptParseResult extends ParseResult { public static final ParseMode<ScriptParseResult> MODE = new ParseMode<>() { @Override ScriptParseResult createResult(List<SqlNode> list, int dynamicParamsCount) { - return new ScriptParseResult(list, dynamicParamsCount); + if (list.size() == 1) { + return new ScriptParseResult(List.of(new StatementParseResult(list.get(0), dynamicParamsCount)), dynamicParamsCount); + } + + SqlDynamicParamsCounter paramsCounter = dynamicParamsCount == 0 ? null : new SqlDynamicParamsCounter(); + List<StatementParseResult> results = list.stream() + .map(node -> new StatementParseResult(node, paramsCounter == null ? 0 : paramsCounter.forNode(node))) + .collect(Collectors.toList()); + + return new ScriptParseResult(results, dynamicParamsCount); } }; - private final List<SqlNode> statements; + private final List<StatementParseResult> results; /** * Constructor. * - * @param statements A list of parsed statements. + * @param results A list of parsing results. * @param dynamicParamsCount The number of dynamic parameters. */ - public ScriptParseResult(List<SqlNode> statements, int dynamicParamsCount) { + private ScriptParseResult(List<StatementParseResult> results, int dynamicParamsCount) { super(dynamicParamsCount); - this.statements = statements; + this.results = results; } /** Returns a list of parsed statements. */ - @Override - public List<SqlNode> statements() { - return statements; + public List<StatementParseResult> results() { + return results; + } + + /** + * Counts the number of {@link SqlDynamicParam} nodes in the tree. + */ + @NotThreadSafe + static class SqlDynamicParamsCounter extends SqlBasicVisitor<Object> { + int count; + + @Override + public @Nullable Object visit(SqlDynamicParam param) { + count++; + + return null; + } + + int forNode(SqlNode node) { + count = 0; + + this.visitNode(node); + + return count; + } } /** {@inheritDoc} **/ diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/StatementParseResult.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/StatementParseResult.java index c2728f9a8d..c91dd970ed 100644 --- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/StatementParseResult.java +++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/StatementParseResult.java @@ -45,7 +45,7 @@ public final class StatementParseResult extends ParseResult { } }; - private final List<SqlNode> list; + private final SqlNode statement; /** * Constructor. @@ -53,21 +53,15 @@ public final class StatementParseResult extends ParseResult { * @param sqlNode A parsed statement. * @param dynamicParamsCount The number of dynamic parameters. */ - public StatementParseResult(SqlNode sqlNode, int dynamicParamsCount) { + StatementParseResult(SqlNode sqlNode, int dynamicParamsCount) { super(dynamicParamsCount); assert !(sqlNode instanceof SqlNodeList) : "Can not create a statement result from a node list: " + sqlNode; - this.list = List.of(sqlNode); + this.statement = sqlNode; } /** Returns a parsed statement. */ public SqlNode statement() { - return list.get(0); - } - - /** {@inheritDoc} **/ - @Override - public List<SqlNode> statements() { - return list; + return statement; } /** {@inheritDoc} **/ diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlParserTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlParserTest.java index 6af20f5495..8bc52a3c6a 100644 --- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlParserTest.java +++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlParserTest.java @@ -21,9 +21,13 @@ import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThro import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import java.util.Arrays; import java.util.List; import org.apache.ignite.lang.ErrorGroups.Sql; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; /** * Tests for sql parser. @@ -35,15 +39,24 @@ public class IgniteSqlParserTest { assertEquals(1, result.dynamicParamsCount()); assertNotNull(result.statement()); - assertEquals(List.of(result.statement()), result.statements()); } - @Test - public void testScriptMode() { - ScriptParseResult result = IgniteSqlParser.parse("SELECT 1 + ?; SELECT 1; SELECT 2 + ?", ScriptParseResult.MODE); + @ParameterizedTest + @MethodSource("multiStatementQueries") + public void testScriptMode(String query, int[] expectedParamsCountPerStatement) { + ScriptParseResult scriptParseResult = IgniteSqlParser.parse(query, ScriptParseResult.MODE); + int expectedTotalParams = Arrays.stream(expectedParamsCountPerStatement).sum(); + int expectedStatementsCount = expectedParamsCountPerStatement.length; + + assertEquals(expectedTotalParams, scriptParseResult.dynamicParamsCount()); + assertEquals(expectedStatementsCount, scriptParseResult.results().size()); + + for (int i = 0; i < scriptParseResult.results().size(); i++) { + StatementParseResult res = scriptParseResult.results().get(i); - assertEquals(2, result.dynamicParamsCount()); - assertEquals(3, result.statements().size()); + assertNotNull(res.statement()); + assertEquals(expectedParamsCountPerStatement[i], res.dynamicParamsCount()); + } } /** @@ -55,7 +68,6 @@ public class IgniteSqlParserTest { Sql.STMT_VALIDATION_ERR, "Multiple statements are not allowed", () -> IgniteSqlParser.parse("SELECT 1; SELECT 2", StatementParseResult.MODE)); - } @Test @@ -120,4 +132,19 @@ public class IgniteSqlParserTest { "Failed to parse query: Invalid decimal literal. At line 1, column 16", () -> IgniteSqlParser.parse("SELECT decimal '2a'", StatementParseResult.MODE)); } + + private static List<Arguments> multiStatementQueries() { + return List.of( + Arguments.of( + "-- insert\n" + + "INSERT INTO TEST VALUES(1, 1);;\n" + + "--- select\n" + + ";;;SELECT 1 + 2", + new int[]{0, 0} + ), + Arguments.of("SELECT 1 + ? - ?", new int[]{2}), + Arguments.of("SELECT 1; INSERT INTO TEST VALUES(?, ?, ?)", new int[] {0, 3}), + Arguments.of("INSERT INTO TEST VALUES(?, ?); SELECT 1; SELECT 2 + ?", new int[] {2, 0, 1}) + ); + } } diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/ParserServiceImplTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/ParserServiceImplTest.java index 8b18cd3490..850aca5b41 100644 --- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/ParserServiceImplTest.java +++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/ParserServiceImplTest.java @@ -17,20 +17,28 @@ package org.apache.ignite.internal.sql.engine.sql; +import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertSame; +import java.util.List; import java.util.function.Function; import org.apache.calcite.sql.SqlNode; +import org.apache.ignite.internal.lang.IgniteStringBuilder; import org.apache.ignite.internal.sql.engine.SqlQueryType; import org.apache.ignite.internal.sql.engine.util.EmptyCacheFactory; import org.apache.ignite.internal.sql.engine.util.cache.Cache; import org.apache.ignite.internal.sql.engine.util.cache.CacheFactory; import org.apache.ignite.internal.sql.engine.util.cache.CaffeineCacheFactory; import org.apache.ignite.internal.sql.engine.util.cache.StatsCounter; +import org.apache.ignite.lang.ErrorGroups.Sql; import org.jetbrains.annotations.Nullable; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.EnumSource; @@ -107,6 +115,50 @@ public class ParserServiceImplTest { assertThat(firstCall.toString(), is(secondCall.toString())); } + /** + * Checks the parsing of a query containing multiple statements. + * + * <p>This parsing mode is only supported using the {@link ParserService#parseScript(String)} method, + * so {@link ParserService#parse(String)}} must fail with a validation error. + * + * <p>Parsing produces a list of parsing results, each of which must match the parsing + * result of the corresponding single statement. + */ + @Test + void parseMultiStatementQuery() { + ParserService service = new ParserServiceImpl(0, EmptyCacheFactory.INSTANCE); + + List<Statement> statements = List.of(Statement.values()); + IgniteStringBuilder buf = new IgniteStringBuilder(); + + for (Statement statement : statements) { + buf.app(statement.text).app(';'); + } + + String multiStatementQuery = buf.toString(); + + //noinspection ThrowableNotThrown + assertThrowsSqlException( + Sql.STMT_VALIDATION_ERR, + "Multiple statements are not allowed", + () -> service.parse(multiStatementQuery) + ); + + List<ParsedResult> results = service.parseScript(multiStatementQuery); + assertThat(results, hasSize(statements.size())); + + for (int i = 0; i < results.size(); i++) { + ParsedResult result = results.get(i); + ParsedResult singleStatementResult = service.parse(statements.get(i).text); + + assertThat(result.queryType(), equalTo(statements.get(i).type)); + assertThat(result.parsedTree(), notNullValue()); + assertThat(result.parsedTree().toString(), equalTo(singleStatementResult.parsedTree().toString())); + assertThat(result.normalizedQuery(), equalTo(singleStatementResult.normalizedQuery())); + assertThat(result.originalQuery(), equalTo(singleStatementResult.normalizedQuery())); + } + } + /** * Parsed result that throws {@link AssertionError} on every method call. *