You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by ko...@apache.org on 2023/10/26 12:55:20 UTC
[ignite-3] branch main updated: IGNITE-20743 Sql. QueryChecker doesn't check results (#2755)
This is an automated email from the ASF dual-hosted git repository.
korlov 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 d108d7b1d0 IGNITE-20743 Sql. QueryChecker doesn't check results (#2755)
d108d7b1d0 is described below
commit d108d7b1d0cc15716e265fc5402f0e64071c74c8
Author: korlov42 <ko...@gridgain.com>
AuthorDate: Thu Oct 26 15:55:14 2023 +0300
IGNITE-20743 Sql. QueryChecker doesn't check results (#2755)
---
.../sql/engine/QueryTransactionWrapper.java | 2 +-
.../internal/sql/engine/util/QueryCheckerTest.java | 336 +++++++++++++++++++++
.../internal/sql/engine/util/QueryChecker.java | 12 +-
.../sql/engine/util/QueryCheckerFactory.java | 4 +-
.../internal/sql/engine/util/QueryCheckerImpl.java | 2 +
5 files changed, 350 insertions(+), 6 deletions(-)
diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/QueryTransactionWrapper.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/QueryTransactionWrapper.java
index 5d85c8e72f..2729f4efa5 100644
--- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/QueryTransactionWrapper.java
+++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/QueryTransactionWrapper.java
@@ -27,7 +27,7 @@ public class QueryTransactionWrapper {
private final InternalTransaction transaction;
- QueryTransactionWrapper(InternalTransaction transaction, boolean implicit) {
+ public QueryTransactionWrapper(InternalTransaction transaction, boolean implicit) {
this.transaction = transaction;
this.implicit = implicit;
}
diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/util/QueryCheckerTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/util/QueryCheckerTest.java
new file mode 100644
index 0000000000..eee7c3288c
--- /dev/null
+++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/util/QueryCheckerTest.java
@@ -0,0 +1,336 @@
+/*
+ * 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.util;
+
+import static org.apache.ignite.internal.testframework.IgniteTestUtils.assertThrowsWithCause;
+import static org.hamcrest.Matchers.containsString;
+
+import java.util.List;
+import java.util.UUID;
+import java.util.concurrent.CompletableFuture;
+import org.apache.ignite.internal.sql.engine.AsyncSqlCursor;
+import org.apache.ignite.internal.sql.engine.AsyncSqlCursorImpl;
+import org.apache.ignite.internal.sql.engine.QueryContext;
+import org.apache.ignite.internal.sql.engine.QueryProcessor;
+import org.apache.ignite.internal.sql.engine.QueryTransactionWrapper;
+import org.apache.ignite.internal.sql.engine.SqlQueryType;
+import org.apache.ignite.internal.sql.engine.framework.DataProvider;
+import org.apache.ignite.internal.sql.engine.framework.NoOpTransaction;
+import org.apache.ignite.internal.sql.engine.framework.TestBuilders;
+import org.apache.ignite.internal.sql.engine.framework.TestCluster;
+import org.apache.ignite.internal.sql.engine.framework.TestNode;
+import org.apache.ignite.internal.sql.engine.prepare.QueryPlan;
+import org.apache.ignite.internal.sql.engine.property.PropertiesHolder;
+import org.apache.ignite.internal.sql.engine.session.SessionId;
+import org.apache.ignite.internal.sql.engine.session.SessionInfo;
+import org.apache.ignite.internal.sql.engine.trait.IgniteDistributions;
+import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest;
+import org.apache.ignite.internal.type.NativeTypes;
+import org.apache.ignite.internal.util.AsyncCursor;
+import org.apache.ignite.sql.ColumnMetadata;
+import org.apache.ignite.sql.ColumnType;
+import org.apache.ignite.tx.IgniteTransactions;
+import org.apache.ignite.tx.Transaction;
+import org.apache.ignite.tx.TransactionOptions;
+import org.jetbrains.annotations.Nullable;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Tests to verify correctness of {@link QueryChecker} framework.
+ */
+@SuppressWarnings("ThrowableNotThrown")
+@ExtendWith(QueryCheckerExtension.class)
+public class QueryCheckerTest extends BaseIgniteAbstractTest {
+ private static final String NODE_NAME = "N1";
+
+ @InjectQueryCheckerFactory
+ private static QueryCheckerFactory queryCheckerFactory;
+
+ // @formatter:off
+ private static final TestCluster CLUSTER = TestBuilders.cluster()
+ .nodes(NODE_NAME)
+ .addTable()
+ .name("T1")
+ .addKeyColumn("ID", NativeTypes.INT32)
+ .addColumn("VAL", NativeTypes.INT32)
+ .distribution(IgniteDistributions.hash(List.of(0)))
+ .defaultDataProvider(DataProvider.fromCollection(List.of(
+ new Object[] {1, 1}, new Object[] {2, 2}
+ )))
+ .end()
+ .build();
+ // @formatter:on
+
+ @BeforeAll
+ static void startCluster() {
+ CLUSTER.start();
+ }
+
+ @AfterAll
+ static void stopCluster() throws Exception {
+ CLUSTER.stop();
+ }
+
+ @Test
+ void testDisabledRulesAndPlanMatcher() {
+ {
+ String[] disableAllAggregatesButColocatedHash = {
+ "MapReduceHashAggregateConverterRule",
+ "MapReduceSortAggregateConverterRule",
+ "ColocatedSortAggregateConverterRule"
+ };
+
+ assertQuery("SELECT COUNT(*) FROM t1")
+ .disableRules(disableAllAggregatesButColocatedHash)
+ .matches(containsString("ColocatedHashAggregate"))
+ .check();
+
+ assertThrowsWithCause(
+ () -> assertQuery("SELECT COUNT(*) FROM t1")
+ .disableRules(disableAllAggregatesButColocatedHash)
+ .matches(containsString("ColocatedSortAggregate"))
+ .check(),
+ AssertionError.class,
+ "Invalid plan"
+ );
+ }
+
+ {
+ String[] disableAllAggregatesButColocatedSort = {
+ "MapReduceHashAggregateConverterRule",
+ "MapReduceSortAggregateConverterRule",
+ "ColocatedHashAggregateConverterRule"
+ };
+
+ assertQuery("SELECT COUNT(*) FROM t1")
+ .disableRules(disableAllAggregatesButColocatedSort)
+ .matches(containsString("ColocatedSortAggregate"))
+ .check();
+
+ assertThrowsWithCause(
+ () -> assertQuery("SELECT COUNT(*) FROM t1")
+ .disableRules(disableAllAggregatesButColocatedSort)
+ .matches(containsString("ColocatedHashAggregate"))
+ .check(),
+ AssertionError.class,
+ "Invalid plan"
+ );
+ }
+ }
+
+ @Test
+ void testReturns() {
+ assertQuery("SELECT * FROM t1")
+ .returnSomething()
+ .check();
+
+ assertQuery("SELECT * FROM t1")
+ .returns(1, 1)
+ .returns(2, 2)
+ .check();
+
+ // by default comparison is order agnostic
+ assertQuery("SELECT * FROM t1")
+ .returns(2, 2)
+ .returns(1, 1)
+ .check();
+
+ // query returns in different order
+ assertThrowsWithCause(
+ () -> assertQuery("SELECT * FROM t1")
+ .ordered()
+ .returns(2, 2)
+ .returns(1, 1)
+ .check(),
+ AssertionError.class,
+ "Collections are not equal (position 0)"
+ );
+
+ // query returns something
+ assertThrowsWithCause(
+ () -> assertQuery("SELECT * FROM t1")
+ .returnNothing()
+ .check(),
+ AssertionError.class,
+ "Expected: an empty collection"
+ );
+
+ // query returns more than expected
+ assertThrowsWithCause(
+ () -> assertQuery("SELECT * FROM t1")
+ .returns(1, 1)
+ .check(),
+ AssertionError.class,
+ "Collections sizes are not equal"
+ );
+
+ // query returns less than expected
+ assertThrowsWithCause(
+ () -> assertQuery("SELECT * FROM t1")
+ .returns(1, 1)
+ .returns(2, 2)
+ .returns(3, 3)
+ .check(),
+ AssertionError.class,
+ "Collections sizes are not equal"
+ );
+
+ // query returns different values
+ assertThrowsWithCause(
+ () -> assertQuery("SELECT * FROM t1")
+ .ordered()
+ .returns(1, 1)
+ .returns(3, 3)
+ .check(),
+ AssertionError.class,
+ "Collections are not equal (position 0)"
+ );
+
+ // query returns different types
+ assertThrowsWithCause(
+ () -> assertQuery("SELECT * FROM t1")
+ .ordered()
+ .returns(1, 1)
+ .returns(2, 2L)
+ .check(),
+ AssertionError.class,
+ "Collections are not equal (position 1)"
+ );
+ }
+
+ @Test
+ void testMetadata() {
+ assertQuery("SELECT * FROM t1")
+ .columnNames("ID", "VAL")
+ .check();
+
+ assertQuery("SELECT * FROM t1")
+ .columnTypes(Integer.class, Integer.class)
+ .check();
+
+ assertQuery("SELECT id, val::DECIMAL(19, 2) as val_dec, id::VARCHAR(64) as id_str FROM t1")
+ .columnMetadata(
+ new MetadataMatcher()
+ .name("ID")
+ .type(ColumnType.INT32)
+ .precision(10)
+ .scale(0)
+ .nullable(false),
+
+ new MetadataMatcher()
+ .name("VAL_DEC")
+ .type(ColumnType.DECIMAL)
+ .precision(19)
+ .scale(2)
+ .nullable(true),
+
+ new MetadataMatcher()
+ .name("ID_STR")
+ .type(ColumnType.STRING)
+ .precision(64)
+ .scale(ColumnMetadata.UNDEFINED_SCALE)
+ .nullable(false)
+ )
+ .check();
+ }
+
+ private static QueryChecker assertQuery(String qry) {
+ TestNode testNode = CLUSTER.node(NODE_NAME);
+
+ return queryCheckerFactory.create(
+ new TestQueryProcessor(testNode),
+ new TestIgniteTransactions(),
+ null,
+ qry
+ );
+ }
+
+ private static class TestQueryProcessor implements QueryProcessor {
+ private final TestNode node;
+
+ TestQueryProcessor(TestNode node) {
+ this.node = node;
+ }
+
+ @Override
+ public SessionId createSession(PropertiesHolder properties) {
+ return new SessionId(UUID.randomUUID());
+ }
+
+ @Override
+ public CompletableFuture<Void> closeSession(SessionId sessionId) {
+ return CompletableFuture.completedFuture(null);
+ }
+
+ @Override
+ public List<SessionInfo> liveSessions() {
+ return List.of();
+ }
+
+ @Override
+ public CompletableFuture<AsyncSqlCursor<List<Object>>> querySingleAsync(SessionId sessionId, QueryContext context,
+ IgniteTransactions transactions, String qry, Object... params) {
+ assert params == null || params.length == 0 : "params are not supported";
+
+ QueryPlan plan = node.prepare(qry);
+ AsyncCursor<List<Object>> dataCursor = node.executePlan(plan);
+
+ SqlQueryType type = plan.type();
+
+ assert type != null;
+
+ AsyncSqlCursor<List<Object>> sqlCursor = new AsyncSqlCursorImpl<>(
+ type,
+ plan.metadata(),
+ new QueryTransactionWrapper(new NoOpTransaction("test"), false),
+ dataCursor
+ );
+
+ return CompletableFuture.completedFuture(sqlCursor);
+ }
+
+ @Override
+ public void start() {
+ // NO-OP
+ }
+
+ @Override
+ public void stop() {
+ // NO-OP
+ }
+ }
+
+ private static class TestIgniteTransactions implements IgniteTransactions {
+ @Override
+ public Transaction begin(@Nullable TransactionOptions options) {
+ String name = "test";
+
+ return options != null && options.readOnly()
+ ? NoOpTransaction.readOnly(name)
+ : NoOpTransaction.readWrite(name);
+ }
+
+ @Override
+ public CompletableFuture<Transaction> beginAsync(@Nullable TransactionOptions options) {
+ return CompletableFuture.completedFuture(begin(options));
+ }
+ }
+}
diff --git a/modules/sql-engine/src/testFixtures/java/org/apache/ignite/internal/sql/engine/util/QueryChecker.java b/modules/sql-engine/src/testFixtures/java/org/apache/ignite/internal/sql/engine/util/QueryChecker.java
index 5fb1965bbf..a64f9d9cf5 100644
--- a/modules/sql-engine/src/testFixtures/java/org/apache/ignite/internal/sql/engine/util/QueryChecker.java
+++ b/modules/sql-engine/src/testFixtures/java/org/apache/ignite/internal/sql/engine/util/QueryChecker.java
@@ -18,7 +18,6 @@
package org.apache.ignite.internal.sql.engine.util;
import static org.apache.ignite.internal.util.ArrayUtils.nullOrEmpty;
-import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.fail;
import java.lang.reflect.Array;
@@ -26,10 +25,10 @@ import java.lang.reflect.Type;
import java.util.Arrays;
import java.util.Collection;
import java.util.Iterator;
-import java.util.List;
import java.util.Objects;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
+import org.apache.ignite.internal.lang.IgniteStringBuilder;
import org.hamcrest.CoreMatchers;
import org.hamcrest.Matcher;
import org.hamcrest.core.SubstringMatcher;
@@ -37,7 +36,6 @@ import org.hamcrest.core.SubstringMatcher;
/** Query checker interface. */
public interface QueryChecker {
Object[] NULL_AS_VARARG = {null};
- List<List<?>> EMPTY_RES = List.of(List.of());
/** Creates a matcher that matches if the examined string contains the specified string anywhere. */
static Matcher<String> containsUnion(boolean all) {
@@ -186,7 +184,13 @@ public interface QueryChecker {
* @param act Actual collection.
*/
static void assertEqualsCollections(Collection<?> exp, Collection<?> act) {
- assertEquals(exp.size(), act.size(), "Collections sizes are not equal:\nExpected: " + exp + "\nActual: " + act);
+ if (exp.size() != act.size()) {
+ String errorMsg = new IgniteStringBuilder("Collections sizes are not equal:").nl()
+ .app("\tExpected: ").app(exp).nl()
+ .app("\t Actual: ").app(act).toString();
+
+ throw new AssertionError(errorMsg);
+ }
Iterator<?> it1 = exp.iterator();
Iterator<?> it2 = act.iterator();
diff --git a/modules/sql-engine/src/testFixtures/java/org/apache/ignite/internal/sql/engine/util/QueryCheckerFactory.java b/modules/sql-engine/src/testFixtures/java/org/apache/ignite/internal/sql/engine/util/QueryCheckerFactory.java
index a0d56fbe8b..86015e7ac7 100644
--- a/modules/sql-engine/src/testFixtures/java/org/apache/ignite/internal/sql/engine/util/QueryCheckerFactory.java
+++ b/modules/sql-engine/src/testFixtures/java/org/apache/ignite/internal/sql/engine/util/QueryCheckerFactory.java
@@ -23,13 +23,15 @@ import org.apache.ignite.internal.sql.engine.util.QueryChecker.QueryTemplate;
import org.apache.ignite.sql.ResultSetMetadata;
import org.apache.ignite.tx.IgniteTransactions;
import org.apache.ignite.tx.Transaction;
+import org.jetbrains.annotations.Nullable;
/**
* Interface for {@link QueryChecker} factory.
*/
public interface QueryCheckerFactory {
/** Creates query checker instance. */
- QueryChecker create(QueryProcessor queryProcessor, IgniteTransactions transactions, Transaction tx, String query);
+ QueryChecker create(QueryProcessor queryProcessor, IgniteTransactions transactions,
+ @Nullable Transaction tx, String query);
/** Creates query checker with custom metadata validator. */
QueryChecker create(
diff --git a/modules/sql-engine/src/testFixtures/java/org/apache/ignite/internal/sql/engine/util/QueryCheckerImpl.java b/modules/sql-engine/src/testFixtures/java/org/apache/ignite/internal/sql/engine/util/QueryCheckerImpl.java
index 84746901cd..506cfc5be7 100644
--- a/modules/sql-engine/src/testFixtures/java/org/apache/ignite/internal/sql/engine/util/QueryCheckerImpl.java
+++ b/modules/sql-engine/src/testFixtures/java/org/apache/ignite/internal/sql/engine/util/QueryCheckerImpl.java
@@ -158,6 +158,8 @@ abstract class QueryCheckerImpl implements QueryChecker {
if (rowByRowResultChecker == null) {
rowByRowResultChecker = new RowByRowResultChecker();
+
+ resultChecker = rowByRowResultChecker;
}
// let's interpret null array as simple single null.