korlov42 commented on code in PR #2047:
URL: https://github.com/apache/ignite-3/pull/2047#discussion_r1221528338
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteSqlValidator.java:
##########
@@ -306,70 +358,46 @@ public void validateAggregateParams(SqlCall aggCall,
public RelDataType deriveType(SqlValidatorScope scope, SqlNode expr) {
RelDataType dataType = super.deriveType(scope, expr);
- if (!(expr instanceof SqlCall)) {
+ // Dynamic params
+ if (dataType.equals(unknownType) && expr instanceof SqlDynamicParam) {
+ // If type of dynamic parameter has not been inferred, use a type
of its value.
+ RelDataType paramType = getDynamicParamType((SqlDynamicParam)
expr);
+
+ // If paramType is unknown setValidatedNodeType is a no-op.
+ setValidatedNodeType(expr, paramType);
+ return paramType;
+ } else if (!(expr instanceof SqlCall)) {
return dataType;
}
SqlKind sqlKind = expr.getKind();
// See the comments below.
- // IgniteCustomType: at the moment we allow operations, involving
custom data types, that are binary comparison to fail at runtime.
if (!SqlKind.BINARY_COMPARISON.contains(sqlKind)) {
return dataType;
}
+
// Comparison and arithmetic operators are SqlCalls.
SqlCall sqlCall = (SqlCall) expr;
- // IgniteCustomType: We only handle binary operations here.
var lhs = getValidatedNodeType(sqlCall.operand(0));
var rhs = getValidatedNodeType(sqlCall.operand(1));
- if (lhs.getSqlTypeName() != SqlTypeName.ANY && rhs.getSqlTypeName() !=
SqlTypeName.ANY) {
- return dataType;
- }
-
// IgniteCustomType:
- //
- // The correct way to implement the validation error bellow would be
to move these coercion rules to SqlOperandTypeChecker:
- // 1) Implement the SqlOperandTypeChecker that prohibit arithmetic
operations
- // between types that neither support binary/unary operators nor
support type coercion.
- // 2) Specify that SqlOperandTypeChecker for every binary/unary
operator defined in
- // IgniteSqlOperatorTable
- //
- // This would allow to reject plans that contain type errors
- // at the validation stage.
- //
- // Similar approach can also be used to handle casts between types
that can not
- // be converted into one another.
- //
- // And if applied to dynamic parameters with combination of a modified
SqlOperandTypeChecker for
- // a cast function this would allow to reject plans with invalid
values w/o at validation stage.
- //
- var customTypeCoercionRules =
typeFactory().getCustomTypeCoercionRules();
- boolean canConvert;
-
- // IgniteCustomType: To enable implicit casts to a custom data type.
- if (SqlTypeUtil.equalSansNullability(typeFactory(), lhs, rhs)) {
- // We can always perform binary comparison operations between
instances of the same type.
- canConvert = true;
- } else if (lhs instanceof IgniteCustomType) {
- canConvert = customTypeCoercionRules.needToCast(rhs,
(IgniteCustomType) lhs);
- } else if (rhs instanceof IgniteCustomType) {
- canConvert = customTypeCoercionRules.needToCast(lhs,
(IgniteCustomType) rhs);
- } else {
- // We should not get here because at least one operand type must
be a IgniteCustomType
- // and only custom data types must use SqlTypeName::ANY.
- throw new AssertionError("At least one operand must be a custom
data type: " + expr);
- }
+ // Check compatibility for operands of binary comparison operation
between custom data types vs built-in SQL types.
+ // We get here because in calcite ANY type can be assigned/casted to
all other types.
+ // This check can be a part of some SqlOperandTypeChecker?
- if (!canConvert) {
- var ex = RESOURCE.invalidTypesForComparison(
- lhs.getFullTypeString(), sqlKind.sql,
rhs.getFullTypeString());
+ if (lhs instanceof IgniteCustomType || rhs instanceof
IgniteCustomType) {
+ boolean lhsRhsCompatible =
IgniteTypeSystem.INSTANCE.typeFamiliesAreCompatible(typeFactory, lhs, rhs);
+ boolean rhsLhsCompatible =
IgniteTypeSystem.INSTANCE.typeFamiliesAreCompatible(typeFactory, rhs, lhs);
Review Comment:
I believe the proper way to derive types is through type coercion
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
