trinodb / trino

Official repository of Trino, the distributed SQL query engine for big data, formerly known as PrestoSQL (https://trino.io)
https://trino.io
Apache License 2.0
9.88k stars 2.86k forks source link

Cryptic error when function declaration uses non-canonical type name #3374

Open martint opened 4 years ago

martint commented 4 years ago

Given this function:

@ScalarFunction
@SqlType("bigint")
public static long test(@SqlType("int") long value)
{
    return 1;
}

SELECT test(1) fails with:

java.lang.IllegalArgumentException: Could not extract bound variables
    at io.prestosql.metadata.FunctionRegistry.lambda$getSpecializedFunctionKey$4(FunctionRegistry.java:745)
    at java.util.Optional.orElseThrow(Optional.java:290)
    at io.prestosql.metadata.FunctionRegistry.getSpecializedFunctionKey(FunctionRegistry.java:745)
    at io.prestosql.metadata.FunctionRegistry.getScalarFunctionImplementation(FunctionRegistry.java:704)
    at io.prestosql.metadata.MetadataManager.getScalarFunctionImplementation(MetadataManager.java:1463)
    at io.prestosql.sql.InterpretedFunctionInvoker.invoke(InterpretedFunctionInvoker.java:55)
    at io.prestosql.sql.planner.ExpressionInterpreter$Visitor.visitFunctionCall(ExpressionInterpreter.java:942)
    at io.prestosql.sql.tree.FunctionCall.accept(FunctionCall.java:110)
    at io.prestosql.sql.tree.AstVisitor.process(AstVisitor.java:27)
    at io.prestosql.sql.planner.ExpressionInterpreter.optimize(ExpressionInterpreter.java:286)
    at io.prestosql.sql.planner.iterative.rule.SimplifyExpressions.rewrite(SimplifyExpressions.java:54)
    at io.prestosql.sql.planner.iterative.rule.SimplifyExpressions.lambda$createRewrite$0(SimplifyExpressions.java:79)
    at io.prestosql.sql.planner.iterative.rule.ExpressionRewriteRuleSet$ProjectExpressionRewrite.lambda$apply$0(ExpressionRewriteRuleSet.java:125)
    at io.prestosql.sql.planner.plan.Assignments.lambda$rewrite$1(Assignments.java:108)
    at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
    at java.util.Iterator.forEachRemaining(Iterator.java:116)
    at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
    at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
    at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
    at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
    at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
    at io.prestosql.sql.planner.plan.Assignments.rewrite(Assignments.java:109)
    at io.prestosql.sql.planner.iterative.rule.ExpressionRewriteRuleSet$ProjectExpressionRewrite.apply(ExpressionRewriteRuleSet.java:125)
    at io.prestosql.sql.planner.iterative.rule.ExpressionRewriteRuleSet$ProjectExpressionRewrite.apply(ExpressionRewriteRuleSet.java:106)
    at io.prestosql.sql.planner.iterative.IterativeOptimizer.transform(IterativeOptimizer.java:165)
    at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreNode(IterativeOptimizer.java:140)
    at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreGroup(IterativeOptimizer.java:105)
    at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreChildren(IterativeOptimizer.java:190)
    at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreGroup(IterativeOptimizer.java:107)
    at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreChildren(IterativeOptimizer.java:190)
    at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreGroup(IterativeOptimizer.java:107)
    at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreChildren(IterativeOptimizer.java:190)
    at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreGroup(IterativeOptimizer.java:107)
    at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreChildren(IterativeOptimizer.java:190)
    at io.prestosql.sql.planner.iterative.IterativeOptimizer.exploreGroup(IterativeOptimizer.java:107)
    at io.prestosql.sql.planner.iterative.IterativeOptimizer.optimize(IterativeOptimizer.java:96)
    at io.prestosql.sql.planner.LogicalPlanner.plan(LogicalPlanner.java:196)
    at io.prestosql.sql.planner.LogicalPlanner.plan(LogicalPlanner.java:185)
    ...

The query succeeds if the canonical name for the integer type is used in the function argument declaration: @SqlType("integer")

Ideally, this should work for both type synonyms, but at a minimum, we should improve the error message.

yuokada commented 4 years ago

Should we add a check in FunctionRegistry.class to improve the error message like this?

            checkArgument(!functionMetadata.getSignature().getArgumentTypes().contains(new TypeSignature("int")), 
                 "Function arguments should use a canonical name integer of int", functionMetadata.getSignature());
findepi commented 4 years ago

@yuokada ideally we would try to determine something is an alias in a generic manner (types can be provided by plugins too).