You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by pp...@apache.org on 2023/11/02 10:33:22 UTC
(ignite-3) branch main updated: IGNITE-20764 Sql. Rework script parsing to return correct number of dynamic parameters for each statement (#2774)
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 <xx...@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.
*