Skip to content

feat(core): derive return types parameterized by a length or precision - #1141

Open
alexandrefimov wants to merge 1 commit into
substrait-io:mainfrom
alexandrefimov:issue-1139-parameterized-return-types
Open

feat(core): derive return types parameterized by a length or precision#1141
alexandrefimov wants to merge 1 commit into
substrait-io:mainfrom
alexandrefimov:issue-1139-parameterized-return-types

Conversation

@alexandrefimov

Copy link
Copy Markdown
Contributor

TypeExpressionEvaluator bound and evaluated two shapes — a numbered wildcard and DECIMAL<P,S> — and every other parameterized class reached the throwing base of TypeExpressionVisitor. So resolveType, which FunctionBindingResolver.deriveOutputType and validateOutputType are built on, could not derive the output type of 86 declared variants across the standard catalog: 306 of 377 scalar variants evaluated, 70 of 83 aggregate, 81 of 94 window.

The remaining classes whose parameter is an integer to substitute now bind and evaluate the way DECIMAL already did: varchar, fixedchar, fixedbinary, the three precision_* types, interval_day and interval_compound. The last two are there for symmetry; no standard extension declares them parameterized today.

What this changes in practice is AggregateConversion.FunctionBindingValidation.EXTENSION_DECLARATION, whose own documentation said it was not adoptable for plans using these functions. It still is not adoptable for two groups, and both Javadocs listing them were wrong in a second way: assume_timezone and the strptime_* family were filed under the parameterized classes, when they fail on a return program. Both lists are now taken from the catalog rather than from memory.

list<anyN> and return programs are unchanged — their parameter is a type to evaluate rather than an integer to substitute — and are worth their own issues rather than being folded in here.

Part of #1139. The inconsistent-P question raised there is not settled by this: binding now makes bindType's existing Inconsistent binding reachable for a shared parameter, which is what the tests pin, but nothing turns validation on by default.

TypeExpressionEvaluator bound and evaluated two shapes, a numbered
wildcard and DECIMAL<P,S>, and every other parameterized class reached
the throwing base. So resolveType — which FunctionBindingResolver's
deriveOutputType and validateOutputType are built on — could not derive
the output type of 86 declared variants across the standard catalog.

The remaining classes whose parameter is an integer to substitute bind
and evaluate the same way DECIMAL already did: varchar, fixedchar,
fixedbinary, the three precision_* types, interval_day and
interval_compound. The last two have no declared users today and are
included for symmetry.

list<anyN> and return programs are unchanged: their parameter is a type
to evaluate rather than an integer to substitute, and they need work of
a different kind.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant