From 82c2f50af7d8b3490708773c37ed59fb2a479625 Mon Sep 17 00:00:00 2001 From: Vladimir Steshin Date: Mon, 3 Feb 2025 13:32:09 +0300 Subject: [PATCH 1/4] raw --- .../calcite/integration/DynamicParametersIntegrationTest.java | 4 ++++ .../processors/query/h2/sql/GridQueryParsingTest.java | 1 + 2 files changed, 5 insertions(+) diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java index b3eea80c0f32f..b0a6b205531a6 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java +++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java @@ -63,6 +63,10 @@ public void testMetadataTypesForDynamicParameters() { /** */ @Test public void testDynamicParameters() { + assertQuery("select 1 + ?").withParams(new BigDecimal("2")).returns(3L); + assertQuery("select 1 + ?").withParams(2L).returns(3L); + assertQuery("select 1 + CAST(? AS DECIMAL(5,2)").withParams(2).returns(3L); + assertQuery("SELECT COALESCE(?, ?)").withParams("a", 10).returns("a").check(); assertQuery("SELECT COALESCE(null, ?)").withParams(13).returns(13).check(); assertQuery("SELECT LOWER(?)").withParams("ASD").returns("asd").check(); diff --git a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/sql/GridQueryParsingTest.java b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/sql/GridQueryParsingTest.java index ec08ff5dac1b2..7866daf50d4c3 100644 --- a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/sql/GridQueryParsingTest.java +++ b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/sql/GridQueryParsingTest.java @@ -145,6 +145,7 @@ public void testParseSelectAndUnion() throws Exception { checkQuery("select (select 1)"); checkQuery("select (select 1, select ?)"); checkQuery("select ((select 1), select ? + ?)"); + checkQuery("select 1 + ?"); checkQuery("select CURRENT_DATE"); checkQuery("select CURRENT_DATE()"); From 23618fee58b3551b4fd00c3193fd318114b64157 Mon Sep 17 00:00:00 2001 From: Vladimir Steshin Date: Mon, 3 Feb 2025 20:19:41 +0300 Subject: [PATCH 2/4] fix_with_no_coerce_nulls --- .../query/calcite/exec/exp/IgniteRexBuilder.java | 11 +++++++++++ .../query/calcite/prepare/IgniteSqlValidator.java | 5 +---- .../query/calcite/prepare/IgniteTypeCoercion.java | 6 +++++- .../DynamicParametersIntegrationTest.java | 11 ++++++++--- .../calcite/integration/PartitionPruneTest.java | 14 +++++++------- .../query/h2/sql/GridQueryParsingTest.java | 2 ++ 6 files changed, 34 insertions(+), 15 deletions(-) diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/IgniteRexBuilder.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/IgniteRexBuilder.java index d22c9079fe11c..a6b5cde2df191 100644 --- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/IgniteRexBuilder.java +++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/IgniteRexBuilder.java @@ -23,7 +23,9 @@ import org.apache.calcite.rel.type.RelDataTypeFactory; import org.apache.calcite.rex.RexBuilder; import org.apache.calcite.rex.RexLiteral; +import org.apache.calcite.rex.RexNode; import org.apache.calcite.sql.type.IntervalSqlType; +import org.apache.calcite.sql.type.SqlTypeFamily; import org.apache.calcite.sql.type.SqlTypeName; import org.apache.calcite.sql.type.SqlTypeUtil; import org.apache.ignite.internal.processors.query.IgniteSQLException; @@ -64,4 +66,13 @@ public IgniteRexBuilder(RelDataTypeFactory typeFactory) { return super.makeLiteral(o, type, typeName); } + + /** {@inheritDoc} */ + @Override public RexNode ensureType(RelDataType type, RexNode node, boolean matchNullability) { + // Do not wrap with additional CAST if NULL is acceptable. + if (node.getType().getFamily() == SqlTypeFamily.NULL && (type.isNullable() || !matchNullability)) + return node; + + return super.ensureType(type, node, matchNullability); + } } diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java index 91fe45db4b382..fadd1ce258cd2 100644 --- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java +++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java @@ -565,9 +565,6 @@ private boolean isSystemFieldName(String alias) { Object val = parameters[node.getIndex()]; - if (val == null && type != null) - return type; - type = val == null ? typeFactory.createSqlType(SqlTypeName.NULL) : typeFactory().createTypeWithNullability(typeFactory().toSql(typeFactory().createType(val.getClass())), true); @@ -579,7 +576,7 @@ private boolean isSystemFieldName(String alias) { /** {@inheritDoc} */ @Override protected void inferUnknownTypes(RelDataType inferredType, SqlValidatorScope scope, SqlNode node) { - if (node instanceof SqlDynamicParam && unknownType.equals(inferredType) + if (node instanceof SqlDynamicParam && !(inferredType instanceof OtherType) && deriveDynamicParameterType((SqlDynamicParam)node) != null) return; 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 8cba041988535..9f5629f8bdcb9 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 @@ -86,7 +86,11 @@ public IgniteTypeCoercion(RelDataTypeFactory typeFactory, SqlValidator validator // RelDataType targetType = factory.leastRestrictive(Arrays.asList(leftType, rightType)); - if (targetType == null || targetType.getFamily() == SqlTypeFamily.ANY) + // Do not wrap additional CAST if NULL is acceptable. + if (targetType == null || targetType.getFamily() == SqlTypeFamily.ANY + || (leftType.getFamily() == SqlTypeFamily.NULL && rightType.isNullable()) + || (rightType.getFamily() == SqlTypeFamily.NULL && leftType.isNullable()) + ) return super.binaryComparisonCoercion(binding); else return coerceOperandsType(scope, call, targetType); diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java index 7921cf8037359..af3f92dd25b89 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java +++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/DynamicParametersIntegrationTest.java @@ -105,9 +105,14 @@ public void testCasts() { /** */ @Test public void testDynamicParameters() { - assertQuery("select 1 + ?").withParams(new BigDecimal("2")).returns(3L); - assertQuery("select 1 + ?").withParams(2L).returns(3L); - assertQuery("select 1 + CAST(? AS DECIMAL(5,2)").withParams(2).returns(3L); + assertQuery("select 1 + ?").withParams(1).returns(2).check(); + assertQuery("select ? + 1").withParams(1).returns(2).check(); + assertQuery("select 1 + CAST(? AS INTEGER)").withParams(2L).returns(3).check(); + assertQuery("select CAST(? AS INTEGER) + 1").withParams(2L).returns(3).check(); + assertQuery("select 1 + ?").withParams(1L).returns(2L).check(); + assertQuery("select ? + 1").withParams(1L).returns(2L).check(); + assertQuery("select 1 + ?").withParams(new BigDecimal("2")).returns(new BigDecimal(3)).check(); + assertQuery("select ? + 1").withParams(new BigDecimal("2")).returns(new BigDecimal(3)).check(); assertQuery("SELECT COALESCE(?, ?)").withParams("a", 10).returns("a").check(); assertQuery("SELECT COALESCE(null, ?)").withParams(13).returns(13).check(); diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/PartitionPruneTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/PartitionPruneTest.java index ac6fc2c47adee..e8d893cb7ffa0 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/PartitionPruneTest.java +++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/PartitionPruneTest.java @@ -217,13 +217,13 @@ public void testSimple() { /** */ @Test public void testNullsInCondition() { - execute("select * from T1 where T1.ID is NULL", - res -> { - assertPartitions(); - assertNodes(); - - assertTrue(res.isEmpty()); - }); +// execute("select * from T1 where T1.ID is NULL", +// res -> { +// assertPartitions(); +// assertNodes(); +// +// assertTrue(res.isEmpty()); +// }); execute("select * from T1 where T1.ID = ?", res -> { diff --git a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/sql/GridQueryParsingTest.java b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/sql/GridQueryParsingTest.java index 7866daf50d4c3..71f290d54cf41 100644 --- a/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/sql/GridQueryParsingTest.java +++ b/modules/indexing/src/test/java/org/apache/ignite/internal/processors/query/h2/sql/GridQueryParsingTest.java @@ -146,6 +146,8 @@ public void testParseSelectAndUnion() throws Exception { checkQuery("select (select 1, select ?)"); checkQuery("select ((select 1), select ? + ?)"); checkQuery("select 1 + ?"); + checkQuery("select ? + 1"); + checkQuery("select ? + ?"); checkQuery("select CURRENT_DATE"); checkQuery("select CURRENT_DATE()"); From 0974790a08521a58cc6fe5384a92f7a0c4bd7b7c Mon Sep 17 00:00:00 2001 From: Vladimir Steshin Date: Tue, 4 Feb 2025 14:03:23 +0300 Subject: [PATCH 3/4] revert test --- .../calcite/integration/PartitionPruneTest.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/PartitionPruneTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/PartitionPruneTest.java index e8d893cb7ffa0..ac6fc2c47adee 100644 --- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/PartitionPruneTest.java +++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/PartitionPruneTest.java @@ -217,13 +217,13 @@ public void testSimple() { /** */ @Test public void testNullsInCondition() { -// execute("select * from T1 where T1.ID is NULL", -// res -> { -// assertPartitions(); -// assertNodes(); -// -// assertTrue(res.isEmpty()); -// }); + execute("select * from T1 where T1.ID is NULL", + res -> { + assertPartitions(); + assertNodes(); + + assertTrue(res.isEmpty()); + }); execute("select * from T1 where T1.ID = ?", res -> { From b77f3e1348963ac5f859249455c5dbba5b6a314d Mon Sep 17 00:00:00 2001 From: Vladimir Steshin Date: Sat, 8 Feb 2025 01:30:42 +0300 Subject: [PATCH 4/4] review fix --- .../calcite/exec/exp/IgniteRexBuilder.java | 11 ---------- .../calcite/prepare/IgniteSqlValidator.java | 20 +++++++++++++------ .../calcite/prepare/IgniteTypeCoercion.java | 6 +----- 3 files changed, 15 insertions(+), 22 deletions(-) diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/IgniteRexBuilder.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/IgniteRexBuilder.java index a6b5cde2df191..d22c9079fe11c 100644 --- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/IgniteRexBuilder.java +++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/IgniteRexBuilder.java @@ -23,9 +23,7 @@ import org.apache.calcite.rel.type.RelDataTypeFactory; import org.apache.calcite.rex.RexBuilder; import org.apache.calcite.rex.RexLiteral; -import org.apache.calcite.rex.RexNode; import org.apache.calcite.sql.type.IntervalSqlType; -import org.apache.calcite.sql.type.SqlTypeFamily; import org.apache.calcite.sql.type.SqlTypeName; import org.apache.calcite.sql.type.SqlTypeUtil; import org.apache.ignite.internal.processors.query.IgniteSQLException; @@ -66,13 +64,4 @@ public IgniteRexBuilder(RelDataTypeFactory typeFactory) { return super.makeLiteral(o, type, typeName); } - - /** {@inheritDoc} */ - @Override public RexNode ensureType(RelDataType type, RexNode node, boolean matchNullability) { - // Do not wrap with additional CAST if NULL is acceptable. - if (node.getType().getFamily() == SqlTypeFamily.NULL && (type.isNullable() || !matchNullability)) - return node; - - return super.ensureType(type, node, matchNullability); - } } diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java index fadd1ce258cd2..9bf66bd52574a 100644 --- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java +++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IgniteSqlValidator.java @@ -105,6 +105,9 @@ public class IgniteSqlValidator extends SqlValidatorImpl { /** Dynamic parameters. */ @Nullable private final Object[] parameters; + /** */ + private final RelDataType nullType; + /** * Creates a validator. * @@ -124,6 +127,8 @@ public IgniteSqlValidator( super(opTab, catalogReader, typeFactory, cfg); this.parameters = parameters; + + nullType = typeFactory.createSqlType(SqlTypeName.NULL); } /** {@inheritDoc} */ @@ -543,7 +548,7 @@ private boolean isSystemFieldName(String alias) { /** {@inheritDoc} */ @Override public RelDataType deriveType(SqlValidatorScope scope, SqlNode expr) { if (expr instanceof SqlDynamicParam) { - RelDataType type = deriveDynamicParameterType((SqlDynamicParam)expr); + RelDataType type = deriveDynamicParameterType((SqlDynamicParam)expr, nullType); if (type != null) return type; @@ -553,20 +558,23 @@ private boolean isSystemFieldName(String alias) { } /** @return A derived type or {@code null} if unable to determine. */ - @Nullable private RelDataType deriveDynamicParameterType(SqlDynamicParam node) { + @Nullable private RelDataType deriveDynamicParameterType(SqlDynamicParam node, RelDataType nullValType) { + if (parameters == null || node.getIndex() >= parameters.length) + return null; + RelDataType type = getValidatedNodeTypeIfKnown(node); // Do not clarify the widest type for any value. if (type instanceof OtherType) return type; - if (parameters == null || node.getIndex() >= parameters.length) - return null; + if (type == nullType) + return nullValType; Object val = parameters[node.getIndex()]; type = val == null - ? typeFactory.createSqlType(SqlTypeName.NULL) + ? nullValType : typeFactory().createTypeWithNullability(typeFactory().toSql(typeFactory().createType(val.getClass())), true); setValidatedNodeType(node, type); @@ -577,7 +585,7 @@ private boolean isSystemFieldName(String alias) { /** {@inheritDoc} */ @Override protected void inferUnknownTypes(RelDataType inferredType, SqlValidatorScope scope, SqlNode node) { if (node instanceof SqlDynamicParam && !(inferredType instanceof OtherType) - && deriveDynamicParameterType((SqlDynamicParam)node) != null) + && deriveDynamicParameterType((SqlDynamicParam)node, unknownType.equals(inferredType) ? nullType : inferredType) != null) return; if (node instanceof SqlCall) { 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 9f5629f8bdcb9..8cba041988535 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 @@ -86,11 +86,7 @@ public IgniteTypeCoercion(RelDataTypeFactory typeFactory, SqlValidator validator // RelDataType targetType = factory.leastRestrictive(Arrays.asList(leftType, rightType)); - // Do not wrap additional CAST if NULL is acceptable. - if (targetType == null || targetType.getFamily() == SqlTypeFamily.ANY - || (leftType.getFamily() == SqlTypeFamily.NULL && rightType.isNullable()) - || (rightType.getFamily() == SqlTypeFamily.NULL && leftType.isNullable()) - ) + if (targetType == null || targetType.getFamily() == SqlTypeFamily.ANY) return super.binaryComparisonCoercion(binding); else return coerceOperandsType(scope, call, targetType);