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 b158d2fe180 IGNITE-22716 SQL Calcite: Fix implicit conversion to
DECIMAL for some functions - Fixes #11441.
b158d2fe180 is described below
commit b158d2fe18068cff183dd58a1f05382ae4944b69
Author: Aleksey Plekhanov <[email protected]>
AuthorDate: Fri Jul 19 10:36:38 2024 +0300
IGNITE-22716 SQL Calcite: Fix implicit conversion to DECIMAL for some
functions - Fixes #11441.
Signed-off-by: Aleksey Plekhanov <[email protected]>
---
.../processors/query/calcite/RootQuery.java | 8 +++++++-
.../query/calcite/exec/exp/ConverterUtils.java | 17 ++++++++++++++++
.../query/calcite/exec/exp/RexImpTable.java | 2 +-
.../query/calcite/exec/exp/RexToLixTranslator.java | 22 +++++++++++----------
.../processors/query/calcite/QueryChecker.java | 18 +++++++++++++++--
.../query/calcite/integration/DataTypesTest.java | 23 ++++++++++++++++++++++
6 files changed, 76 insertions(+), 14 deletions(-)
diff --git
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/RootQuery.java
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/RootQuery.java
index 31fafc96eee..e67e7f98435 100644
---
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/RootQuery.java
+++
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/RootQuery.java
@@ -29,6 +29,7 @@ import java.util.function.BiConsumer;
import java.util.stream.Collectors;
import org.apache.calcite.plan.Context;
import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.tools.FrameworkConfig;
import org.apache.calcite.tools.Frameworks;
import org.apache.calcite.util.CancelFlag;
import org.apache.ignite.IgniteCheckedException;
@@ -139,10 +140,15 @@ public class RootQuery<RowT> extends Query<RowT>
implements TrackableQuery {
Context parent = Commons.convert(qryCtx);
+ FrameworkConfig frameworkCfg = qryCtx != null ?
qryCtx.unwrap(FrameworkConfig.class) : null;
+
+ if (frameworkCfg == null)
+ frameworkCfg = FRAMEWORK_CONFIG;
+
ctx = BaseQueryContext.builder()
.parentContext(parent)
.frameworkConfig(
- Frameworks.newConfigBuilder(FRAMEWORK_CONFIG)
+ Frameworks.newConfigBuilder(frameworkCfg)
.defaultSchema(schema)
.build()
)
diff --git
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ConverterUtils.java
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ConverterUtils.java
index 0beb1197011..90c0c2af06b 100644
---
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ConverterUtils.java
+++
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/ConverterUtils.java
@@ -34,8 +34,10 @@ import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.runtime.SqlFunctions;
import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.calcite.sql.type.SqlTypeUtil;
import org.apache.calcite.util.BuiltInMethod;
import org.apache.calcite.util.Util;
+import org.apache.ignite.internal.processors.query.calcite.util.Commons;
/** */
public class ConverterUtils {
@@ -166,6 +168,21 @@ public class ConverterUtils {
return Util.transform(operandList, node -> toInternal(node.getType()));
}
+ /**
+ * Convert {@code operand} to {@code targetType}.
+ *
+ * @param operand The expression to convert
+ * @param targetType Target type
+ * @return A new expression with java type corresponding to {@code
targetType}
+ * or original expression if there is no need to convert.
+ */
+ public static Expression convert(Expression operand, RelDataType
targetType) {
+ if (SqlTypeUtil.isDecimal(targetType))
+ return convertToDecimal(operand, targetType);
+ else
+ return convert(operand,
Commons.typeFactory().getJavaClass(targetType));
+ }
+
/**
* Convert {@code operand} to target type {@code toType}.
*
diff --git
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/RexImpTable.java
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/RexImpTable.java
index e52342a4320..433fb9d2963 100644
---
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/RexImpTable.java
+++
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/RexImpTable.java
@@ -1963,7 +1963,7 @@ public class RexImpTable {
final Expression convertedCallVal =
noConvert
? callVal
- : ConverterUtils.convert(callVal, returnType);
+ : ConverterUtils.convert(callVal, call.getType());
final Expression valExpression =
Expressions.condition(condition,
diff --git
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/RexToLixTranslator.java
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/RexToLixTranslator.java
index 43ed1dbd638..e697948cc06 100644
---
a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/RexToLixTranslator.java
+++
b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/RexToLixTranslator.java
@@ -62,7 +62,6 @@ import org.apache.calcite.sql.SqlIntervalQualifier;
import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
import org.apache.calcite.sql.parser.SqlParserPos;
-import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.calcite.sql.type.SqlTypeUtil;
import org.apache.calcite.sql.validate.SqlConformance;
import org.apache.calcite.util.BuiltInMethod;
@@ -552,11 +551,9 @@ public class RexToLixTranslator implements
RexVisitor<RexToLixTranslator.Result>
}
break;
}
- if (targetType.getSqlTypeName() == SqlTypeName.DECIMAL)
- convert = ConverterUtils.convertToDecimal(operand, targetType);
if (convert == null)
- convert = ConverterUtils.convert(operand,
typeFactory.getJavaClass(targetType));
+ convert = ConverterUtils.convert(operand, targetType);
// Going from anything to CHAR(n) or VARCHAR(n), make sure value is no
// longer than n.
@@ -1073,7 +1070,7 @@ public class RexToLixTranslator implements
RexVisitor<RexToLixTranslator.Result>
list.newName("case_when_value"));
list.add(Expressions.declare(0, valVariable, null));
final List<RexNode> operandList = call.getOperands();
- implementRecursively(this, operandList, valVariable, 0);
+ implementRecursively(this, operandList, valVariable, call.getType(),
0);
final Expression isNullExpression = checkNull(valVariable);
final ParameterExpression isNullVariable =
Expressions.parameter(
@@ -1108,8 +1105,13 @@ public class RexToLixTranslator implements
RexVisitor<RexToLixTranslator.Result>
* }
* </pre></blockquote>
*/
- private void implementRecursively(final RexToLixTranslator
currentTranslator,
- final List<RexNode> operandList, final ParameterExpression
valueVariable, int pos) {
+ private void implementRecursively(
+ final RexToLixTranslator currentTranslator,
+ final List<RexNode> operandList,
+ final ParameterExpression valueVariable,
+ final RelDataType valueType,
+ int pos
+ ) {
final BlockBuilder curBlockBuilder =
currentTranslator.getBlockBuilder();
final List<Type> storageTypes =
ConverterUtils.internalTypes(operandList);
// [ELSE] clause
@@ -1119,7 +1121,7 @@ public class RexToLixTranslator implements
RexVisitor<RexToLixTranslator.Result>
curBlockBuilder.add(
Expressions.statement(
Expressions.assign(valueVariable,
- ConverterUtils.convert(res,
valueVariable.getType()))));
+ ConverterUtils.convert(res, valueType))));
return;
}
// Condition code: !a_isNull && a_value
@@ -1141,7 +1143,7 @@ public class RexToLixTranslator implements
RexVisitor<RexToLixTranslator.Result>
ifTrueBlockBuilder.add(
Expressions.statement(
Expressions.assign(valueVariable,
- ConverterUtils.convert(ifTrueRes,
valueVariable.getType()))));
+ ConverterUtils.convert(ifTrueRes, valueType))));
final BlockStatement ifTrue = ifTrueBlockBuilder.toBlock();
// There is no [ELSE] clause
if (pos + 1 == operandList.size() - 1) {
@@ -1154,7 +1156,7 @@ public class RexToLixTranslator implements
RexVisitor<RexToLixTranslator.Result>
new BlockBuilder(true, curBlockBuilder);
final RexToLixTranslator ifFalseTranslator =
currentTranslator.setBlock(ifFalseBlockBuilder);
- implementRecursively(ifFalseTranslator, operandList, valueVariable,
pos + 2);
+ implementRecursively(ifFalseTranslator, operandList, valueVariable,
valueType, pos + 2);
final BlockStatement ifFalse = ifFalseBlockBuilder.toBlock();
curBlockBuilder.add(
Expressions.ifThenElse(tester, ifTrue, ifFalse));
diff --git
a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/QueryChecker.java
b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/QueryChecker.java
index 19f12244783..2d8f936919d 100644
---
a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/QueryChecker.java
+++
b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/QueryChecker.java
@@ -28,12 +28,14 @@ import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
+import org.apache.calcite.tools.FrameworkConfig;
import org.apache.ignite.Ignite;
import org.apache.ignite.cache.query.FieldsQueryCursor;
import org.apache.ignite.internal.IgniteEx;
import org.apache.ignite.internal.IgniteInterruptedCheckedException;
import
org.apache.ignite.internal.processors.cache.distributed.dht.atomic.GridDhtAtomicCache;
import
org.apache.ignite.internal.processors.cache.distributed.dht.topology.GridDhtLocalPartition;
+import org.apache.ignite.internal.processors.query.QueryContext;
import org.apache.ignite.internal.processors.query.QueryEngine;
import
org.apache.ignite.internal.processors.query.schema.management.SchemaManager;
import org.apache.ignite.internal.util.typedef.F;
@@ -296,6 +298,9 @@ public abstract class QueryChecker {
/** */
private String exactPlan;
+ /** */
+ private FrameworkConfig frameworkCfg;
+
/** */
public QueryChecker(String qry) {
this.qry = qry;
@@ -322,6 +327,13 @@ public abstract class QueryChecker {
return this;
}
+ /** */
+ public QueryChecker withFrameworkConfig(FrameworkConfig frameworkCfg) {
+ this.frameworkCfg = frameworkCfg;
+
+ return this;
+ }
+
/** */
public QueryChecker returns(Object... res) {
if (expectedResult == null)
@@ -370,8 +382,10 @@ public abstract class QueryChecker {
// Check plan.
QueryEngine engine = getEngine();
+ QueryContext ctx = frameworkCfg != null ?
QueryContext.of(frameworkCfg) : null;
+
List<FieldsQueryCursor<List<?>>> explainCursors =
- engine.query(null, "PUBLIC", "EXPLAIN PLAN FOR " + qry, params);
+ engine.query(ctx, "PUBLIC", "EXPLAIN PLAN FOR " + qry, params);
FieldsQueryCursor<List<?>> explainCursor = explainCursors.get(0);
List<List<?>> explainRes = explainCursor.getAll();
@@ -387,7 +401,7 @@ public abstract class QueryChecker {
// Check result.
List<FieldsQueryCursor<List<?>>> cursors =
- engine.query(null, "PUBLIC", qry, params);
+ engine.query(ctx, "PUBLIC", qry, params);
FieldsQueryCursor<List<?>> cur = cursors.get(0);
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 b6dd0aa7b82..23474698fe5 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
@@ -24,6 +24,8 @@ 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.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;
@@ -31,6 +33,8 @@ import
org.apache.ignite.internal.processors.query.IgniteSQLException;
import org.apache.ignite.internal.util.typedef.F;
import org.junit.Test;
+import static
org.apache.ignite.internal.processors.query.calcite.CalciteQueryProcessor.FRAMEWORK_CONFIG;
+
/**
* Test SQL data types.
*/
@@ -467,6 +471,25 @@ public class DataTypesTest extends
AbstractBasicIntegrationTest {
.check();
}
+ /** */
+ @Test
+ public void testFunctionArgsToNumericImplicitConversion() {
+ assertQuery("select decode(?, 0, 0, 1,
1.0)").withParams(0).returns(new BigDecimal("0.0")).check();
+ assertQuery("select decode(?, 0, 0, 1,
1.0)").withParams(1).returns(new BigDecimal("1.0")).check();
+ assertQuery("select decode(?, 0, 0, 1,
1.000)").withParams(0).returns(new BigDecimal("0.000")).check();
+ assertQuery("select decode(?, 0, 0, 1,
1.000)").withParams(1).returns(new BigDecimal("1.000")).check();
+ assertQuery("select decode(?, 0, 0.0, 1,
1.000)").withParams(0).returns(new BigDecimal("0.000")).check();
+ assertQuery("select decode(?, 0, 0.000, 1,
1.0)").withParams(1).returns(new BigDecimal("1.000")).check();
+
+ // With callRewrite==true function COALESCE is rewritten to CASE and
CoalesceImplementor can't be checked.
+ FrameworkConfig frameworkCfg =
Frameworks.newConfigBuilder(FRAMEWORK_CONFIG)
+
.sqlValidatorConfig(FRAMEWORK_CONFIG.getSqlValidatorConfig().withCallRewrite(false))
+ .build();
+
+ assertQuery("select coalesce(?,
1.000)").withParams(0).withFrameworkConfig(frameworkCfg)
+ .returns(new BigDecimal("0.000")).check();
+ }
+
/** */
@Test
public void testArithmeticOverflow() {
