Skip to content

core: the spec's derivation-function spellings fail catalog load with IllegalStateException, taking the whole YAML file with them #1311

Description

@nielspardon

site/docs/expressions/scalar_functions.md defines the derivation language's operations as named functions: type_parameter, integer_parameter, not, and, or, multiply, divide, add, subtract, min, max, equal, greater_than, less_than, covers, and the if/then/else form. The grammar's #FunctionCall alternative accepts any identifier, but ParseToPojo handles only two of them.

ParseToPojo.getFunctionType maps MIN and MAX and throws IllegalStateException for everything else, and visitFunctionCall rejects any arity other than 2 before that, so the unary forms cannot parse at all. integer_parameter is special-cased earlier and unwrapped.

Measured on main at cf581f4:

Expression Result
varchar<min(L, 6)> parses
varchar<add(L, 1)> IllegalStateException: The following operation was unrecognized: add
varchar<equal(L, 10) ? 1 : 2> IllegalStateException: The following operation was unrecognized: equal
varchar<covers(varchar<20>, varchar<L>) ? 1 : 2> IllegalStateException: The following operation was unrecognized: covers
varchar<abs(L)> IllegalStateException: Only two argument functions exist for type expressions, got 1 for: abs
varchar<not(L > 1) ? 1 : 2> IllegalStateException: Only two argument functions exist for type expressions, got 1 for: not
varchar<type_parameter(L)> IllegalStateException: Only two argument functions exist for type expressions, got 1 for: type_parameter

Two problems beyond the missing coverage. The failure is an IllegalStateException during parsing, which Deserializers.ParseDeserializer wraps into a JsonMappingException, so one declaration using a spec-defined spelling fails SimpleExtension.load() for the entire YAML file and every other function in it becomes unresolvable — rather than that one derivation failing closed with the UnsupportedOperationException TypeExpressionEvaluator's Javadoc documents as the contract. And covers has a case COVERS mapping from := in getBinaryExpressionType, but no expr production in SubstraitType.g4 consumes that token, so the operator spelling is unreachable while the function spelling that the spec actually defines is the one that throws.

abs is worth noting separately: it is not in the spec's list either, but substrait-go's FunctionCallExpr supports abs, min and max, so a declaration written against that binding would fail here.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcorePull requests that update java code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions