IGNITE-23179 SQL Calcite: Fix wrong numeric type coercion with 'IS DI…

…STINCT FROM' - Fixes #11525.

Signed-off-by: Aleksey Plekhanov <[email protected]>

modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java

@@ -114,6 +114,8 @@
 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 LogicalRelImplementor(
 
         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));

modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactory.java

@@ -60,9 +60,11 @@ Supplier<List<AccumulatorWrapper<Row>>> accumulatorsFactory(
      *
      * @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.

modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ExpressionFactoryImpl.java

@@ -192,7 +192,7 @@ else if (c2 != HIGHEST_VALUE)
     }
 
     /** {@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 @@ else if (c2 != HIGHEST_VALUE)
                 }
 
                 // 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;
             }
         };
     }

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.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 IgniteTypeCoercion(RelDataTypeFactory typeFactory, SqlValidator validator
         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,

modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/TypeUtils.java

@@ -145,6 +145,11 @@ public static boolean needCast(RelDataTypeFactory factory, RelDataType fromType,
             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;

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 void testDecimalScale() {
             .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() {