lowka commented on code in PR #2857:
URL: https://github.com/apache/ignite-3/pull/2857#discussion_r1405881436
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/IgniteSqlValidator.java:
##########
@@ -772,91 +858,189 @@ private boolean isSystemFieldName(String alias) {
/** {@inheritDoc} */
@Override
protected void inferUnknownTypes(RelDataType inferredType,
SqlValidatorScope scope, SqlNode node) {
- if (node instanceof SqlDynamicParam) {
- RelDataType result = inferDynamicParamType(inferredType,
(SqlDynamicParam) node);
- this.setValidatedNodeType(node, result);
+ // See SqlStdOperatorTable::IS_NULL and
SqlStdOperatorTable::IS_NOT_NULL
+ // Operators use VARCHAR_1024 for argument type inference, so we need
to manually fix this.
+ if (node.getKind() == SqlKind.IS_NULL || node.getKind() ==
SqlKind.IS_NOT_NULL) {
+ SqlCall call = (SqlCall) node;
+
+ if (isUnspecifiedDynamicParam(call.operand(0))) {
+ SqlCallBinding binding = new SqlCallBinding(this, scope, call);
+ String signature = IgniteResource.makeSignature(binding,
List.of(unknownType));
+
+ throw
binding.newValidationError(IgniteResource.INSTANCE.ambiguousOperator1(signature));
+ }
+ }
+
+ if (node instanceof SqlDynamicParam) {
+ SqlDynamicParam dynamicParam = (SqlDynamicParam) node;
+ inferDynamicParamType(inferredType, dynamicParam);
} else {
super.inferUnknownTypes(inferredType, scope, node);
}
}
- private RelDataType inferDynamicParamType(RelDataType inferredType,
SqlDynamicParam dynamicParam) {
- RelDataType type = getDynamicParamType(dynamicParam);
- RelDataType paramTypeToUse;
+ private void inferDynamicParamType(RelDataType inferredType,
SqlDynamicParam dynamicParam) {
+ RelDataType type = deriveDynamicParamType(dynamicParam);
/*
- * If inferredType is unknown - use a type of dynamic parameter since
there is no other source of type information.
- *
- * If parameter's type and the inferredType do not match - use
parameter's type.
- * This makes CAST operations to work correctly. Otherwise cast's
operand is going to have
- * the same type as a target type which is not correct as it
- * makes every CAST operation eligible to redundant type conversion
elimination
- * at later stages:
- * E.g: CAST(? AS INTEGER) where ?='hello' operand is going to be
inferred as INTEGER
- * although it is a string.
+ * If inferredType type is unknown and parameter is not specified,
then use unknown type.
+ * Otherwise use parameter type if it set, or inferredType provided by
operator's
+ * SqlOperandTypeInference and SqlOperandTypeCheckers.
*
- * In other cases use the inferredType and we rely on type inference
rules provided by
- * operator's SqlOperandTypeInference and SqlOperandTypeCheckers.
+ * If dynamic parameter is an operand of a CAST expression, its type
is set to
+ * the type equal to the target type. Although CAST(p:T AS T) is a
no-op and later removed at the sql-to-rel conversion phase,
+ * the runtime _should_ perform type conversion of unspecified dynamic
parameter values to complete type checking.
*/
- if (inferredType.equals(unknownType) || (!equalSansNullability(type,
inferredType))) {
- paramTypeToUse = type;
+ if (inferredType == unknownType && type == unknownType) {
+ setDynamicParamType(dynamicParam, unknownType);
+ } else if (type != unknownType) {
+ setDynamicParamType(dynamicParam, type);
} else {
- paramTypeToUse = inferredType;
+ // Make sure to set nullability to true, so types are nullable in
all cases.
+ RelDataType nullableType =
typeFactory.createTypeWithNullability(inferredType, true);
+ setDynamicParamType(dynamicParam, nullableType);
}
+ }
- return typeFactory.createTypeWithNullability(paramTypeToUse, true);
+ /** Derives the type of the given dynamic parameter. */
+ private RelDataType deriveDynamicParamType(SqlDynamicParam dynamicParam) {
+ IgniteTypeFactory typeFactory = typeFactory();
+
+ if (isUnspecified(dynamicParam)) {
+ RelDataType validatedNodeType =
getValidatedNodeTypeIfKnown(dynamicParam);
+
+ if (validatedNodeType == null) {
+ setDynamicParamType(dynamicParam, unknownType);
+ return unknownType;
+ } else {
+ setDynamicParamType(dynamicParam, validatedNodeType);
+ return validatedNodeType;
+ }
+ } else {
+ Object value = dynamicParamValues[dynamicParam.getIndex()];
+
+ RelDataType parameterType;
+ // IgniteCustomType: first we must check whether dynamic parameter
is a custom data type.
+ // If so call createCustomType with appropriate arguments.
+ if (value instanceof UUID) {
+ parameterType = typeFactory.createCustomType(UuidType.NAME);
+ } else if (value == null) {
+ parameterType = typeFactory.createSqlType(SqlTypeName.NULL);
+ } else {
+ parameterType =
typeFactory.toSql(typeFactory.createType(value.getClass()));
+ }
+
+ // Dynamic parameters are always nullable.
+ // Otherwise it seem to cause "Conversion to relational algebra
failed to preserve datatypes" errors
+ // in some cases.
+ RelDataType nullableType =
typeFactory.createTypeWithNullability(parameterType, true);
+
+ setDynamicParamType(dynamicParam, nullableType);
+
+ return nullableType;
+ }
}
- /** Returns type of the given dynamic parameter. */
- RelDataType getDynamicParamType(SqlDynamicParam dynamicParam) {
- IgniteTypeFactory typeFactory = (IgniteTypeFactory)
this.getTypeFactory();
- Object value = getDynamicParamValue(dynamicParam);
+ /** if dynamic parameter is not specified, set its type to the provided
type, otherwise return the type of its value. */
+ RelDataType resolveDynamicParameterType(SqlDynamicParam dynamicParam,
RelDataType contextType) {
+ if (isUnspecified(dynamicParam)) {
+ RelDataType nullableContextType =
typeFactory.createTypeWithNullability(contextType, true);
+
+ setDynamicParamType(dynamicParam, nullableContextType);
- RelDataType parameterType;
- // IgniteCustomType: first we must check whether dynamic parameter is
a custom data type.
- // If so call createCustomType with appropriate arguments.
- if (value instanceof UUID) {
- parameterType = typeFactory.createCustomType(UuidType.NAME);
- } else if (value == null) {
- parameterType = typeFactory.createSqlType(SqlTypeName.NULL);
+ return nullableContextType;
} else {
- parameterType =
typeFactory.toSql(typeFactory.createType(value.getClass()));
+ return deriveDynamicParamType(dynamicParam);
}
+ }
+
+ private void setDynamicParamType(SqlDynamicParam dynamicParam, RelDataType
dataType) {
+ setValidatedNodeType(dynamicParam, dataType);
- // Dynamic parameters are always nullable.
- // Otherwise it seem to cause "Conversion to relational algebra failed
to preserve datatypes" errors
- // in some cases.
- return typeFactory.createTypeWithNullability(parameterType, true);
+ setDynamicParamResolvedType(dynamicParam, dataType);
}
+ /**
+ * Returns the value of the given dynamic parameter. If the value is not
specified,
+ * this method throws {@link IllegalArgumentException}.
+ */
private Object getDynamicParamValue(SqlDynamicParam dynamicParam) {
- Object value = dynamicParamValues[dynamicParam.getIndex()];
- // save dynamic parameter for later validation.
- dynamicParamNodes[dynamicParam.getIndex()] = dynamicParam;
+ int index = dynamicParam.getIndex();
+ Object value = dynamicParamValues[index];
+ if (value == Unspecified.UNKNOWN) {
+ throw new IllegalArgumentException(format("Value of dynamic
parameter#{} is not specified", index));
+ }
return value;
}
private void validateInferredDynamicParameters() {
- // Derived types of dynamic parameters should not change (current type
inference behavior).
-
for (int i = 0; i < dynamicParamValues.length; i++) {
- SqlDynamicParam param = dynamicParamNodes[i];
- assert param != null : format("Dynamic parameter#{} has not been
checked", i);
+ DynamicParamState paramState = parametersState[i];
+
+ if (paramState.resolvedType == null || paramState.node == null) {
+ throw new AssertionError("Dynamic parameter has not been
validated: " + i);
+ } else if (paramState.resolvedType == unknownType) {
+ throw newValidationError(paramState.node,
IgniteResource.INSTANCE.unableToResolveDynamicParameterType(i));
+ } else {
+ SqlDynamicParam dynamicParam = paramState.node;
+ RelDataType paramType = paramState.resolvedType;
+ RelDataType derivedType = getValidatedNodeType(dynamicParam);
+
+ // We can check for nullability, but it was set to true.
+ if (!SqlTypeUtil.equalSansNullability(derivedType, paramType))
{
+ String message = format(
+ "Type of dynamic parameter#{} does not match.
Expected: {} derived: {}", i, paramType.getFullTypeString(),
+ derivedType.getFullTypeString()
+ );
+
+ throw new AssertionError(message);
+ }
+ }
+ }
+ }
- RelDataType paramType = getDynamicParamType(param);
- RelDataType derivedType = getValidatedNodeType(param);
+ private void setDynamicParamResolvedType(SqlDynamicParam param,
RelDataType type) {
+ int index = param.getIndex();
+ DynamicParamState state = parametersState[index];
- // We can check for nullability, but it was set to true.
- if (!equalSansNullability(derivedType, paramType)) {
- String message = format(
- "Type of dynamic parameter#{} does not match.
Expected: {} derived: {}", i, paramType.getFullTypeString(),
- derivedType.getFullTypeString()
- );
+ state.node = param;
+ state.resolvedType = type;
+ }
- throw new AssertionError(message);
- }
+ /** Returns {@code true} if the given dynamic parameter has no value set.
*/
+ public boolean isUnspecified(SqlDynamicParam param) {
+ return dynamicParamValues[param.getIndex()] == Unspecified.UNKNOWN;
+ }
+
+ /** Returns {@code true} if the given node is dynamic parameter that has
no value set. */
+ public boolean isUnspecifiedDynamicParam(SqlNode node) {
+ if (node.getKind() != SqlKind.DYNAMIC_PARAM) {
+ return false;
+ } else {
+ return isUnspecified((SqlDynamicParam) node);
}
}
+
+ private static final class DynamicParamState {
+
+ SqlDynamicParam node;
+
+ /**
+ * Resolved type of a dynamic parameter.
+ *
+ * <ul>
+ * <li>{@code null} - parameter has not been checked - this is a
bug.</li>
+ * <li>{@code unknownType} means the type of this parameter has not
been resolved due to ambiguity.</li>
+ * <li>Otherwise contains a type of a parameter.</li>
+ * </ul>
+ */
+ RelDataType resolvedType;
+ }
+
+ /** Unspecified value. */
Review Comment:
A placeholder of unspecified value is debatable / maybe there should be a
better way to pass unspecified dynamic parameters.
--
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]
