From 480c442fad5c237f8f19fe5e0f2df338eba06ff3 Mon Sep 17 00:00:00 2001 From: Haozhun Jin Date: Mon, 1 Aug 2016 11:37:22 -0700 Subject: [PATCH] Reimplement SignatureBinder to support repeated literal variable --- .../presto/metadata/SignatureBinder.java | 787 ++++++++++++------ .../facebook/presto/type/TypeRegistry.java | 5 + .../presto/metadata/TestSignatureBinder.java | 260 +++++- 3 files changed, 765 insertions(+), 287 deletions(-) diff --git a/presto-main/src/main/java/com/facebook/presto/metadata/SignatureBinder.java b/presto-main/src/main/java/com/facebook/presto/metadata/SignatureBinder.java index 84d3d369b6d3..709aeb3f784c 100644 --- a/presto-main/src/main/java/com/facebook/presto/metadata/SignatureBinder.java +++ b/presto-main/src/main/java/com/facebook/presto/metadata/SignatureBinder.java @@ -15,28 +15,58 @@ import com.facebook.presto.spi.type.NamedTypeSignature; import com.facebook.presto.spi.type.ParameterKind; -import com.facebook.presto.spi.type.StandardTypes; import com.facebook.presto.spi.type.Type; import com.facebook.presto.spi.type.TypeManager; import com.facebook.presto.spi.type.TypeSignature; import com.facebook.presto.spi.type.TypeSignatureParameter; -import com.facebook.presto.type.UnknownType; +import com.google.common.base.VerifyException; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.Set; import static com.facebook.presto.type.TypeCalculation.calculateLiteralValue; +import static com.facebook.presto.type.TypeRegistry.isCovariantTypeBase; +import static com.facebook.presto.type.UnknownType.UNKNOWN; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; -import static com.google.common.collect.Iterables.getLast; +import static com.google.common.base.Verify.verify; +import static java.lang.String.format; import static java.util.Objects.requireNonNull; +import static java.util.function.Function.identity; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toMap; +/** + * Determines whether, and how, a callsite matches a generic function signature. + * Which is equivalent to finding assignments for the variables in the generic signature, + * such that all of the function's declared parameters are super types of the corresponding + * arguments, and also satisfy the declared constraints (such as a given type parameter must + * bind to an orderable type) + *

+ * This implementation has made assumptions. When any of the assumptions is not satisfied, it will fail loudly. + *

+ * Here are some known implementation limitations: + *

+ */ public class SignatureBinder { + // 4 is chosen arbitrarily here. This limit is set to avoid having infinite loops in iterative solving. + private static final int SOLVE_ITERATION_LIMIT = 4; + private final TypeManager typeManager; private final Signature declaredSignature; private final boolean allowCoercion; @@ -44,12 +74,12 @@ public class SignatureBinder public SignatureBinder(TypeManager typeManager, Signature declaredSignature, boolean allowCoercion) { + checkNoLiteralVariableUsageAcrossTypes(declaredSignature); this.typeManager = requireNonNull(typeManager, "typeManager is null"); this.declaredSignature = requireNonNull(declaredSignature, "parametrizedSignature is null"); this.allowCoercion = allowCoercion; - this.typeVariableConstraints = declaredSignature.getTypeVariableConstraints() - .stream() - .collect(toMap(TypeVariableConstraint::getName, t -> t)); + this.typeVariableConstraints = declaredSignature.getTypeVariableConstraints().stream() + .collect(toMap(TypeVariableConstraint::getName, identity())); } public Optional bind(List actualArgumentTypes) @@ -72,242 +102,305 @@ public Optional bind(List actualArgumentTypes, Type a public Optional bindVariables(List actualArgumentTypes) { - List expectedArgumentSignatures = declaredSignature.getArgumentTypes(); - boolean variableArity = declaredSignature.isVariableArity(); - BoundVariables.Builder variableBinder = BoundVariables.builder(); - if (!matchArguments(expectedArgumentSignatures, actualArgumentTypes, variableArity, variableBinder)) { + ImmutableList.Builder constraintSolvers = ImmutableList.builder(); + if (!appendConstraintSolversForArguments(constraintSolvers, actualArgumentTypes)) { return Optional.empty(); } - calculateVariableValuesForLongConstraints(variableBinder); - BoundVariables boundVariables = variableBinder.build(); - if (!allTypeVariablesBound(boundVariables)) { - return Optional.empty(); - } - return Optional.of(boundVariables); + + return iterativeSolve(constraintSolvers.build()); } public Optional bindVariables(List actualArgumentTypes, Type actualReturnType) { - BoundVariables.Builder variableBinder = BoundVariables.builder(); - - TypeSignature expectedReturnTypeSignature = declaredSignature.getReturnType(); - if (!bind(expectedReturnTypeSignature, actualReturnType, variableBinder)) { + ImmutableList.Builder constraintSolvers = ImmutableList.builder(); + if (!appendConstraintSolversForReturnValue(constraintSolvers, actualReturnType)) { return Optional.empty(); } - - List expectedArgumentSignatures = declaredSignature.getArgumentTypes(); - boolean variableArity = declaredSignature.isVariableArity(); - if (!matchArguments(expectedArgumentSignatures, actualArgumentTypes, variableArity, variableBinder)) { + if (!appendConstraintSolversForArguments(constraintSolvers, actualArgumentTypes)) { return Optional.empty(); } - calculateVariableValuesForLongConstraints(variableBinder); - BoundVariables boundVariables = variableBinder.build(); - if (!allTypeVariablesBound(boundVariables)) { - return Optional.empty(); + return iterativeSolve(constraintSolvers.build()); + } + + public static Signature applyBoundVariables(Signature signature, BoundVariables boundVariables, int arity) + { + List argumentSignatures; + if (signature.isVariableArity()) { + argumentSignatures = expandVarargFormalTypeSignature(signature.getArgumentTypes(), arity); } - return Optional.of(boundVariables); + else { + checkArgument(signature.getArgumentTypes().size() == arity); + argumentSignatures = signature.getArgumentTypes(); + } + List boundArgumentSignatures = applyBoundVariables(argumentSignatures, boundVariables); + TypeSignature boundReturnTypeSignature = applyBoundVariables(signature.getReturnType(), boundVariables); + + return new Signature( + signature.getName(), + signature.getKind(), + ImmutableList.of(), + ImmutableList.of(), + boundReturnTypeSignature, + boundArgumentSignatures, + false); } - private boolean matchArguments( - List expectedTypes, - List actualTypes, - boolean variableArity, - BoundVariables.Builder variableBinder) + public static List applyBoundVariables(List typeSignatures, BoundVariables boundVariables) { - if (variableArity && (actualTypes.size() < (expectedTypes.size() - 1))) { - return false; + ImmutableList.Builder builder = ImmutableList.builder(); + for (TypeSignature typeSignature : typeSignatures) { + builder.add(applyBoundVariables(typeSignature, boundVariables)); } + return builder.build(); + } - if (!variableArity && expectedTypes.size() != actualTypes.size()) { - return false; + public static TypeSignature applyBoundVariables(TypeSignature typeSignature, BoundVariables boundVariables) + { + String baseType = typeSignature.getBase(); + if (boundVariables.containsTypeVariable(baseType)) { + checkState(typeSignature.getParameters().isEmpty(), "Type parameters cannot have parameters"); + return boundVariables.getTypeVariable(baseType).getTypeSignature(); } - // Bind the variable arity argument first, to make sure it's bound to the common super type - if (variableArity && actualTypes.size() >= expectedTypes.size()) { - List tail = actualTypes.subList(expectedTypes.size() - 1, actualTypes.size()); - Optional commonType = typeManager.getCommonSuperType(tail); - if (!commonType.isPresent()) { - return false; - } - TypeSignature expected = expectedTypes.get(expectedTypes.size() - 1); - if (!bind(expected, commonType.get(), variableBinder)) { - return false; + List parameters = typeSignature.getParameters().stream() + .map(typeSignatureParameter -> applyBoundVariables(typeSignatureParameter, boundVariables)) + .collect(toList()); + + return new TypeSignature(baseType, parameters); + } + + /** + * Example of not allowed literal variable usages across typeSignatures: + *

    + *
  • x used in different base types: char(x) and varchar(x) + *
  • x used in different positions of the same base type: decimal(x,y) and decimal(z,x) + *
  • p used in combination with different literals, types, or literal variables: decimal(p,s1) and decimal(p,s2) + *
+ */ + private static void checkNoLiteralVariableUsageAcrossTypes(Signature declaredSignature) + { + Map existingUsages = new HashMap<>(); + for (TypeSignature parameter : declaredSignature.getArgumentTypes()) { + checkNoLiteralVariableUsageAcrossTypes(parameter, existingUsages); + } + } + + private static void checkNoLiteralVariableUsageAcrossTypes(TypeSignature typeSignature, Map existingUsages) + { + List variables = typeSignature.getParameters().stream() + .filter(TypeSignatureParameter::isVariable) + .collect(toList()); + for (TypeSignatureParameter variable : variables) { + TypeSignature existing = existingUsages.get(variable.getVariable()); + if (existing != null && !existing.equals(typeSignature)) { + throw new UnsupportedOperationException("Literal parameters may not be shared across different types"); } + existingUsages.put(variable.getVariable(), typeSignature); } - for (int i = 0; i < actualTypes.size(); i++) { - // Get the current argument signature, or the last one, if this is a varargs function - TypeSignature expectedArgumentSignature = expectedTypes.get(Math.min(i, expectedTypes.size() - 1)); - Type actualArgumentType = actualTypes.get(i); - if (!bind(expectedArgumentSignature, actualArgumentType, variableBinder)) { - return false; + for (TypeSignatureParameter parameter : typeSignature.getParameters()) { + if (parameter.isLongLiteral() || parameter.isVariable()) { + continue; } + checkNoLiteralVariableUsageAcrossTypes(parameter.getTypeSignatureOrNamedTypeSignature().get(), existingUsages); } + } - return true; + private boolean appendConstraintSolversForReturnValue(ImmutableList.Builder resultBuilder, Type actualReturnType) + { + TypeSignature formalReturnTypeSignature = declaredSignature.getReturnType(); + appendTypeRelationshipConstraintSolver(resultBuilder, formalReturnTypeSignature, actualReturnType, false); + return appendConstraintSolvers(resultBuilder, formalReturnTypeSignature, actualReturnType, false); } - private boolean bind( - TypeSignature expectedSignature, - Type actualType, - BoundVariables.Builder variableBinder) + private boolean appendConstraintSolversForArguments(ImmutableList.Builder resultBuilder, List actualTypes) { - if (isTypeVariable(expectedSignature)) { - String variable = expectedSignature.getBase(); - return matchAndBindTypeVariable(variable, actualType, variableBinder); + boolean variableArity = declaredSignature.isVariableArity(); + List formalTypeSignatures = declaredSignature.getArgumentTypes(); + if (variableArity) { + if (actualTypes.size() < formalTypeSignatures.size() - 1) { + return false; + } + formalTypeSignatures = expandVarargFormalTypeSignature(formalTypeSignatures, actualTypes.size()); } - // TODO: Refactor VARCHAR function to accept parametrized VARCHAR(X) instead of VARCHAR and remove this hack - if (expectedSignature.getBase().equals(StandardTypes.VARCHAR) && expectedSignature.getParameters().isEmpty()) { - return actualType.getTypeSignature().getBase().equals(StandardTypes.VARCHAR) || - (allowCoercion && typeManager.coerceTypeBase(actualType, StandardTypes.VARCHAR).isPresent()); + if (formalTypeSignatures.size() != actualTypes.size()) { + return false; } - // If the expected signature is a signature of the concrete type no variables are need to be bound - // For such case it is enough to just check whether the actual type is coercible to the expected - if (isConcreteType(expectedSignature)) { - Type expectedType = typeManager.getType(expectedSignature); - if (allowCoercion) { - return typeManager.canCoerce(actualType, expectedType); - } - else { - return actualType.equals(expectedType); - } + for (int i = 0; i < formalTypeSignatures.size(); i++) { + appendTypeRelationshipConstraintSolver(resultBuilder, formalTypeSignatures.get(i), actualTypes.get(i), allowCoercion); } - String actualTypeBase = actualType.getTypeSignature().getBase(); - String expectedTypeBase = expectedSignature.getBase(); - if (actualTypeBase.equals(expectedTypeBase)) { - return matchAndBindTypeParameters(expectedSignature, actualType, variableBinder); + return appendConstraintSolvers(resultBuilder, formalTypeSignatures, actualTypes, allowCoercion); + } + + private boolean appendConstraintSolvers( + ImmutableList.Builder resultBuilder, + List formalTypeSignatures, + List actualTypes, + boolean allowCoercion) + { + if (formalTypeSignatures.size() != actualTypes.size()) { + return false; } - else if (allowCoercion) { - Optional coercionResult = typeManager.coerceTypeBase(actualType, expectedTypeBase); - if (coercionResult.isPresent()) { - return matchAndBindTypeParameters(expectedSignature, coercionResult.get(), variableBinder); - } - // UNKNOWN matches to all the types, but based on the UNKNOWN type parameters can't be bound - if (actualType.equals(UnknownType.UNKNOWN)) { - return true; + for (int i = 0; i < formalTypeSignatures.size(); i++) { + if (!appendConstraintSolvers(resultBuilder, formalTypeSignatures.get(i), actualTypes.get(i), allowCoercion)) { + return false; } } - - return false; + return true; } - private boolean matchAndBindTypeVariable( - String variable, + private boolean appendConstraintSolvers( + ImmutableList.Builder resultBuilder, + TypeSignature formalTypeSignature, Type actualType, - BoundVariables.Builder variableBinder) + boolean allowCoercion) { - TypeVariableConstraint typeVariableConstraint = typeVariableConstraints.get(variable); - if (!variableBinder.containsTypeVariable(variable)) { - if (typeVariableConstraint.canBind(actualType)) { - variableBinder.setTypeVariable(variable, actualType); + // formalTypeSignature can be categorized into one of the 4 cases below: + // * type without type parameter + // * type parameter of type/named_type kind + // * type with type parameter of literal/variable kind + // * type with type parameter of type/named_type kind + + if (formalTypeSignature.getParameters().isEmpty()) { + TypeVariableConstraint typeVariableConstraint = typeVariableConstraints.get(formalTypeSignature.getBase()); + if (typeVariableConstraint == null) { return true; } + resultBuilder.add(new TypeParameterSolver( + formalTypeSignature.getBase(), + actualType, + typeVariableConstraint.isComparableRequired(), + typeVariableConstraint.isOrderableRequired(), + Optional.ofNullable(typeVariableConstraint.getVariadicBound()))); + return true; + } - // UNKNOWN matches to all the types, but based on UNKNOWN we can't determine the actual type parameters - if (allowCoercion && actualType.equals(UnknownType.UNKNOWN)) { - return true; - } + if (isTypeWithLiteralParameters(formalTypeSignature)) { + resultBuilder.add(new TypeWithLiteralParametersSolver(formalTypeSignature, actualType)); + return true; + } - // TODO Refactor type registry and support coercion to variadic bound fully + List actualTypeTypeParameters; + if (UNKNOWN.equals(actualType)) { + actualTypeTypeParameters = Collections.nCopies(formalTypeSignature.getParameters().size(), UNKNOWN); } - // type variable is already bound. just check that bound types are compatible. else { - Type currentBoundType = variableBinder.getTypeVariable(variable); - if (currentBoundType.equals(actualType)) { - return true; - } - if (allowCoercion) { - Optional commonSuperType = typeManager.getCommonSuperType(currentBoundType, actualType); - if (commonSuperType.isPresent() && typeVariableConstraint.canBind(commonSuperType.get())) { - variableBinder.setTypeVariable(variable, commonSuperType.get()); - return true; - } + actualTypeTypeParameters = actualType.getTypeParameters(); + } + + ImmutableList.Builder formalTypeParameterTypeSignatures = ImmutableList.builder(); + for (TypeSignatureParameter formalTypeSignatureParameter : formalTypeSignature.getParameters()) { + Optional typeSignature = formalTypeSignatureParameter.getTypeSignatureOrNamedTypeSignature(); + if (!typeSignature.isPresent()) { + throw new UnsupportedOperationException("Types with both type parameters and literal parameters at the same time are not supported"); } + formalTypeParameterTypeSignatures.add(typeSignature.get()); } - return false; + + return appendConstraintSolvers( + resultBuilder, + formalTypeParameterTypeSignatures.build(), + actualTypeTypeParameters, + allowCoercion && isCovariantTypeBase(formalTypeSignature.getBase())); } - private boolean matchAndBindTypeParameters( - TypeSignature expectedArgumentSignature, - Type actualArgumentType, - BoundVariables.Builder variableBinder) + private Set typeVariablesOf(TypeSignature typeSignature) { - TypeSignature actualArgumentSignature = actualArgumentType.getTypeSignature(); - checkState(expectedArgumentSignature.getBase().equals(actualArgumentSignature.getBase()), "equal base types are expected here"); - - List expectedTypeParameters = expectedArgumentSignature.getParameters(); - List actualTypeParameters = actualArgumentSignature.getParameters(); - - if (expectedTypeParameters.size() != actualTypeParameters.size()) { - return false; + if (typeVariableConstraints.containsKey(typeSignature.getBase())) { + return ImmutableSet.of(typeSignature.getBase()); } - - for (int typeParameterIndex = 0; typeParameterIndex < expectedTypeParameters.size(); typeParameterIndex++) { - TypeSignatureParameter expectedTypeParameter = expectedTypeParameters.get(typeParameterIndex); - TypeSignatureParameter actualTypeParameter = actualTypeParameters.get(typeParameterIndex); - if (!matchAndBindTypeParameter(expectedTypeParameter, actualTypeParameter, variableBinder)) { - return false; + Set variables = new HashSet<>(); + for (TypeSignatureParameter parameter : typeSignature.getParameters()) { + switch (parameter.getKind()) { + case TYPE: + variables.addAll(typeVariablesOf(parameter.getTypeSignature())); + break; + case NAMED_TYPE: + variables.addAll(typeVariablesOf(parameter.getNamedTypeSignature().getTypeSignature())); + break; + case LONG: + break; + case VARIABLE: + break; + default: + throw new UnsupportedOperationException(); } } - return true; + return variables; } - private boolean matchAndBindTypeParameter(TypeSignatureParameter expected, TypeSignatureParameter actual, BoundVariables.Builder variableBinder) + private static Set longVariablesOf(TypeSignature typeSignature) { - switch (expected.getKind()) { - case VARIABLE: { - String variable = expected.getVariable(); - checkState(actual.getKind() == ParameterKind.LONG, - "LONG parameter kind is expected here"); - Long variableValue = actual.getLongLiteral(); - return matchAndBindLongVariable(variable, variableValue, variableBinder); - } - case LONG: { - checkState(actual.getKind() == ParameterKind.LONG, - "LONG parameter kind is expected here"); - return actual.getLongLiteral().equals(expected.getLongLiteral()); - } - case TYPE: { - checkState(actual.getKind() == ParameterKind.TYPE, - "TYPE parameter kind is expected here"); - TypeSignature expectedTypeSignature = expected.getTypeSignature(); - TypeSignature actualTypeSignature = actual.getTypeSignature(); - Type actualType = typeManager.getType(actualTypeSignature); - return bind(expectedTypeSignature, actualType, variableBinder); - } - case NAMED_TYPE: { - checkState(actual.getKind() == ParameterKind.NAMED_TYPE, - "NAMED_TYPE parameter kind is expected here"); - NamedTypeSignature expectedNamedTypeSignature = expected.getNamedTypeSignature(); - NamedTypeSignature actualNamedTypeSignature = actual.getNamedTypeSignature(); - if (!expectedNamedTypeSignature.getName().equals(actualNamedTypeSignature.getName())) { - return false; - } - TypeSignature expectedTypeSignature = expectedNamedTypeSignature.getTypeSignature(); - TypeSignature actualTypeSignature = actualNamedTypeSignature.getTypeSignature(); - Type actualType = typeManager.getType(actualTypeSignature); - return bind(expectedTypeSignature, actualType, variableBinder); + Set variables = new HashSet<>(); + for (TypeSignatureParameter parameter : typeSignature.getParameters()) { + switch (parameter.getKind()) { + case TYPE: + variables.addAll(longVariablesOf(parameter.getTypeSignature())); + break; + case NAMED_TYPE: + variables.addAll(longVariablesOf(parameter.getNamedTypeSignature().getTypeSignature())); + break; + case LONG: + break; + case VARIABLE: + variables.add(parameter.getVariable()); + break; + default: + throw new UnsupportedOperationException(); } - default: - throw new IllegalStateException("Unknown parameter kind: " + expected.getKind()); } + + return variables; + } + + private static boolean isTypeWithLiteralParameters(TypeSignature typeSignature) + { + return typeSignature.getParameters().stream() + .map(TypeSignatureParameter::getKind) + .allMatch(kind -> kind == ParameterKind.LONG || kind == ParameterKind.VARIABLE); } - private boolean matchAndBindLongVariable(String variable, Long value, BoundVariables.Builder variableBinder) + private Optional iterativeSolve(List constraints) { - if (variableBinder.containsLongVariable(variable)) { - Long currentVariableValue = variableBinder.getLongVariable(variable); - return value.equals(currentVariableValue); + BoundVariables.Builder boundVariablesBuilder = BoundVariables.builder(); + for (int i = 0; true; i++) { + if (i == SOLVE_ITERATION_LIMIT) { + throw new VerifyException(format("SignatureBinder.iterativeSolve does not converge after %d iterations.", SOLVE_ITERATION_LIMIT)); + } + SolverReturnStatusMerger statusMerger = new SolverReturnStatusMerger(); + for (TypeConstraintSolver constraint : constraints) { + statusMerger.add(constraint.update(boundVariablesBuilder)); + if (statusMerger.getCurrent() == SolverReturnStatus.UNSOLVABLE) { + return Optional.empty(); + } + } + switch (statusMerger.getCurrent()) { + case UNCHANGED_SATISFIED: + break; + case UNCHANGED_NOT_SATISFIED: + return Optional.empty(); + case CHANGED: + continue; + case UNSOLVABLE: + throw new VerifyException(); + default: + throw new UnsupportedOperationException("unknown status"); + } + break; } - else { - variableBinder.setLongVariable(variable, value); - return true; + + calculateVariableValuesForLongConstraints(boundVariablesBuilder); + + BoundVariables boundVariables = boundVariablesBuilder.build(); + if (!allTypeVariablesBound(boundVariables)) { + return Optional.empty(); } + return Optional.of(boundVariables); } private void calculateVariableValuesForLongConstraints(BoundVariables.Builder variableBinder) @@ -325,133 +418,291 @@ private void calculateVariableValuesForLongConstraints(BoundVariables.Builder va } } - private boolean isTypeVariable(TypeSignature signature) + private boolean allTypeVariablesBound(BoundVariables boundVariables) { - if (typeVariableConstraints.containsKey(signature.getBase())) { - checkState(signature.getParameters().isEmpty(), - "TypeSignature that represent type variable shouldn't be parametrized"); - return true; - } - return false; + return boundVariables.getTypeVariables().keySet().equals(typeVariableConstraints.keySet()); } - private boolean isConcreteType(TypeSignature typeSignature) + private static TypeSignatureParameter applyBoundVariables(TypeSignatureParameter parameter, BoundVariables boundVariables) { - if (isTypeVariable(typeSignature)) { - return false; - } - for (TypeSignatureParameter typeSignatureParameter : typeSignature.getParameters()) { - switch (typeSignatureParameter.getKind()) { - case LONG: - continue; - case VARIABLE: - return false; - case TYPE: { - if (!isConcreteType(typeSignatureParameter.getTypeSignature())) { - return false; - } - continue; - } - case NAMED_TYPE: { - if (!isConcreteType(typeSignatureParameter.getNamedTypeSignature().getTypeSignature())) { - return false; - } - continue; - } - default: - throw new UnsupportedOperationException("Unsupported TypeSignatureParameter kind: " + typeSignatureParameter.getKind()); + ParameterKind parameterKind = parameter.getKind(); + switch (parameterKind) { + case TYPE: { + TypeSignature typeSignature = parameter.getTypeSignature(); + return TypeSignatureParameter.of(applyBoundVariables(typeSignature, boundVariables)); + } + case NAMED_TYPE: { + NamedTypeSignature namedTypeSignature = parameter.getNamedTypeSignature(); + TypeSignature typeSignature = namedTypeSignature.getTypeSignature(); + return TypeSignatureParameter.of(new NamedTypeSignature( + namedTypeSignature.getName(), + applyBoundVariables(typeSignature, boundVariables))); + } + case VARIABLE: { + String variableName = parameter.getVariable(); + checkState(boundVariables.containsLongVariable(variableName), + "Variable is not bound: %s", variableName); + Long variableValue = boundVariables.getLongVariable(variableName); + return TypeSignatureParameter.of(variableValue); + } + case LONG: { + return parameter; } + default: + throw new IllegalStateException("Unknown parameter kind: " + parameter.getKind()); } - return true; } - private boolean allTypeVariablesBound(BoundVariables boundVariables) + private static List expandVarargFormalTypeSignature(List formalTypeSignatures, int actualArity) { - return boundVariables.getTypeVariables().keySet().equals(typeVariableConstraints.keySet()); + int variableArityArgumentsCount = actualArity - formalTypeSignatures.size() + 1; + if (variableArityArgumentsCount == 0) { + return formalTypeSignatures.subList(0, formalTypeSignatures.size() - 1); + } + if (variableArityArgumentsCount == 1) { + return formalTypeSignatures; + } + checkArgument(variableArityArgumentsCount > 1 && !formalTypeSignatures.isEmpty()); + + ImmutableList.Builder builder = ImmutableList.builder(); + builder.addAll(formalTypeSignatures); + TypeSignature lastTypeSignature = formalTypeSignatures.get(formalTypeSignatures.size() - 1); + for (int i = 1; i < variableArityArgumentsCount; i++) { + builder.add(lastTypeSignature); + } + return builder.build(); } - public static Signature applyBoundVariables(Signature signature, BoundVariables boundVariables, int arity) + private interface TypeConstraintSolver { - List argumentSignatures = fillInMissingVariableArguments(signature.getArgumentTypes(), arity); - List boundArgumentSignatures = applyBoundVariables(argumentSignatures, boundVariables); - TypeSignature boundReturnTypeSignature = applyBoundVariables(signature.getReturnType(), boundVariables); + SolverReturnStatus update(BoundVariables.Builder bindings); + } - return new Signature( - signature.getName(), - signature.getKind(), - ImmutableList.of(), - ImmutableList.of(), - boundReturnTypeSignature, - boundArgumentSignatures, - false - ); + private enum SolverReturnStatus + { + UNCHANGED_SATISFIED, + UNCHANGED_NOT_SATISFIED, + CHANGED, + UNSOLVABLE, } - private static List fillInMissingVariableArguments(List argumentSignatures, int arity) + private class SolverReturnStatusMerger { - int variableArityArgumentsCount = arity - argumentSignatures.size(); - if (variableArityArgumentsCount > 0 && !argumentSignatures.isEmpty()) { - ImmutableList.Builder builder = ImmutableList.builder(); - builder.addAll(argumentSignatures); - for (int i = 0; i < variableArityArgumentsCount; i++) { - builder.add(getLast(argumentSignatures)); + // This class gives the overall status when multiple status are seen from different parts. + // The logic is simple and can be summarized as finding the right most item (based on the list below) seen so far: + // UNCHANGED_SATISFIED, UNCHANGED_NOT_SATISFIED, CHANGED, UNSOLVABLE + // If no item was seen ever, it provides UNCHANGED_SATISFIED. + + private SolverReturnStatus current = SolverReturnStatus.UNCHANGED_SATISFIED; + + public void add(SolverReturnStatus newStatus) + { + switch (newStatus) { + case UNCHANGED_SATISFIED: + break; + case UNCHANGED_NOT_SATISFIED: + if (current == SolverReturnStatus.UNCHANGED_SATISFIED) { + current = SolverReturnStatus.UNCHANGED_NOT_SATISFIED; + } + break; + case CHANGED: + if (current == SolverReturnStatus.UNCHANGED_SATISFIED || current == SolverReturnStatus.UNCHANGED_NOT_SATISFIED) { + current = SolverReturnStatus.CHANGED; + } + break; + case UNSOLVABLE: + current = SolverReturnStatus.UNSOLVABLE; + break; } - return builder.build(); } - return argumentSignatures; + + public SolverReturnStatus getCurrent() + { + return current; + } } - public static List applyBoundVariables(List typeSignatures, BoundVariables boundVariables) + private class TypeParameterSolver + implements TypeConstraintSolver { - ImmutableList.Builder builder = ImmutableList.builder(); - for (TypeSignature typeSignature : typeSignatures) { - builder.add(applyBoundVariables(typeSignature, boundVariables)); + private final String typeParameter; + private final Type actualType; + private final boolean comparableRequired; + private final boolean orderableRequired; + private final Optional requiredBaseName; + + public TypeParameterSolver(String typeParameter, Type actualType, boolean comparableRequired, boolean orderableRequired, Optional requiredBaseName) + { + this.typeParameter = typeParameter; + this.actualType = actualType; + this.comparableRequired = comparableRequired; + this.orderableRequired = orderableRequired; + this.requiredBaseName = requiredBaseName; + } + + @Override + public SolverReturnStatus update(BoundVariables.Builder bindings) + { + if (!bindings.containsTypeVariable(typeParameter)) { + if (!satisfiesConstraints(actualType)) { + return SolverReturnStatus.UNSOLVABLE; + } + bindings.setTypeVariable(typeParameter, actualType); + return SolverReturnStatus.CHANGED; + } + Type originalType = bindings.getTypeVariable(typeParameter); + Optional commonSuperType = typeManager.getCommonSuperType(originalType, actualType); + if (!commonSuperType.isPresent()) { + return SolverReturnStatus.UNSOLVABLE; + } + if (!satisfiesConstraints(commonSuperType.get())) { + // This check must not be skipped even if commonSuperType is equal to originalType + return SolverReturnStatus.UNSOLVABLE; + } + if (commonSuperType.get().equals(originalType)) { + return SolverReturnStatus.UNCHANGED_SATISFIED; + } + bindings.setTypeVariable(typeParameter, commonSuperType.get()); + return SolverReturnStatus.CHANGED; + } + + private boolean satisfiesConstraints(Type type) + { + if (comparableRequired && !type.isComparable()) { + return false; + } + if (orderableRequired && !type.isOrderable()) { + return false; + } + if (requiredBaseName.isPresent() && !UNKNOWN.equals(type) && !requiredBaseName.get().equals(type.getTypeSignature().getBase())) { + // TODO: the case below should be properly handled: + // * `type` does not have the `requiredBaseName` but can be coerced to some type that has the `requiredBaseName`. + return false; + } + return true; } - return builder.build(); } - public static TypeSignature applyBoundVariables(TypeSignature typeSignature, BoundVariables boundVariables) + private class TypeWithLiteralParametersSolver + implements TypeConstraintSolver { - String baseType = typeSignature.getBase(); - if (boundVariables.containsTypeVariable(baseType)) { - checkState(typeSignature.getParameters().isEmpty(), "Type parameters cannot have parameters"); - return boundVariables.getTypeVariable(baseType).getTypeSignature(); + private final TypeSignature formalTypeSignature; + private final Type actualType; + + public TypeWithLiteralParametersSolver(TypeSignature formalTypeSignature, Type actualType) + { + this.formalTypeSignature = formalTypeSignature; + this.actualType = actualType; } - List parameters = typeSignature.getParameters().stream() - .map(typeSignatureParameter -> applyBoundVariables(typeSignatureParameter, boundVariables)) - .collect(toList()); + @Override + public SolverReturnStatus update(BoundVariables.Builder bindings) + { + ImmutableList.Builder originalTypeTypeParametersBuilder = ImmutableList.builder(); + List parameters = formalTypeSignature.getParameters(); + for (int i = 0; i < parameters.size(); i++) { + TypeSignatureParameter typeSignatureParameter = parameters.get(i); + if (typeSignatureParameter.getKind() == ParameterKind.VARIABLE) { + if (bindings.containsLongVariable(typeSignatureParameter.getVariable())) { + originalTypeTypeParametersBuilder.add(TypeSignatureParameter.of(bindings.getLongVariable(typeSignatureParameter.getVariable()))); + } + else { + // if an existing value doesn't exist for the given variable name, use the value that comes from the actual type. + Optional type = typeManager.coerceTypeBase(actualType, formalTypeSignature.getBase()); + if (!type.isPresent()) { + return SolverReturnStatus.UNSOLVABLE; + } + TypeSignature typeSignature = type.get().getTypeSignature(); + originalTypeTypeParametersBuilder.add(TypeSignatureParameter.of(typeSignature.getParameters().get(i).getLongLiteral())); + } + } + else { + verify(typeSignatureParameter.getKind() == ParameterKind.LONG); + originalTypeTypeParametersBuilder.add(typeSignatureParameter); + } + } + Type originalType = typeManager.getType(new TypeSignature(formalTypeSignature.getBase(), originalTypeTypeParametersBuilder.build())); + Optional commonSuperType = typeManager.getCommonSuperType(originalType, actualType); + if (!commonSuperType.isPresent()) { + return SolverReturnStatus.UNSOLVABLE; + } + TypeSignature commonSuperTypeSignature = commonSuperType.get().getTypeSignature(); + if (!commonSuperTypeSignature.getBase().equals(formalTypeSignature.getBase())) { + return SolverReturnStatus.UNSOLVABLE; + } + SolverReturnStatus result = SolverReturnStatus.UNCHANGED_SATISFIED; + for (int i = 0; i < parameters.size(); i++) { + TypeSignatureParameter typeSignatureParameter = parameters.get(i); + long commonSuperLongLiteral = commonSuperTypeSignature.getParameters().get(i).getLongLiteral(); + if (typeSignatureParameter.getKind() == ParameterKind.VARIABLE) { + String variableName = typeSignatureParameter.getVariable(); + if (!bindings.containsLongVariable(variableName) || !bindings.getLongVariable(variableName).equals(commonSuperLongLiteral)) { + bindings.setLongVariable(variableName, commonSuperLongLiteral); + result = SolverReturnStatus.CHANGED; + } + } + else { + verify(typeSignatureParameter.getKind() == ParameterKind.LONG); + if (commonSuperLongLiteral != typeSignatureParameter.getLongLiteral()) { + return SolverReturnStatus.UNSOLVABLE; + } + } + } + return result; + } + } - return new TypeSignature(baseType, parameters); + private void appendTypeRelationshipConstraintSolver( + ImmutableList.Builder resultBuilder, + TypeSignature formalTypeSignature, + Type actualType, + boolean allowCoercion) + { + Set typeVariables = typeVariablesOf(formalTypeSignature); + Set longVariables = longVariablesOf(formalTypeSignature); + resultBuilder.add(new TypeRelationshipConstraintSolver(formalTypeSignature, typeVariables, longVariables, actualType, allowCoercion)); } - private static TypeSignatureParameter applyBoundVariables(TypeSignatureParameter parameter, BoundVariables boundVariables) + private class TypeRelationshipConstraintSolver + implements TypeConstraintSolver { - ParameterKind parameterKind = parameter.getKind(); - switch (parameterKind) { - case TYPE: { - TypeSignature typeSignature = parameter.getTypeSignature(); - return TypeSignatureParameter.of(applyBoundVariables(typeSignature, boundVariables)); + private final TypeSignature superTypeSignature; + private final Set typeVariables; + private final Set longVariables; + private final Type actualType; + private final boolean allowCoercion; + + public TypeRelationshipConstraintSolver(TypeSignature superTypeSignature, Set typeVariables, Set longVariables, Type actualType, boolean allowCoercion) + { + this.superTypeSignature = superTypeSignature; + this.typeVariables = typeVariables; + this.longVariables = longVariables; + this.actualType = actualType; + this.allowCoercion = allowCoercion; + } + + @Override + public SolverReturnStatus update(BoundVariables.Builder bindings) + { + for (String variable : typeVariables) { + if (!bindings.containsTypeVariable(variable)) { + return SolverReturnStatus.UNCHANGED_NOT_SATISFIED; + } } - case NAMED_TYPE: { - NamedTypeSignature namedTypeSignature = parameter.getNamedTypeSignature(); - TypeSignature typeSignature = namedTypeSignature.getTypeSignature(); - return TypeSignatureParameter.of(new NamedTypeSignature( - namedTypeSignature.getName(), - applyBoundVariables(typeSignature, boundVariables))); + for (String variable : longVariables) { + if (!bindings.containsLongVariable(variable)) { + return SolverReturnStatus.UNCHANGED_NOT_SATISFIED; + } } - case VARIABLE: { - String variableName = parameter.getVariable(); - checkState(boundVariables.containsLongVariable(variableName), - "Variable is not bound: %s", variableName); - Long variableValue = boundVariables.getLongVariable(variableName); - return TypeSignatureParameter.of(variableValue); + + TypeSignature boundSignature = applyBoundVariables(superTypeSignature, bindings.build()); + + if (allowCoercion) { + return typeManager.canCoerce(actualType, typeManager.getType(boundSignature)) ? SolverReturnStatus.UNCHANGED_SATISFIED : SolverReturnStatus.UNSOLVABLE; } - case LONG: { - return parameter; + else { + return actualType.getTypeSignature().equals(boundSignature) ? SolverReturnStatus.UNCHANGED_SATISFIED : SolverReturnStatus.UNSOLVABLE; } - default: - throw new IllegalStateException("Unknown parameter kind: " + parameter.getKind()); } } } diff --git a/presto-main/src/main/java/com/facebook/presto/type/TypeRegistry.java b/presto-main/src/main/java/com/facebook/presto/type/TypeRegistry.java index 7a7b3092b91d..65d7b6a3c1e7 100644 --- a/presto-main/src/main/java/com/facebook/presto/type/TypeRegistry.java +++ b/presto-main/src/main/java/com/facebook/presto/type/TypeRegistry.java @@ -521,4 +521,9 @@ private static boolean isCovariantParametrizedType(Type type) // if we ever introduce contravariant, this function should be changed to return an enumeration: INVARIANT, COVARIANT, CONTRAVARIANT return type instanceof MapType || type instanceof ArrayType; } + + public static boolean isCovariantTypeBase(String typeBase) + { + return typeBase.equals(StandardTypes.ARRAY) || typeBase.equals(StandardTypes.MAP); + } } diff --git a/presto-main/src/test/java/com/facebook/presto/metadata/TestSignatureBinder.java b/presto-main/src/test/java/com/facebook/presto/metadata/TestSignatureBinder.java index 1ce0d19bf136..2a54c8cbdebc 100644 --- a/presto-main/src/test/java/com/facebook/presto/metadata/TestSignatureBinder.java +++ b/presto-main/src/test/java/com/facebook/presto/metadata/TestSignatureBinder.java @@ -31,11 +31,11 @@ import static com.facebook.presto.metadata.FunctionKind.SCALAR; import static com.facebook.presto.metadata.Signature.comparableTypeParameter; -import static com.facebook.presto.metadata.Signature.longVariableExpression; import static com.facebook.presto.metadata.Signature.typeVariable; import static com.facebook.presto.metadata.Signature.withVariadicBound; -import static com.facebook.presto.metadata.SignatureBinder.applyBoundVariables; +import static com.facebook.presto.spi.type.BooleanType.BOOLEAN; import static com.facebook.presto.spi.type.TypeSignature.parseTypeSignature; +import static java.util.Objects.requireNonNull; import static java.util.stream.Collectors.toList; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; @@ -72,7 +72,52 @@ public void testBindLiteralForDecimal() } @Test - public void testResolveCalculatedTypes() + public void testBindPartialDecimal() + { + Signature function = functionSignature() + .returnType(parseTypeSignature(StandardTypes.BOOLEAN)) + .argumentTypes(parseTypeSignature("decimal(4,s)", ImmutableSet.of("s"))) + .build(); + + assertThat(function) + .boundTo("decimal(2,1)") + .withCoercion() + .produces(new BoundVariables( + ImmutableMap.of(), + ImmutableMap.of("s", 1L) + )); + + function = functionSignature() + .returnType(parseTypeSignature(StandardTypes.BOOLEAN)) + .argumentTypes(parseTypeSignature("decimal(p,1)", ImmutableSet.of("p"))) + .build(); + + assertThat(function) + .boundTo("decimal(2,0)") + .withCoercion() + .produces(new BoundVariables( + ImmutableMap.of(), + ImmutableMap.of("p", 3L) + )); + + assertThat(function) + .boundTo("decimal(2,1)") + .produces(new BoundVariables( + ImmutableMap.of(), + ImmutableMap.of("p", 2L) + )); + + assertThat(function) + .boundTo("bigint") + .withCoercion() + .produces(new BoundVariables( + ImmutableMap.of(), + ImmutableMap.of("p", 20L) + )); + } + + @Test + public void testBindLiteralForVarchar() { TypeSignature leftType = parseTypeSignature("varchar(x)", ImmutableSet.of("x")); TypeSignature rightType = parseTypeSignature("varchar(y)", ImmutableSet.of("y")); @@ -104,6 +149,121 @@ public void testResolveCalculatedTypes() )); } + @Test + public void testBindLiteralForRepeatedVarcharWithReturn() + { + TypeSignature leftType = parseTypeSignature("varchar(x)", ImmutableSet.of("x")); + TypeSignature rightType = parseTypeSignature("varchar(x)", ImmutableSet.of("x")); + + Signature function = functionSignature() + .returnType(parseTypeSignature(StandardTypes.BOOLEAN)) + .argumentTypes(leftType, rightType) + .build(); + + assertThat(function) + .boundTo("varchar(44)", "varchar(44)") + .produces(new BoundVariables( + ImmutableMap.of(), + ImmutableMap.of("x", 44L) + )); + assertThat(function) + .boundTo("varchar(44)", "varchar(42)") + .withCoercion() + .produces(new BoundVariables( + ImmutableMap.of(), + ImmutableMap.of("x", 44L) + )); + assertThat(function) + .boundTo("varchar(42)", "varchar(44)") + .withCoercion() + .produces(new BoundVariables( + ImmutableMap.of(), + ImmutableMap.of("x", 44L) + )); + assertThat(function) + .boundTo("unknown", "varchar(44)") + .withCoercion() + .produces(new BoundVariables( + ImmutableMap.of(), + ImmutableMap.of("x", 44L) + )); + } + + @Test + public void testBindLiteralForRepeatedDecimal() + { + TypeSignature leftType = parseTypeSignature("decimal(p,s)", ImmutableSet.of("p", "s")); + TypeSignature rightType = parseTypeSignature("decimal(p,s)", ImmutableSet.of("p", "s")); + + Signature function = functionSignature() + .returnType(parseTypeSignature(StandardTypes.BOOLEAN)) + .argumentTypes(leftType, rightType) + .build(); + + assertThat(function) + .boundTo("decimal(10,5)", "decimal(10,5)") + .produces(new BoundVariables( + ImmutableMap.of(), + ImmutableMap.of("p", 10L, "s", 5L) + )); + assertThat(function) + .boundTo("decimal(10,8)", "decimal(9,8)") + .withCoercion() + .produces(new BoundVariables( + ImmutableMap.of(), + ImmutableMap.of("p", 10L, "s", 8L) + )); + assertThat(function) + .boundTo("decimal(10,2)", "decimal(10,8)") + .withCoercion() + .produces(new BoundVariables( + ImmutableMap.of(), + ImmutableMap.of("p", 16L, "s", 8L) + )); + assertThat(function) + .boundTo("unknown", "decimal(10,5)") + .withCoercion() + .produces(new BoundVariables( + ImmutableMap.of(), + ImmutableMap.of("p", 10L, "s", 5L) + )); + } + + @Test + public void testBindLiteralForRepeatedVarchar() + throws Exception + { + Set literalParameters = ImmutableSet.of("x"); + TypeSignature leftType = parseTypeSignature("varchar(x)", literalParameters); + TypeSignature rightType = parseTypeSignature("varchar(x)", literalParameters); + TypeSignature returnType = parseTypeSignature("varchar(x)", literalParameters); + + Signature function = functionSignature() + .returnType(returnType) + .argumentTypes(leftType, rightType) + .build(); + + assertThat(function) + .withCoercion() + .boundTo(ImmutableList.of("varchar(3)", "varchar(5)"), "varchar(5)") + .produces(new BoundVariables( + ImmutableMap.of(), + ImmutableMap.of( + "x", 5L + ) + )); + + assertThat(function) + .withCoercion() + .boundTo(ImmutableList.of("varchar(3)", "varchar(5)"), "varchar(6)") + .produces(new BoundVariables( + ImmutableMap.of(), + ImmutableMap.of( + "x", 6L + ) + )); + } + @Test public void testBindUnknown() { @@ -162,31 +322,24 @@ public void testBindDifferentLiteralParameters() .fails(); } - @Test - public void testBindCalculatedLiteralParameter() + @Test(expectedExceptions = UnsupportedOperationException.class) + public void testNoVariableReuseAcrossTypes() throws Exception { - Set literalParameters = ImmutableSet.of("p1", "p2", "p3", "s"); + Set literalParameters = ImmutableSet.of("p1", "p2", "s"); TypeSignature leftType = parseTypeSignature("decimal(p1,s)", literalParameters); TypeSignature rightType = parseTypeSignature("decimal(p2,s)", literalParameters); - TypeSignature returnType = parseTypeSignature("decimal(p3,s)", literalParameters); Signature function = functionSignature() - .returnType(returnType) + .returnType(BOOLEAN.getTypeSignature()) .argumentTypes(leftType, rightType) - .longVariableConstraints(longVariableExpression("p3", "p1 + p2")) .build(); assertThat(function) .boundTo("decimal(2,1)", "decimal(3,1)") .produces(new BoundVariables( ImmutableMap.of(), - ImmutableMap.of( - "p1", 2L, - "p2", 3L, - "p3", 5L, - "s", 1L - ) + ImmutableMap.of() )); } @@ -259,7 +412,7 @@ public void testBindParametricTypeParameterToUnknown() assertThat(function) .withCoercion() .boundTo("unknown") - .fails(); + .succeeds(); } @Test @@ -333,6 +486,11 @@ public void testBindVarchar() assertThat(function) .boundTo(ImmutableList.of("varchar(1)"), "varchar(1)") .withCoercion() + .fails(); + + assertThat(function) + .boundTo(ImmutableList.of("varchar(1)"), "varchar(42)") + .withCoercion() .succeeds(); assertThat(function) @@ -371,6 +529,7 @@ public void testBindToUnparametrizedVarchar() assertThat(function) .boundTo("varchar(3)") + .withCoercion() .succeeds(); assertThat(function) @@ -415,6 +574,24 @@ public void testBasic() )); } + @Test + public void testMismatchedArgumentCount() + throws Exception + { + Signature function = functionSignature() + .returnType(parseTypeSignature(StandardTypes.BOOLEAN)) + .argumentTypes(parseTypeSignature(StandardTypes.BIGINT), parseTypeSignature(StandardTypes.BIGINT)) + .build(); + + assertThat(function) + .boundTo("bigint", "bigint", "bigint") + .fails(); + + assertThat(function) + .boundTo("bigint") + .fails(); + } + @Test public void testNonParametric() throws Exception @@ -466,6 +643,11 @@ public void testArray() .withCoercion() .fails(); + assertThat(getFunction) + .boundTo("row(bigint)") + .withCoercion() + .fails(); + Signature containsFunction = functionSignature() .returnType(parseTypeSignature("T")) .argumentTypes(parseTypeSignature("array(T)"), parseTypeSignature("T")) @@ -551,6 +733,46 @@ public void testMap() .fails(); } + @Test + public void testRow() + throws Exception + { + Signature function = functionSignature() + .returnType(BOOLEAN.getTypeSignature()) + .argumentTypes(parseTypeSignature("row(bigint)")) + .build(); + + assertThat(function) + .boundTo("row(bigint)") + .withCoercion() + .produces(new BoundVariables( + ImmutableMap.of(), + ImmutableMap.of() + )); + assertThat(function) + .boundTo("row(integer)") + .withCoercion() + .fails(); + + Signature biFunction = functionSignature() + .returnType(BOOLEAN.getTypeSignature()) + .argumentTypes(parseTypeSignature("row(T)"), parseTypeSignature("row(T)")) + .typeVariableConstraints(ImmutableList.of(typeVariable("T"))) + .build(); + + assertThat(biFunction) + .boundTo("row(bigint)", "row(bigint)") + .withCoercion() + .produces(new BoundVariables( + ImmutableMap.of("T", type("bigint")), + ImmutableMap.of() + )); + assertThat(biFunction) + .boundTo("row(integer)", "row(bigint)") + .withCoercion() + .fails(); + } + @Test public void testVariadic() throws Exception @@ -792,7 +1014,7 @@ public void testBindParameters() private void assertBindVariablesFails(String typeSignature, BoundVariables boundVariables, String reason) { try { - applyBoundVariables(parseTypeSignature(typeSignature, ImmutableSet.of("p", "s")), boundVariables); + SignatureBinder.applyBoundVariables(parseTypeSignature(typeSignature, ImmutableSet.of("p", "s")), boundVariables); fail(reason); } catch (RuntimeException e) { @@ -803,7 +1025,7 @@ private void assertBindVariablesFails(String typeSignature, BoundVariables bound private void assertThat(String typeSignature, BoundVariables boundVariables, String expectedTypeSignature) { assertEquals( - applyBoundVariables(parseTypeSignature(typeSignature, ImmutableSet.of("p", "s")), boundVariables).toString(), + SignatureBinder.applyBoundVariables(parseTypeSignature(typeSignature, ImmutableSet.of("p", "s")), boundVariables).toString(), expectedTypeSignature ); } @@ -818,7 +1040,7 @@ private SignatureBuilder functionSignature() private Type type(String signature) { TypeSignature typeSignature = TypeSignature.parseTypeSignature(signature); - return typeRegistry.getType(typeSignature); + return requireNonNull(typeRegistry.getType(typeSignature)); } private List types(String... signatures)