This is an automated email from the ASF dual-hosted git repository.
jooger 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 6f940a66ee6 IGNITE-24452 Sql. Assertion for unsupported Calcite
statements (#5288)
6f940a66ee6 is described below
commit 6f940a66ee6f937d63b4577ac8352eb8d1c79ff6
Author: Max Zhuravkov <[email protected]>
AuthorDate: Fri Feb 28 10:58:38 2025 +0200
IGNITE-24452 Sql. Assertion for unsupported Calcite statements (#5288)
---
.../internal/sql/engine/sql/IgniteSqlParser.java | 23 +++++++++++++-
.../ignite/internal/sql/engine/util/Commons.java | 14 ++++++++-
.../internal/sql/engine/util/IgniteResource.java | 4 +++
.../sql/engine/sql/IgniteSqlParserSelfTest.java | 36 ++++++++++++++++++++++
4 files changed, 75 insertions(+), 2 deletions(-)
diff --git
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlParser.java
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlParser.java
index 2fe97d27c19..7f06d2db7cc 100644
---
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlParser.java
+++
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlParser.java
@@ -48,6 +48,8 @@ import
org.apache.ignite.internal.generated.query.calcite.sql.IgniteSqlParserImp
import org.apache.ignite.internal.generated.query.calcite.sql.ParseException;
import org.apache.ignite.internal.generated.query.calcite.sql.Token;
import org.apache.ignite.internal.generated.query.calcite.sql.TokenMgrError;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.sql.engine.util.IgniteResource;
import org.apache.ignite.internal.util.StringUtils;
import org.apache.ignite.sql.SqlException;
import org.checkerframework.checker.nullness.qual.Nullable;
@@ -118,6 +120,9 @@ public final class IgniteSqlParser {
for (int i = 0; i < list.size(); i++) {
SqlNode original = list.get(i);
SqlNode node = fixNodesIfNecessary(original);
+
+ validateTopLevelNode(node);
+
list.set(i, node);
}
@@ -132,6 +137,22 @@ public final class IgniteSqlParser {
}
}
+ private static void validateTopLevelNode(SqlNode node) {
+ boolean knownType = Commons.getQueryType(node) != null;
+
+ if (!knownType) {
+ String sqlString = node.toString();
+ // Extract first two tokens of a statement for an error message:
+ // DESCRIBE TABLE t -> DESCRIBE TABLE
+ // UPSERT INTO t (a, b) -> UPSERT INTO
+ int index1 = sqlString.indexOf(' ');
+ int index2 = index1 > 0 ? sqlString.indexOf(' ', index1 + 1) :
index1;
+ String sql = sqlString.substring(0, index2);
+
+ throw SqlUtil.newContextException(node.getParserPosition(),
IgniteResource.INSTANCE.unexpectedStatement(sql));
+ }
+ }
+
/**
* Converts the given exception to the {@link SqlException}.
*
@@ -217,7 +238,7 @@ public final class IgniteSqlParser {
);
}
- return new SqlException(STMT_PARSE_ERR, "Failed to parse query: " +
message);
+ return new SqlException(STMT_PARSE_ERR, "Failed to parse query: " +
message, ex);
}
private static final class InternalIgniteSqlParser extends
IgniteSqlParserImpl {
diff --git
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/Commons.java
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/Commons.java
index a316bb92489..ae0802b7160 100644
---
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/Commons.java
+++
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/Commons.java
@@ -43,6 +43,7 @@ import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
+import java.util.EnumSet;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -69,6 +70,7 @@ import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rex.RexBuilder;
import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.SqlInsert;
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.type.SqlTypeCoercionRule;
@@ -136,6 +138,13 @@ public final class Commons {
public static final int IO_BATCH_SIZE = 256;
public static final int IO_BATCH_COUNT = 4;
+ private static final EnumSet<SqlKind> SUPPORTED_DDL = EnumSet.of(
+ SqlKind.CREATE_SCHEMA, SqlKind.DROP_SCHEMA,
+ SqlKind.CREATE_TABLE, SqlKind.ALTER_TABLE, SqlKind.DROP_TABLE,
+ SqlKind.CREATE_INDEX, SqlKind.ALTER_INDEX, SqlKind.DROP_INDEX,
+ SqlKind.OTHER_DDL
+ );
+
/**
* The number of elements to be prefetched from each partition when
scanning the sorted index.
* The higher the value, the fewer calls to the upstream will be, but at
the same time, the bigger
@@ -828,7 +837,7 @@ public final class Commons {
return SqlQueryType.TX_CONTROL;
}
- if (SqlKind.DDL.contains(sqlKind)) {
+ if (SUPPORTED_DDL.contains(sqlKind)) {
return SqlQueryType.DDL;
}
@@ -847,6 +856,9 @@ public final class Commons {
return SqlQueryType.QUERY;
case INSERT:
+ assert sqlNode instanceof SqlInsert;
+ SqlInsert insert = (SqlInsert) sqlNode;
+ return insert.isUpsert() ? null : SqlQueryType.DML;
case DELETE:
case UPDATE:
case MERGE:
diff --git
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/IgniteResource.java
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/IgniteResource.java
index 1dd78ca6c21..fc409b31e36 100644
---
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/IgniteResource.java
+++
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/IgniteResource.java
@@ -23,6 +23,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.runtime.CalciteException;
import org.apache.calcite.runtime.Resources;
import org.apache.calcite.runtime.Resources.BaseMessage;
import org.apache.calcite.runtime.Resources.ExInst;
@@ -104,6 +105,9 @@ public interface IgniteResource {
@BaseMessage("A recursive query is not supported.")
ExInst<SqlValidatorException> recursiveQueryIsNotSupported();
+ @BaseMessage("Unexpected statement: {0} ")
+ ExInst<CalciteException> unexpectedStatement(String type);
+
/** Constructs a signature string to use in error messages. */
static String makeSignature(SqlCallBinding binding, RelDataType...
operandTypes) {
return makeSignature(binding, Arrays.asList(operandTypes));
diff --git
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlParserSelfTest.java
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlParserSelfTest.java
index a4f6fde83f2..6a139814ee5 100644
---
a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlParserSelfTest.java
+++
b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlParserSelfTest.java
@@ -18,11 +18,16 @@
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.containsString;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import java.util.Arrays;
import java.util.List;
+import java.util.stream.Stream;
+import org.apache.calcite.runtime.CalciteContextException;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.parser.SqlParserPos;
import org.apache.ignite.lang.ErrorGroups.Sql;
@@ -161,6 +166,37 @@ public class IgniteSqlParserSelfTest {
assertEquals(statement.clone(pos).toString(), statement.toString());
}
+ @ParameterizedTest
+ @MethodSource("unsupportedStatements")
+ public void testRejectUnsupportedStatements(String stmt, String error) {
+ CalciteContextException err1 =
assertThrows(CalciteContextException.class,
+ () -> IgniteSqlParser.parse(stmt, StatementParseResult.MODE));
+ assertThat(err1.getMessage(), containsString("Unexpected statement: "
+ error));
+
+ assertThrows(CalciteContextException.class, () ->
IgniteSqlParser.parse(stmt, ScriptParseResult.MODE));
+ }
+
+ private static Stream<Arguments> unsupportedStatements() {
+ return Stream.of(
+ Arguments.of("ALTER SYSTEM SET identifier = expression",
"ALTER SYSTEM"),
+ Arguments.of("ALTER SYSTEM RESET identifier", "ALTER SYSTEM"),
+ Arguments.of("ALTER SYSTEM RESET ALL", "ALTER SYSTEM"),
+
+ Arguments.of("ALTER SESSION SET identifier = expression",
"ALTER SESSION"),
+ Arguments.of("ALTER SESSION RESET identifier", "ALTER
SESSION"),
+ Arguments.of("ALTER SESSION RESET ALL", "ALTER SESSION"),
+
+ Arguments.of("DESCRIBE DATABASE db", "DESCRIBE SCHEMA"),
+ Arguments.of("DESCRIBE CATALOG cat", "DESCRIBE SCHEMA"),
+ Arguments.of("DESCRIBE SCHEMA db.cat.s", "DESCRIBE SCHEMA"),
+ Arguments.of("DESCRIBE TABLE db.cat.s.t", "DESCRIBE TABLE"),
+ // TODO https://issues.apache.org/jira/browse/IGNITE-24630
DESCRIBE <statement> is converted to EXPLAIN PLAN FOR
+ // Arguments.of("DESCRIBE STATEMENT SELECT 1", "EXPLAIN PLAN"),
+
+ Arguments.of("UPSERT INTO t (a, b, c) SELECT 1, 2, 3", "UPSERT
INTO")
+ );
+ }
+
@ParameterizedTest
@MethodSource("dmlStatements")
public void testScriptDmlStatementsClone(String stmt) {
