IGNITE-21936 Sql. Cover SQL E161(SQL comments using leading double minus) feature by tests (#3716)
diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/AbstractDdlParserTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/AbstractParserTest.java
similarity index 89%
rename from modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/AbstractDdlParserTest.java
rename to modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/AbstractParserTest.java
index 49af15c..8966b1e 100644
--- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/AbstractDdlParserTest.java
+++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/AbstractParserTest.java
@@ -27,9 +27,9 @@
import org.hamcrest.Matcher;
/**
- * Common methods to verify parsing of the DDL command.
+ * Common methods to verify parsing of SQL statements.
*/
-public abstract class AbstractDdlParserTest {
+public abstract class AbstractParserTest {
/**
* Parses a given statement and returns a resulting AST.
*
@@ -68,16 +68,17 @@
* Also compares the expected string on a cloned node.
*/
protected static void expectUnparsed(SqlNode node, String expectedStmt) {
- SqlPrettyWriter w = new SqlPrettyWriter();
- node.unparse(w, 0, 0);
-
- assertEquals(expectedStmt, w.toString(), "Unparsed does not match");
-
- w.reset();
+ assertEquals(expectedStmt, unparse(node), "Unparsed does not match");
// Verify that clone works correctly.
SqlNode cloned = node.clone(node.getParserPosition());
- cloned.unparse(w, 0, 0);
- assertEquals(expectedStmt, w.toString(), "Unparsed does not match for cloned node");
+ assertEquals(expectedStmt, unparse(cloned), "Unparsed does not match for cloned node");
+ }
+
+ static String unparse(SqlNode node) {
+ SqlPrettyWriter w = new SqlPrettyWriter();
+ node.unparse(w, 0, 0);
+
+ return w.toString();
}
}
diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/CommentParsingTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/CommentParsingTest.java
new file mode 100644
index 0000000..4714b77
--- /dev/null
+++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/CommentParsingTest.java
@@ -0,0 +1,204 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.sql.engine.sql;
+
+import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.Random;
+import java.util.concurrent.ThreadLocalRandom;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.ignite.internal.lang.IgniteStringBuilder;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+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.EnumSource;
+
+/**
+ * Tests to verify parsing of comments.
+ *
+ * <p>Covers:<ol>
+ * <li>E161: SQL comments using leading double minus</li>
+ * </ol>
+ *
+ * <p>According to SQL standard, SQL text containing one or more instances of comment is equivalent to the same SQL text with the comment
+ * replaced with newline.
+ */
+@SuppressWarnings("ThrowableNotThrown")
+public class CommentParsingTest extends AbstractParserTest {
+ private static final IgniteLogger LOG = Loggers.forClass(CommentParsingTest.class);
+
+ private static final String NL = System.lineSeparator();
+
+ @ParameterizedTest
+ @EnumSource(Statement.class)
+ void leadingSimpleComment(Statement statement) {
+ String originalQueryString = statement.text;
+ String queryWithComment = "-- this is comment " + NL + originalQueryString;
+
+ assertQueries(
+ originalQueryString,
+ queryWithComment
+ );
+ }
+
+ @ParameterizedTest
+ @EnumSource(Statement.class)
+ void trailingSimpleComment(Statement statement) {
+ String originalQueryString = statement.text;
+ String queryWithComment = originalQueryString + NL + "-- this is comment";
+
+ assertQueries(
+ originalQueryString,
+ queryWithComment
+ );
+ }
+
+ @Test
+ void emptyStatementSimpleComment() {
+ assertThrowsSqlException(
+ Sql.STMT_PARSE_ERR,
+ "Failed to parse query",
+ () -> parse("-- this is comment")
+ );
+ }
+
+ /**
+ * This test injects simple comment before random line break.
+ */
+ @ParameterizedTest
+ @EnumSource(Statement.class)
+ void infixSimpleComment(Statement statement) {
+ int iterations = 50;
+ long seed = ThreadLocalRandom.current().nextLong();
+
+ LOG.info("Seed is {}", seed);
+
+ Random rnd = new Random(seed);
+
+ // it's well-known query that has less than
+ // Integer.MAX_VALUE lines
+ @SuppressWarnings("NumericCastThatLosesPrecision")
+ int linesCount = (int) statement.text.lines().count();
+
+ for (int i = 0; i < iterations; i++) {
+ int lineToInject = Integer.min(statement.maxLineBreak, rnd.nextInt(linesCount));
+
+ int lineNum = 0;
+ IgniteStringBuilder sb = new IgniteStringBuilder();
+ for (String line : statement.text.split(NL)) {
+ sb.app(line);
+
+ if (lineNum++ == lineToInject) {
+ sb.app(" -- this is simple comment");
+ }
+
+ sb.app(NL);
+ }
+
+ assertQueries(statement.text, sb.toString());
+ }
+ }
+
+ private void assertQueries(String expected, String actual) {
+ SqlNode expectedAst;
+ SqlNode actualAst;
+ try {
+ expectedAst = parse(expected);
+ } catch (RuntimeException ex) {
+ LOG.error("Unable to parse statement: \n{}\n", ex, expected);
+
+ throw ex;
+ }
+
+ try {
+ actualAst = parse(actual);
+ } catch (RuntimeException ex) {
+ LOG.error("Unable to parse statement: \n{}\n", ex, actual);
+
+ throw ex;
+ }
+
+ assertEquals(unparse(expectedAst), unparse(actualAst));
+ }
+
+ private enum Statement {
+ QUERY(Q15_STRING),
+
+ DML_INSERT("INSERT", "INTO", "t", Q15_STRING),
+
+ DML_UPDATE("UPDATE", "t", "SET", "a=1", "WHERE", "EXISTS(" + Q15_STRING + ")"),
+
+ DML_DELETE("DELETE", "FROM", "t", "WHERE", "EXISTS(" + Q15_STRING + ")"),
+
+ DDL("CREATE", "TABLE", "t", "(", "id", "INT", "PRIMARY", "KEY", ",", "val VARCHAR", "NOT", "NULL",
+ ",", "PRIMARY", "KEY (", "id", ")", ")", "WITH", "PRIMARY_ZONE", "=", "mZone"),
+
+ EXPLAIN("EXPLAIN", "PLAN", "FOR", Q15_STRING),
+
+ TX("START", "TRANSACTION")
+ ;
+
+ private final int maxLineBreak;
+ private final String text;
+
+ Statement(String... queryTokes) {
+ this.text = String.join(NL, queryTokes);
+ this.maxLineBreak = queryTokes.length;
+ }
+ }
+
+ // This is q15 from TPC-H. Didn't use TpchHelper here on purpose to make sure test
+ // won't be affected by accident change in query string. The test is sensible even to
+ // particular formatting
+ @SuppressWarnings("ConcatenationWithEmptyString")
+ private static final String Q15_STRING = ""
+ + "WITH revenue (supplier_no, total_revenue) as (" + NL
+ + " SELECT" + NL
+ + " l_suppkey," + NL
+ + " sum(l_extendedprice * (1-l_discount))" + NL
+ + " FROM" + NL
+ + " lineitem" + NL
+ + " WHERE" + NL
+ + " l_shipdate >= DATE '1996-01-01'" + NL
+ + " AND l_shipdate < DATE '1996-01-01' + INTERVAL '3' MONTH" + NL
+ + " GROUP BY" + NL
+ + " l_suppkey" + NL
+ + ")" + NL
+ + "SELECT" + NL
+ + " s_suppkey," + NL
+ + " s_name," + NL
+ + " s_address," + NL
+ + " s_phone," + NL
+ + " total_revenue" + NL
+ + "FROM" + NL
+ + " supplier," + NL
+ + " revenue" + NL
+ + "WHERE" + NL
+ + " s_suppkey = supplier_no" + NL
+ + " AND total_revenue = (" + NL
+ + " SELECT" + NL
+ + " max(total_revenue)" + NL
+ + " FROM" + NL
+ + " revenue" + NL
+ + ")" + NL
+ + "ORDER BY" + NL
+ + " s_suppkey";
+}
diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/DistributionZoneSqlDdlParserTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/DistributionZoneSqlDdlParserTest.java
index 78041ac..12f8a9f 100644
--- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/DistributionZoneSqlDdlParserTest.java
+++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/DistributionZoneSqlDdlParserTest.java
@@ -40,7 +40,7 @@
/**
* Test suite to verify parsing of the DDL "ZONE" commands.
*/
-public class DistributionZoneSqlDdlParserTest extends AbstractDdlParserTest {
+public class DistributionZoneSqlDdlParserTest extends AbstractParserTest {
/**
* Parse simple CREATE ZONE statement.
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/IgniteSqlParserSelfTest.java
similarity index 99%
rename from modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlParserTest.java
rename to modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlParserSelfTest.java
index 77da3e0..a4f6fde 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/IgniteSqlParserSelfTest.java
@@ -34,7 +34,7 @@
/**
* Tests for sql parser.
*/
-public class IgniteSqlParserTest {
+public class IgniteSqlParserSelfTest {
@Test
public void testStatementMode() {
StatementParseResult result = IgniteSqlParser.parse("SELECT 1 + ?", StatementParseResult.MODE);
diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlAlterColumnDdlParserTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlAlterColumnDdlParserTest.java
index 10fad94..a00ad65 100644
--- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlAlterColumnDdlParserTest.java
+++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlAlterColumnDdlParserTest.java
@@ -34,7 +34,7 @@
/**
* Test suite to verify parsing of the {@code ALTER TABLE ... ALTER COLUMN} DDL commands.
*/
-public class SqlAlterColumnDdlParserTest extends AbstractDdlParserTest {
+public class SqlAlterColumnDdlParserTest extends AbstractParserTest {
/**
* Verifies parsing of {@code ALTER TABLE ... ALTER COLUMN ... SET/DROP NOT NULL} statement.
diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlDdlParserTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlDdlParserTest.java
index 7cbab78..c2319d9 100644
--- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlDdlParserTest.java
+++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlDdlParserTest.java
@@ -48,7 +48,7 @@
/**
* Test suite to verify parsing of the DDL command.
*/
-public class SqlDdlParserTest extends AbstractDdlParserTest {
+public class SqlDdlParserTest extends AbstractParserTest {
/**
* Very simple case where only table name and a few columns are presented.
*/
diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlReservedWordsTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlReservedWordsTest.java
index 9a9de7d7..11c022a 100644
--- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlReservedWordsTest.java
+++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlReservedWordsTest.java
@@ -29,7 +29,7 @@
/**
* Test reserved keywords.
*/
-public class SqlReservedWordsTest extends AbstractDdlParserTest {
+public class SqlReservedWordsTest extends AbstractParserTest {
/** List of keywords reserved in Ignite SQL. */
private static final Set<String> RESERVED_WORDS = Set.of(
"ABS",
diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlTransactionControlParserTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlTransactionControlParserTest.java
index 42993f3..b166b8c 100644
--- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlTransactionControlParserTest.java
+++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/SqlTransactionControlParserTest.java
@@ -30,7 +30,7 @@
/**
* Tests for transaction control SQL statements.
*/
-public class SqlTransactionControlParserTest extends AbstractDdlParserTest {
+public class SqlTransactionControlParserTest extends AbstractParserTest {
@ParameterizedTest
@CsvSource({
