feat(core): derive return types parameterized by a length or precision - #1141
Open
alexandrefimov wants to merge 1 commit into
Open
feat(core): derive return types parameterized by a length or precision#1141alexandrefimov wants to merge 1 commit into
alexandrefimov wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TypeExpressionEvaluatorbound and evaluated two shapes — a numbered wildcard andDECIMAL<P,S>— and every other parameterized class reached the throwing base ofTypeExpressionVisitor. SoresolveType, whichFunctionBindingResolver.deriveOutputTypeandvalidateOutputTypeare 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
DECIMALalready did:varchar,fixedchar,fixedbinary, the threeprecision_*types,interval_dayandinterval_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_timezoneand thestrptime_*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-
Pquestion raised there is not settled by this: binding now makesbindType's existingInconsistent bindingreachable for a shared parameter, which is what the tests pin, but nothing turns validation on by default.