You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by st...@apache.org on 2026/10/13 02:02:22 UTC
(impala) 01/02: IMPALA-15269: Decimal operands in comparisons do not need casting
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 313c8062b6ec1d07715a8b9f3fc6c606a3ea2ab3
Author: Steve Carlin <sc...@cloudera.com>
AuthorDate: Tue Aug 18 09:04:16 2026 -0700
IMPALA-15269: Decimal operands in comparisons do not need casting
In the Calcite planner, we need to avoid the casting of decimal operands
in binary comparisons. Before this commit, a query failed because it
could not find a compatible decimal to dec(22,8) and dec(36,35).
Testing: Ran e2e suite with Calcite planner on to make sure there
were no wrong results.
Change-Id: I226e4a7d105e963f3263fb8a71acb5fdef7ffa94
Reviewed-on: http://gerrit.cloudera.org:8080/24709
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../src/main/codegen/templates/Parser.jj | 8 ++--
.../calcite/operators/ImpalaConvertletTable.java | 43 +++++++++++++++---
.../operators/ImpalaCustomOperatorTable.java | 52 ++++++++++++++++++++++
.../queries/QueryTest/calcite.test | 8 ++++
4 files changed, 101 insertions(+), 10 deletions(-)
diff --git a/java/calcite-planner/src/main/codegen/templates/Parser.jj b/java/calcite-planner/src/main/codegen/templates/Parser.jj
index 6b0562592..0a6a17fdd 100644
--- a/java/calcite-planner/src/main/codegen/templates/Parser.jj
+++ b/java/calcite-planner/src/main/codegen/templates/Parser.jj
@@ -3336,7 +3336,7 @@ void AddSubsetDefinition(List<SqlNode> list) :
varList = ExpressionCommaList(span(), ExprContext.ACCEPT_NON_QUERY)
<RPAREN> {
list.add(
- SqlStdOperatorTable.EQUALS.createCall(span().end(var), var,
+ ImpalaCustomOperatorTable.EQUALS.createCall(span().end(var), var,
varList));
}
}
@@ -7775,12 +7775,12 @@ SqlBinaryOperator BinaryRowOperator() :
}
{
// <IN> is handled as a special case
- <EQ> { return SqlStdOperatorTable.EQUALS; }
+ <EQ> { return ImpalaCustomOperatorTable.EQUALS; }
| <GT> { return SqlStdOperatorTable.GREATER_THAN; }
| <LT> { return SqlStdOperatorTable.LESS_THAN; }
| <LE> { return SqlStdOperatorTable.LESS_THAN_OR_EQUAL; }
| <GE> { return SqlStdOperatorTable.GREATER_THAN_OR_EQUAL; }
-| <NE> { return SqlStdOperatorTable.NOT_EQUALS; }
+| <NE> { return ImpalaCustomOperatorTable.NOT_EQUALS; }
| <NULL_SAFE_EQUAL> { return SqlStdOperatorTable.IS_NOT_DISTINCT_FROM; }
| <REGEXP> { return SqlStdOperatorTable.POSIX_REGEX_CASE_SENSITIVE; }
| LOOKAHEAD(2) <NOT> <REGEXP> { return SqlStdOperatorTable.NEGATED_POSIX_REGEX_CASE_SENSITIVE; }
@@ -7790,7 +7790,7 @@ SqlBinaryOperator BinaryRowOperator() :
if (!this.conformance.isBangEqualAllowed()) {
throw SqlUtil.newContextException(getPos(), RESOURCE.bangEqualNotAllowed());
}
- return SqlStdOperatorTable.NOT_EQUALS;
+ return ImpalaCustomOperatorTable.NOT_EQUALS;
}
| <PLUS> { return ImpalaCustomOperatorTable.PLUS; }
| <MINUS> { return ImpalaCustomOperatorTable.MINUS; }
diff --git a/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaConvertletTable.java b/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaConvertletTable.java
index afa10220d..6f3146a8f 100644
--- a/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaConvertletTable.java
+++ b/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaConvertletTable.java
@@ -24,7 +24,6 @@ import org.apache.calcite.rex.RexBuilder;
import org.apache.calcite.rex.RexCall;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.sql.SqlCall;
-import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
@@ -61,8 +60,11 @@ public class ImpalaConvertletTable extends ReflectiveConvertletTable {
public ImpalaConvertletTable() {
addAlias(ImpalaCustomOperatorTable.PERCENT_REMAINDER, SqlStdOperatorTable.MOD);
registerOp(ImpalaCastFunction.INSTANCE, this::convertExplicitCast);
- registerOp(SqlStdOperatorTable.IS_DISTINCT_FROM, this::convertIsDistinctFrom);
- registerOp(SqlStdOperatorTable.IS_NOT_DISTINCT_FROM, this::convertIsNotDistinctFrom);
+ registerOp(ImpalaCustomOperatorTable.IS_DISTINCT_FROM, this::convertIsDistinctFrom);
+ registerOp(ImpalaCustomOperatorTable.IS_NOT_DISTINCT_FROM,
+ this::convertIsNotDistinctFrom);
+ registerOp(ImpalaCustomOperatorTable.EQUALS, this::convertEquals);
+ registerOp(ImpalaCustomOperatorTable.NOT_EQUALS, this::convertNotEquals);
registerOp(ImpalaConcatOrOperator.INSTANCE, this::convertConcatOr);
registerOp(SqlStdOperatorTable.SQRT, this::convertSqrt);
registerOp(SqlStdOperatorTable.PLUS, this::convertPlus);
@@ -78,9 +80,12 @@ public class ImpalaConvertletTable extends ReflectiveConvertletTable {
return super.get(call);
}
- if (call.getOperator().getKind().equals(SqlKind.IS_DISTINCT_FROM) ||
- call.getOperator().getKind().equals(SqlKind.IS_NOT_DISTINCT_FROM)) {
- return super.get(call);
+ switch (call.getOperator().getKind()) {
+ case IS_DISTINCT_FROM:
+ case IS_NOT_DISTINCT_FROM:
+ case EQUALS:
+ case NOT_EQUALS:
+ return super.get(call);
}
// EXPLICIT_CAST convertlet has to be handled by our convertlet. Operation
@@ -170,6 +175,32 @@ public class ImpalaConvertletTable extends ReflectiveConvertletTable {
operands);
}
+ /**
+ * convertEquals calls the regular Calcite convertCall, but changes the operator
+ * from ImpalaCustomOperatorTable.EQUALS to SqlStdOperatorTable.EQUALS
+ */
+ private RexNode convertEquals(
+ @UnknownInitialization ImpalaConvertletTable this,
+ SqlRexContext cx, SqlCall call) {
+ final RexCall rex = (RexCall) StandardConvertletTable.INSTANCE.convertCall(cx, call);
+ final RexBuilder rexBuilder = cx.getRexBuilder();
+ return rexBuilder.makeCall(rex.getType(), SqlStdOperatorTable.EQUALS,
+ rex.getOperands());
+ }
+
+ /**
+ * convertNotEquals calls the regular Calcite convertCall, but changes the operator
+ * from ImpalaCustomOperatorTable.NOT_EQUALS to SqlStdOperatorTable.NOT_EQUALS
+ */
+ private RexNode convertNotEquals(
+ @UnknownInitialization ImpalaConvertletTable this,
+ SqlRexContext cx, SqlCall call) {
+ final RexCall rex = (RexCall) StandardConvertletTable.INSTANCE.convertCall(cx, call);
+ final RexBuilder rexBuilder = cx.getRexBuilder();
+ return rexBuilder.makeCall(rex.getType(), SqlStdOperatorTable.NOT_EQUALS,
+ rex.getOperands());
+ }
+
/**
* Convertlet for the DATETIME PLUS operator to work around CALCITE-5207.
*/
diff --git a/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java b/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java
index 2a13d6ed8..7ea76f6be 100644
--- a/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java
+++ b/java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java
@@ -234,6 +234,58 @@ public class ImpalaCustomOperatorTable extends ReflectiveSqlOperatorTable {
public static final SqlAggFunction GROUPING_ID =
new ImpalaGroupingIdFunction();
+ // IMPALA-15269: Same as SqlStdOperatorTable.EQUALS, but last param
+ // is ORDERED instead of UNORDERED. The UNORDERED checker results in
+ // a consistency change in the params to ensure each operand takes on
+ // the "leastRestrictive" type. In the case of comparing a Dec(22,8)
+ // with a Dec(36,5), Impala returns INVALID_TYPE, causing the query
+ // to fail. However, it is ok to compare these two decimal types.
+ // Any other mismatch of types will be taken care of in the "coercenodes"
+ // module.
+ // Note: This operator is only needed at validation time when doing operand
+ // checks. Unfortunately, there are places in Calcite that look explicitly
+ // for (and create) the SqlStdOperatorTable.EQUALS variable during optimization
+ // rules. So this operator gets converted to the SqlStdOperatorTable one at
+ // SqlNode to RelConverter time via the ImpalaConvertletTable object.
+ public static final SqlBinaryOperator EQUALS =
+ new SqlBinaryOperator(
+ "=",
+ SqlKind.EQUALS,
+ 30,
+ true,
+ ReturnTypes.BOOLEAN_NULLABLE,
+ InferTypes.FIRST_KNOWN,
+ OperandTypes.COMPARABLE_ORDERED_COMPARABLE_ORDERED);
+
+ public static final SqlBinaryOperator NOT_EQUALS =
+ new SqlBinaryOperator(
+ "<>",
+ SqlKind.NOT_EQUALS,
+ 30,
+ true,
+ ReturnTypes.BOOLEAN_NULLABLE,
+ InferTypes.FIRST_KNOWN,
+ OperandTypes.COMPARABLE_ORDERED_COMPARABLE_ORDERED);
+
+ public static final SqlBinaryOperator IS_DISTINCT_FROM =
+ new SqlBinaryOperator(
+ "IS DISTINCT FROM",
+ SqlKind.IS_DISTINCT_FROM,
+ 30,
+ true,
+ ReturnTypes.BOOLEAN,
+ InferTypes.FIRST_KNOWN,
+ OperandTypes.COMPARABLE_ORDERED_COMPARABLE_ORDERED);
+
+ public static final SqlBinaryOperator IS_NOT_DISTINCT_FROM =
+ new SqlBinaryOperator(
+ "IS NOT DISTINCT FROM",
+ SqlKind.IS_NOT_DISTINCT_FROM,
+ 30,
+ true,
+ ReturnTypes.BOOLEAN,
+ InferTypes.FIRST_KNOWN,
+ OperandTypes.COMPARABLE_ORDERED_COMPARABLE_ORDERED);
public static final ImpalaConcatOrOperator CONCAT_OR = ImpalaConcatOrOperator.INSTANCE;
// The explicit cast function was created to deal with the cast function using
diff --git a/testdata/workloads/functional-query/queries/QueryTest/calcite.test b/testdata/workloads/functional-query/queries/QueryTest/calcite.test
index f4e6b8e87..6cc79287b 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/calcite.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/calcite.test
@@ -1459,6 +1459,14 @@ select hex(cast(x'616263' as string)) as text_bytes,
'616263','FF00FE',3,1.5
---- TYPES
STRING, STRING, INT, DOUBLE
+====
+---- QUERY
+# IMPALA-15269, comparing incompatible decimals
+select cast(2.3 as decimal(22,8)) = cast(2.3 as decimal(36,35));
+---- TYPES
+BOOLEAN
+---- RESULTS
+true
---- RUNTIME_PROFILE
row_regex: PlannerType: CalcitePlanner
====