This is an automated email from the ASF dual-hosted git repository.
alexpl pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push:
new 20160fa66f5 IGNITE-23179 SQL Calcite: Fix wrong numeric type coercion
with 'IS DISTINCT FROM' - Fixes #11525.
20160fa66f5 is described below
commit 20160fa66f517ca270b564690b5373ab428e141a
Author: Vladimir Steshin <[email protected]>
AuthorDate: Tue Oct 1 17:40:34 2024 +0300
IGNITE-23179 SQL Calcite: Fix wrong numeric type coercion with 'IS DISTINCT
FROM' - Fixes #11525.
Signed-off-by: Aleksey Plekhanov <[email protected]>
---
.../query/calcite/exec/LogicalRelImplementor.java | 5 +-
.../query/calcite/exec/exp/ExpressionFactory.java | 4 +-
.../calcite/exec/exp/ExpressionFactoryImpl.java | 5 +-
.../query/calcite/prepare/IgniteTypeCoercion.java | 38 ++++++++
.../processors/query/calcite/util/TypeUtils.java | 5 +
.../query/calcite/integration/DataTypesTest.java | 102 +++++++++++++++++++++
6 files changed, 155 insertions(+), 4 deletions(-)
diff --git
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java
index 974113a0401..6b5a2661939 100644
---
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java
+++
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java
@@ -114,6 +114,8 @@ import
org.apache.ignite.internal.processors.query.calcite.util.RexUtils;
import org.apache.ignite.internal.util.typedef.F;
import static org.apache.calcite.rel.RelDistribution.Type.HASH_DISTRIBUTED;
+import static org.apache.calcite.sql.SqlKind.IS_DISTINCT_FROM;
+import static org.apache.calcite.sql.SqlKind.IS_NOT_DISTINCT_FROM;
import static
org.apache.ignite.internal.processors.query.calcite.util.TypeUtils.combinedRowType;
/**
@@ -298,7 +300,8 @@ public class LogicalRelImplementor<Row> implements
IgniteRelVisitor<Node<Row>> {
Comparator<Row> comp = expressionFactory.comparator(
rel.leftCollation().getFieldCollations().subList(0, pairsCnt),
- rel.rightCollation().getFieldCollations().subList(0, pairsCnt)
+ rel.rightCollation().getFieldCollations().subList(0, pairsCnt),
+ rel.getCondition().getKind() == IS_NOT_DISTINCT_FROM ||
rel.getCondition().getKind() == IS_DISTINCT_FROM
);
Node<Row> node = MergeJoinNode.create(ctx, outType, leftType,
rightType, joinType, comp, hasExchange(rel));
diff --git
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactory.java
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactory.java
index a68078f6c42..3fb64377322 100644
---
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactory.java
+++
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactory.java
@@ -60,9 +60,11 @@ public interface ExpressionFactory<Row> {
*
* @param left Collations of left row.
* @param right Collations of right row.
+ * @param nullsEqual If {@code true}, nulls are considered equal. Usually,
NULL <> NULL in SQL. So, the value should
+ * be {@code false}. Except cases with IS DISTINCT / IS
NOT DISTINCT.
* @return Rows comparator.
*/
- Comparator<Row> comparator(List<RelFieldCollation> left,
List<RelFieldCollation> right);
+ Comparator<Row> comparator(List<RelFieldCollation> left,
List<RelFieldCollation> right, boolean nullsEqual);
/**
* Creates a Filter predicate.
diff --git
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactoryImpl.java
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactoryImpl.java
index 187d2739457..910cedc8d14 100644
---
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactoryImpl.java
+++
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactoryImpl.java
@@ -192,7 +192,7 @@ public class ExpressionFactoryImpl<Row> implements
ExpressionFactory<Row> {
}
/** {@inheritDoc} */
- @Override public Comparator<Row> comparator(List<RelFieldCollation> left,
List<RelFieldCollation> right) {
+ @Override public Comparator<Row> comparator(List<RelFieldCollation> left,
List<RelFieldCollation> right, boolean nullsEqual) {
if (F.isEmpty(left) || F.isEmpty(right) || left.size() != right.size())
throw new IllegalArgumentException("Both inputs should be
non-empty and have the same size: left="
+ (left != null ? left.size() : "null") + ", right=" + (right
!= null ? right.size() : "null"));
@@ -237,7 +237,8 @@ public class ExpressionFactoryImpl<Row> implements
ExpressionFactory<Row> {
}
// If compared rows contain NULLs, they shouldn't be treated
as equals, since NULL <> NULL in SQL.
- return hasNulls ? 1 : 0;
+ // Except cases with IS DISTINCT / IS NOT DISTINCT.
+ return hasNulls && !nullsEqual ? 1 : 0;
}
};
}
diff --git
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteTypeCoercion.java
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteTypeCoercion.java
index 0528f37a090..8cba0419885 100644
---
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteTypeCoercion.java
+++
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteTypeCoercion.java
@@ -18,12 +18,14 @@
package org.apache.ignite.internal.processors.query.calcite.prepare;
import java.nio.charset.Charset;
+import java.util.Arrays;
import org.apache.calcite.adapter.java.JavaTypeFactory;
import org.apache.calcite.rel.type.DynamicRecordType;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rel.type.RelDataTypeFactoryImpl;
import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlCallBinding;
import org.apache.calcite.sql.SqlCollation;
import org.apache.calcite.sql.SqlDataTypeSpec;
import org.apache.calcite.sql.SqlIdentifier;
@@ -33,6 +35,7 @@ import org.apache.calcite.sql.SqlNodeList;
import org.apache.calcite.sql.SqlUserDefinedTypeNameSpec;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.sql.type.SqlTypeFamily;
import org.apache.calcite.sql.type.SqlTypeUtil;
import org.apache.calcite.sql.validate.SqlValidator;
import org.apache.calcite.sql.validate.SqlValidatorScope;
@@ -55,6 +58,41 @@ public class IgniteTypeCoercion extends TypeCoercionImpl {
super(typeFactory, validator);
}
+ /** {@inheritDoc} **/
+ @Override public boolean binaryComparisonCoercion(SqlCallBinding binding) {
+ // Although it is not reflected in the docs, this method is also
invoked for MAX, MIN (and other similar operators)
+ // by ComparableOperandTypeChecker. When that is the case, fallback to
default rules.
+ SqlCall call = binding.getCall();
+
+ if (binding.getOperandCount() != 2 ||
!SqlKind.BINARY_COMPARISON.contains(call.getKind()))
+ return super.binaryComparisonCoercion(binding);
+
+ SqlValidatorScope scope = binding.getScope();
+
+ RelDataType leftType = validator.deriveType(scope, call.operand(0));
+ RelDataType rightType = validator.deriveType(scope, call.operand(1));
+
+ if (leftType.equals(rightType))
+ return super.binaryComparisonCoercion(binding);
+ else {
+ // Find the least restrictive type among the operand types
+ // and coerce the operands to that type if such type exists.
+ //
+ // An example of a least restrictive type from the javadoc for
RelDataTypeFactory::leastRestrictive:
+ // leastRestrictive(INT, NUMERIC(3, 2)) could be NUMERIC(12, 2)
+ //
+ // A least restrictive type between types of different type
families does not exist -
+ // the method returns null (See
SqlTypeFactoryImpl::leastRestrictive).
+ //
+ RelDataType targetType =
factory.leastRestrictive(Arrays.asList(leftType, rightType));
+
+ if (targetType == null || targetType.getFamily() ==
SqlTypeFamily.ANY)
+ return super.binaryComparisonCoercion(binding);
+ else
+ return coerceOperandsType(scope, call, targetType);
+ }
+ }
+
/** {@inheritDoc} */
@Override protected boolean coerceOperandType(
SqlValidatorScope scope,
diff --git
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/TypeUtils.java
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/TypeUtils.java
index 811f6c692d1..7fda6a03b6f 100644
---
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/TypeUtils.java
+++
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/TypeUtils.java
@@ -145,6 +145,11 @@ public class TypeUtils {
return false;
}
+ // Currently, RelDataTypeFactoryImpl#CLASS_FAMILIES doesn't consider
the byte type as an integer.
+ if ((fromType.getSqlTypeName() == SqlTypeName.TINYINT &&
SqlTypeUtil.isIntType(toType))
+ || (toType.getSqlTypeName() == SqlTypeName.TINYINT &&
SqlTypeUtil.isIntType(fromType)))
+ return false;
+
// Implicit type coercion does not handle nullability.
if (SqlTypeUtil.equalSansNullability(factory, fromType, toType))
return false;
diff --git
a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DataTypesTest.java
b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DataTypesTest.java
index 23474698fe5..c350fc2d80f 100644
---
a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DataTypesTest.java
+++
b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DataTypesTest.java
@@ -18,18 +18,21 @@
package org.apache.ignite.internal.processors.query.calcite.integration;
import java.math.BigDecimal;
+import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.UUID;
import java.util.stream.Collectors;
import com.google.common.collect.ImmutableSet;
import org.apache.calcite.runtime.CalciteException;
+import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.tools.FrameworkConfig;
import org.apache.calcite.tools.Frameworks;
import org.apache.ignite.IgniteCache;
import org.apache.ignite.cache.QueryEntity;
import org.apache.ignite.configuration.CacheConfiguration;
import org.apache.ignite.internal.processors.query.IgniteSQLException;
+import org.apache.ignite.internal.processors.query.calcite.hint.HintDefinition;
import org.apache.ignite.internal.util.typedef.F;
import org.junit.Test;
@@ -450,6 +453,105 @@ public class DataTypesTest extends
AbstractBasicIntegrationTest {
.check();
}
+ /** */
+ @Test
+ public void testIsNotDistinctFromTypeConversion() {
+ SqlTypeName[] numerics = new SqlTypeName[] {SqlTypeName.TINYINT,
SqlTypeName.SMALLINT, SqlTypeName.INTEGER,
+ SqlTypeName.BIGINT, SqlTypeName.DECIMAL, SqlTypeName.FLOAT,
SqlTypeName.DOUBLE};
+
+ sql("CREATE TABLE t1(key1 INTEGER, i1idx INTEGER, i1 INTEGER, chr1
VARCHAR, PRIMARY KEY(key1))");
+ sql("CREATE INDEX t1_idx ON t1(i1idx)");
+ sql("INSERT INTO t1 VALUES (1, 1, null, '1'), (2, 2, 2, '22'), (3, 33,
3, null), (4, null, 4, '4')");
+
+ for (SqlTypeName type : numerics) {
+ String t = type.getName();
+
+ sql("CREATE TABLE t2(key2 " + t + ", i2idx " + t + ", i2 " + t +
", i3 INTEGER, chr2 VARCHAR, PRIMARY KEY(key2))");
+ sql("CREATE INDEX t2_idx ON t2(i2idx)");
+ sql("INSERT INTO t2 VALUES (0, 0, 0, null, '0'), (11, null, 1, 1,
'1'), (2, 2, 2, 2, '22'), (3, 3, null, 3, null)");
+
+ for (HintDefinition hint :
Arrays.asList(HintDefinition.MERGE_JOIN, HintDefinition.NL_JOIN,
HintDefinition.CNL_JOIN)) {
+ String h = "/*+ " + hint.name() + " */ ";
+
+ // Primary keys, indexed.
+ assertQuery("SELECT " + h + "key1, i3 FROM t1 JOIN t2 ON key2
IS NOT DISTINCT FROM key1")
+ .returns(2, 2)
+ .returns(3, 3)
+ .check();
+
+ // Indexed and not indexed.
+ assertQuery("SELECT " + h + "key1, i3 FROM t1 JOIN t2 ON i2idx
IS NOT DISTINCT FROM i1")
+ .returns(1, 1)
+ .returns(2, 2)
+ .returns(3, 3)
+ .check();
+
+ // Both not indexed.
+ assertQuery("SELECT " + h + "key1, i3 FROM t1 JOIN t2 ON i2 IS
NOT DISTINCT FROM i1")
+ .returns(1, 3)
+ .returns(2, 2)
+ .check();
+
+ // Indexed and casted.
+ assertQuery("SELECT " + h + "key1, i3 FROM t1 JOIN t2 ON i2idx
IS NOT DISTINCT FROM CAST(chr1 as INTEGER)")
+ .returns(3, 1)
+ .check();
+
+ // Not indexed and casted.
+ assertQuery("SELECT " + h + "key1, i3 FROM t1 JOIN t2 ON i2 IS
NOT DISTINCT FROM CAST(chr1 as INTEGER)")
+ .returns(1, 1)
+ .returns(3, 3)
+ .check();
+
+ // @see MergeJoinConverterRule#matchesJoin(RelOptRuleCall)
+ if (hint == HintDefinition.MERGE_JOIN)
+ continue;
+
+ // Primary keys, indexed.
+ assertQuery("SELECT " + h + "key1, i3 FROM t1 JOIN t2 ON key2
IS DISTINCT FROM key1 and key1<2")
+ .returns(1, 1)
+ .returns(1, 2)
+ .returns(1, 3)
+ .returns(1, null)
+ .check();
+
+ // Indexed and not indexed.
+ assertQuery("SELECT " + h + "key1, i3 FROM t1 JOIN t2 ON i2idx
IS NOT DISTINCT FROM i1")
+ .returns(1, 1)
+ .returns(2, 2)
+ .returns(3, 3)
+ .check();
+
+ // Both not indexed.
+ assertQuery("SELECT " + h + "key1, i3 FROM t1 JOIN t2 ON i2 IS
DISTINCT FROM i1 and key1<3")
+ .returns(1, 1)
+ .returns(1, 2)
+ .returns(1, null)
+ .returns(2, 1)
+ .returns(2, 3)
+ .returns(2, null)
+ .check();
+
+ // Indexed and casted.
+ assertQuery("SELECT " + h + "key1, i3 FROM t1 JOIN t2 ON i2idx
IS DISTINCT FROM CAST(chr1 as INTEGER) and key1<2")
+ .returns(1, null)
+ .returns(1, 2)
+ .returns(1, 3)
+ .returns(1, 1)
+ .check();
+
+ // Not indexed and casted.
+ assertQuery("SELECT " + h + "key1, i3 FROM t1 JOIN t2 ON i2 IS
DISTINCT FROM CAST(chr1 as INTEGER) and key1<2")
+ .returns(1, null)
+ .returns(1, 2)
+ .returns(1, 3)
+ .check();
+ }
+
+ sql("DROP TABLE t2");
+ }
+ }
+
/** */
@Test
public void testNumericConversion() {
