You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by zs...@apache.org on 2023/08/22 07:32:29 UTC
[ignite-3] branch main updated: IGNITE-19893: Sql. Add plan checks to BaseIndexDataTypeTest. (#2422)
This is an automated email from the ASF dual-hosted git repository.
zstan 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 fefcadb4d2 IGNITE-19893: Sql. Add plan checks to BaseIndexDataTypeTest. (#2422)
fefcadb4d2 is described below
commit fefcadb4d2fa8cd5ff41327aeb6efd49b1f26250
Author: Max Zhuravkov <sh...@gmail.com>
AuthorDate: Tue Aug 22 10:32:24 2023 +0300
IGNITE-19893: Sql. Add plan checks to BaseIndexDataTypeTest. (#2422)
---
.../engine/datatypes/tests/BaseDataTypeTest.java | 32 ++++---
.../datatypes/tests/BaseIndexDataTypeTest.java | 20 ++++-
.../datatypes/varbinary/ItVarBinaryDmlTest.java | 2 +-
.../varbinary/ItVarBinaryExpressionTest.java | 1 -
.../datatypes/varbinary/ItVarBinaryIndexTest.java | 99 ++++++++++++++++++++++
.../ignite/internal/sql/engine/util/TypeUtils.java | 5 ++
6 files changed, 142 insertions(+), 17 deletions(-)
diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/tests/BaseDataTypeTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/tests/BaseDataTypeTest.java
index 7d303c45bb..1a795c0325 100644
--- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/tests/BaseDataTypeTest.java
+++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/tests/BaseDataTypeTest.java
@@ -56,8 +56,8 @@ import org.junit.jupiter.params.provider.Arguments;
* <ul>
* <li>{@code <type>} - an SQL name of a data type.</li>
* <li>{@code $N} - the {@code N-th} value from sample values (0-based), converted to an SQL expression by
- * {@link DataTypeTestSpec#toValueExpr(Comparable)} call (0-based)</li>
- * <li>{@code $N_lit} - the {@code N-th} value from sample values (0-based) in form of an SQL literal.</li>
+ * {@link DataTypeTestSpec#toLiteral(Comparable)} or by {@link DataTypeTestSpec#toValueExpr(Comparable)} ,
+ * if the type has designated literals or if type does have designated literals.</li>
* </ul>
*
* <p>{@link QueryChecker} is automatically checks columns named {@code test_key}
@@ -145,7 +145,7 @@ public abstract class BaseDataTypeTest<T extends Comparable<T>> extends ClusterP
/**
* Use this method instead of {@link #sql(String, Object...)} because it replaces every {@code <type>} with the name of a data
- * type under test, and {@code $N} with corresponding values, where {@code N} is 1-indexed.
+ * type under test, and {@code $N} with corresponding values, where {@code N} is 0-indexed.
*
* @param query A query.
* @return A {@code QueryChecker}.
@@ -162,12 +162,14 @@ public abstract class BaseDataTypeTest<T extends Comparable<T>> extends ClusterP
* Creates a query template.
* <ul>
* <li>{@code <type>} are replaced with a value of type name of {@link DataTypeTestSpec}.</li>
- * <li>{@code $N_lit} are replaced with corresponding literals, where {@code N} is 1-indexed</li>
- * <li>{@code $N} are replaced with corresponding values, where {@code N} is 1-indexed</li>
+ * <li>{@code $N} are replaced with corresponding values provided by
+ * {@link DataTypeTestSpec#toValueExpr(Comparable)} or {@link DataTypeTestSpec#toLiteral(Comparable)}
+ * depending on whether type supports literals or not}. {@code N} is 0-indexed.</li>
* </ul>
*/
- protected QueryTemplate createQueryTemplate(String query) {
- QueryTemplate parameterProvidingTemplate = new ParameterReplacingTemplate<>(testTypeSpec, query, values);
+ private QueryTemplate createQueryTemplate(String query) {
+ boolean useLiterals = testTypeSpec.hasLiterals();
+ QueryTemplate parameterProvidingTemplate = new ParameterReplacingTemplate<>(testTypeSpec, query, values, useLiterals);
return createQueryTemplate(parameterProvidingTemplate, testTypeSpec.typeName());
}
@@ -194,10 +196,13 @@ public abstract class BaseDataTypeTest<T extends Comparable<T>> extends ClusterP
private final List<T> values;
- ParameterReplacingTemplate(DataTypeTestSpec<T> spec, String query, List<T> values) {
+ private final boolean useLiterals;
+
+ ParameterReplacingTemplate(DataTypeTestSpec<T> spec, String query, List<T> values, boolean useLiterals) {
this.testTypeSpec = spec;
this.query = query;
this.values = values;
+ this.useLiterals = useLiterals;
}
@Override
@@ -211,13 +216,12 @@ public abstract class BaseDataTypeTest<T extends Comparable<T>> extends ClusterP
for (var i = 0; i < values.size(); i++) {
T value = values.get(i);
-
- if (testTypeSpec.hasLiterals()) {
- String literalValue = testTypeSpec.toLiteral(value);
- q = q.replace("$" + i + "_lit", literalValue);
+ String placeHolderValue;
+ if (useLiterals) {
+ placeHolderValue = testTypeSpec.toLiteral(value);
+ } else {
+ placeHolderValue = testTypeSpec.toValueExpr(value);
}
-
- String placeHolderValue = testTypeSpec.toValueExpr(value);
q = q.replace("$" + i, placeHolderValue);
}
diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/tests/BaseIndexDataTypeTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/tests/BaseIndexDataTypeTest.java
index 9bd3d26d62..7880a2a217 100644
--- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/tests/BaseIndexDataTypeTest.java
+++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/tests/BaseIndexDataTypeTest.java
@@ -17,6 +17,8 @@
package org.apache.ignite.internal.sql.engine.datatypes.tests;
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsIndexScan;
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsTableScan;
import static org.apache.ignite.lang.IgniteStringFormatter.format;
import java.util.stream.Stream;
@@ -54,10 +56,12 @@ public abstract class BaseIndexDataTypeTest<T extends Comparable<T>> extends Bas
T value1 = values.get(0);
checkQuery("SELECT * FROM t WHERE test_key = $0")
+ .matches(containsIndexScan("PUBLIC", "T", "T_TEST_KEY_IDX"))
.returns(1, value1)
.check();
checkQuery("SELECT * FROM t WHERE test_key iS NOT DISTINCT FROM $0")
+ .matches(containsIndexScan("PUBLIC", "T", "T_TEST_KEY_IDX"))
.returns(1, value1)
.check();
}
@@ -71,11 +75,13 @@ public abstract class BaseIndexDataTypeTest<T extends Comparable<T>> extends Bas
T value3 = values.get(2);
checkQuery("SELECT * FROM t WHERE test_key != $0 ORDER BY id")
+ .matches(containsTableScan("PUBLIC", "T"))
.returns(2, value2)
.returns(3, value3)
.check();
checkQuery("SELECT * FROM t WHERE test_key IS DISTINCT FROM $0")
+ .matches(containsTableScan("PUBLIC", "T"))
.returns(2, value2)
.returns(3, value3)
.check();
@@ -91,21 +97,25 @@ public abstract class BaseIndexDataTypeTest<T extends Comparable<T>> extends Bas
T value3 = values.get(2);
checkQuery("SELECT * FROM t WHERE test_key > $0 ORDER BY id")
+ .matches(containsIndexScan("PUBLIC", "T", "T_TEST_KEY_IDX"))
.returns(2, value2)
.returns(3, value3)
.check();
checkQuery("SELECT * FROM t WHERE test_key >= $1 ORDER BY id")
+ .matches(containsIndexScan("PUBLIC", "T", "T_TEST_KEY_IDX"))
.returns(2, value2)
.returns(3, value3)
.check();
checkQuery("SELECT * FROM t WHERE test_key < $2 ORDER BY id")
+ .matches(containsIndexScan("PUBLIC", "T", "T_TEST_KEY_IDX"))
.returns(1, value1)
.returns(2, value2)
.check();
checkQuery("SELECT * FROM t WHERE test_key <= $2 ORDER BY id")
+ .matches(containsIndexScan("PUBLIC", "T", "T_TEST_KEY_IDX"))
.returns(1, value1)
.returns(2, value2)
.returns(3, value3)
@@ -122,10 +132,12 @@ public abstract class BaseIndexDataTypeTest<T extends Comparable<T>> extends Bas
T value3 = values.get(2);
checkQuery("SELECT * FROM t WHERE test_key > $0 AND test_key < $2 ORDER BY id")
+ .matches(containsIndexScan("PUBLIC", "T", "T_TEST_KEY_IDX"))
.returns(2, value2)
.check();
checkQuery("SELECT * FROM t WHERE test_key >= $1 AND test_key <= $2 ORDER BY id")
+ .matches(containsIndexScan("PUBLIC", "T", "T_TEST_KEY_IDX"))
.returns(2, value2)
.returns(3, value3)
.check();
@@ -145,6 +157,7 @@ public abstract class BaseIndexDataTypeTest<T extends Comparable<T>> extends Bas
String query = format("SELECT * FROM t WHERE test_key BETWEEN {} AND {} ORDER BY id", lit0, lit1);
checkQuery(query)
+ .matches(containsIndexScan("PUBLIC", "T", "T_TEST_KEY_IDX"))
.returns(1, value0)
.returns(2, value1)
.check();
@@ -156,6 +169,7 @@ public abstract class BaseIndexDataTypeTest<T extends Comparable<T>> extends Bas
@Test
public void testOutOfRangeLookUp() {
checkQuery("SELECT * FROM t WHERE test_key < $0")
+ .matches(containsIndexScan("PUBLIC", "T", "T_TEST_KEY_IDX"))
.returnNothing()
.check();
}
@@ -169,6 +183,7 @@ public abstract class BaseIndexDataTypeTest<T extends Comparable<T>> extends Bas
T value3 = values.get(2);
checkQuery("SELECT * FROM t WHERE test_key IN ($0, $2) ORDER BY id")
+ .matches(containsIndexScan("PUBLIC", "T", "T_TEST_KEY_IDX"))
.returns(1, value1)
.returns(3, value3)
.check();
@@ -182,6 +197,7 @@ public abstract class BaseIndexDataTypeTest<T extends Comparable<T>> extends Bas
T value2 = values.get(1);
checkQuery("SELECT * FROM t WHERE test_key NOT IN ($0, $2) ORDER BY id")
+ .matches(containsTableScan("PUBLIC", "T"))
.returns(2, value2)
.check();
}
@@ -199,6 +215,7 @@ public abstract class BaseIndexDataTypeTest<T extends Comparable<T>> extends Bas
String query = format("select id, test_key from t where test_key = {} and id = 100", arguments.valueExpr(0));
checkQuery(query)
+ .matches(containsIndexScan("PUBLIC", "T", "T_TEST_KEY_PK_IDX"))
.returns(100, arguments.value(0))
.check();
}
@@ -209,11 +226,12 @@ public abstract class BaseIndexDataTypeTest<T extends Comparable<T>> extends Bas
@Test
public void testIndexDynParam() {
assertQuery("SELECT * FROM t WHERE test_key=?")
+ .matches(containsIndexScan("PUBLIC", "T", "T_TEST_KEY_IDX"))
.withParams(values.get(0))
.check();
}
- private Stream<TestTypeArguments<T>> compoundIndex() {
+ public Stream<TestTypeArguments<T>> compoundIndex() {
return TestTypeArguments.unary(testTypeSpec, dataSamples, values.get(0));
}
}
diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/varbinary/ItVarBinaryDmlTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/varbinary/ItVarBinaryDmlTest.java
index ac28967ee9..e8d58cd117 100644
--- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/varbinary/ItVarBinaryDmlTest.java
+++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/varbinary/ItVarBinaryDmlTest.java
@@ -32,7 +32,7 @@ public class ItVarBinaryDmlTest extends BaseDmlDataTypeTest<VarBinary> {
/** {@code INSERT} into a column with DEFAULT constraint. */
@Test
public void testDefault() {
- runSql("CREATE TABLE t_def (id INT PRIMARY KEY, test_key VARBINARY DEFAULT $0_lit)");
+ runSql("CREATE TABLE t_def (id INT PRIMARY KEY, test_key VARBINARY DEFAULT $0)");
runSql("INSERT INTO t_def (id) VALUES (0)");
VarBinary value = values.get(0);
diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/varbinary/ItVarBinaryExpressionTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/varbinary/ItVarBinaryExpressionTest.java
index 90218d97c3..2e91db6bb3 100644
--- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/varbinary/ItVarBinaryExpressionTest.java
+++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/varbinary/ItVarBinaryExpressionTest.java
@@ -145,7 +145,6 @@ public class ItVarBinaryExpressionTest extends BaseExpressionDataTypeTest<VarBin
/**
* {@code CAST} to {@code VARBINARY} with different length with dynamic parameters.
*/
- @Disabled("https://issues.apache.org/jira/browse/IGNITE-19353")
@Test
public void testCastToDifferentLengthsWithDynamicParameters() {
byte[] param = {1, 2, 3};
diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/varbinary/ItVarBinaryIndexTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/varbinary/ItVarBinaryIndexTest.java
index 77bdc58703..d29f0a49a5 100644
--- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/varbinary/ItVarBinaryIndexTest.java
+++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/datatypes/varbinary/ItVarBinaryIndexTest.java
@@ -18,20 +18,119 @@
package org.apache.ignite.internal.sql.engine.datatypes.varbinary;
+import static org.apache.ignite.internal.sql.engine.util.QueryChecker.containsIndexScan;
+import static org.apache.ignite.lang.IgniteStringFormatter.format;
+
+import java.util.stream.Stream;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.ignite.internal.sql.engine.datatypes.DataTypeTestSpecs;
import org.apache.ignite.internal.sql.engine.datatypes.tests.BaseIndexDataTypeTest;
import org.apache.ignite.internal.sql.engine.datatypes.tests.DataTypeTestSpec;
+import org.apache.ignite.internal.sql.engine.datatypes.tests.TestTypeArguments;
import org.apache.ignite.internal.sql.engine.util.VarBinary;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Named;
+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 queries that use indexes with {@link SqlTypeName#VARBINARY} type.
*/
public class ItVarBinaryIndexTest extends BaseIndexDataTypeTest<VarBinary> {
+ @BeforeAll
+ public void createTable() {
+ runSql("CREATE TABLE binary_fixed_length(id INTEGER PRIMARY KEY, test_key BINARY(10))");
+ runSql("CREATE INDEX binary_fixed_length_test_key_idx on binary_fixed_length (test_key)");
+
+ runSql("INSERT INTO binary_fixed_length VALUES(1, $0)");
+ runSql("INSERT INTO binary_fixed_length VALUES(2, $1)");
+ runSql("INSERT INTO binary_fixed_length VALUES(3, $2)");
+ }
+
+ /**
+ * Key lookup. Literal, cast, cast with precision against different column types.
+ */
+ @ParameterizedTest
+ @MethodSource("indexChecks")
+ public void testKeyLookUp2(String table, ValueMode mode) {
+ VarBinary value1 = values.get(0);
+ String value1str = mode.toSql(testTypeSpec, value1);
+
+ // TODO Disable for VARBINARY, remove after https://issues.apache.org/jira/browse/IGNITE-19931 is fixed
+ Assumptions.assumeFalse(mode == ValueMode.CAST);
+
+ checkQuery(format("SELECT * FROM {} WHERE test_key = {}", table, value1str))
+ .matches(containsIndexScan("PUBLIC", table, table + "_TEST_KEY_IDX"))
+ .returns(1, value1)
+ .check();
+ }
+
+ /** {@inheritDoc} */
+ @ParameterizedTest
+ @MethodSource("compoundIndex")
+ @Override
+ public void testCompoundIndex(TestTypeArguments<VarBinary> arguments) throws InterruptedException {
+ // TODO Disable for VARBINARY, remove after https://issues.apache.org/jira/browse/IGNITE-19931 is fixed.
+ // Lookups for VARBINARY and VARCHAR/CHAR work
+ Assumptions.assumeFalse(arguments.toString().startsWith("VARBINARY"));
+
+ super.testCompoundIndex(arguments);
+ }
+
+ /** {@inheritDoc} */
+ @Test
+ @Override
+ @Disabled("https://issues.apache.org/jira/browse/IGNITE-19931")
+ public void testIndexDynParam() {
+ super.testIndexDynParam();
+ }
+
/** {@inheritDoc} */
@Override
protected DataTypeTestSpec<VarBinary> getTypeSpec() {
return DataTypeTestSpecs.VARBINARY_TYPE;
}
+
+ private static Stream<Arguments> indexChecks() {
+ return Stream.of(
+ Arguments.of(Named.of("VARBINARY_DEFAULT_LENGTH", "T"), ValueMode.LITERAL),
+ Arguments.of(Named.of("VARBINARY_DEFAULT_LENGTH", "T"), ValueMode.CAST_WITH_PRECISION),
+
+ Arguments.of("BINARY_FIXED_LENGTH", ValueMode.LITERAL),
+ Arguments.of("BINARY_FIXED_LENGTH", ValueMode.CAST),
+ Arguments.of("BINARY_FIXED_LENGTH", ValueMode.CAST_WITH_PRECISION)
+ );
+ }
+
+ /**
+ * Value generation mode.
+ */
+ public enum ValueMode {
+ LITERAL,
+ CAST,
+ CAST_WITH_PRECISION;
+
+ String toSql(DataTypeTestSpec<VarBinary> spec, VarBinary value) {
+ switch (this) {
+ case LITERAL:
+ return spec.toLiteral(value);
+ case CAST: {
+ String str = IgniteUtils.toHexString(value.get());
+ return format("x'{}'::VARBINARY", str);
+ }
+ case CAST_WITH_PRECISION: {
+ String str = IgniteUtils.toHexString(value.get());
+ return format("x'{}'::VARBINARY(8)", str);
+ }
+ default:
+ throw new IllegalArgumentException("Unexpected mode");
+ }
+ }
+ }
}
diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/TypeUtils.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/TypeUtils.java
index ffb6fb0473..c8e8864df9 100644
--- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/TypeUtils.java
+++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/TypeUtils.java
@@ -539,6 +539,11 @@ public class TypeUtils {
/** Checks whether cast operation is necessary in {@code SearchBound}. */
public static boolean needCastInSearchBounds(IgniteTypeFactory typeFactory, RelDataType fromType, RelDataType toType) {
+ // Checks for character and binary types should allow comparison
+ // between types with precision, types w/o precision, and varying non-varying length variants.
+ // Otherwise the optimizer wouldn't pick an index for conditions such as
+ // col (VARCHAR(M)) = CAST(s AS VARCHAR(N) (M != N) , col (VARCHAR) = CAST(s AS VARCHAR(N))
+
// No need to cast between char and varchar.
if (SqlTypeUtil.isCharacter(toType) && SqlTypeUtil.isCharacter(fromType)) {
return false;