sql/parser: Define overload returnType interface and cleanup builtins #13209
Conversation
|
Reviewed 14 of 14 files at r1, 9 of 9 files at r2. pkg/sql/parser/overload.go, line 203 at r1 (raw file):
Welcome to the world of higher-kinded types. What you're defining here is a family type-level functions (functions from types to types). What you call a "method" is simply a function which, given the types of the arguments, returns a type for the return value. Your "fixed" thing is a constant function -- regardless of the argument types it returns the same type. I'd suggest:
pkg/sql/parser/overload.go, line 437 at r1 (raw file):
Comments from Reviewable |
|
I just ran a Sequelize app against this branch and got: It looks like |
This change introduces the notion of a `returnType` enum for overload implementations. The purpose of this is to more clearly express cases where overloads return types are based on their input argument types. This was previously possible, but was messy and inextensible. > returnType defines the method in which a functions' return type is determined. > The vast majority of functions have a hard-coded return type, denoted as "fixed". > However, some functions' return types are not static, but are instead based on > the types of arguments provided to them.
Fixes cockroachdb#12207. This change adjusts aggregate constructors take their parameter types. This is useful for when aggregate behavior needs to change depending on the input parameters. For instance, the same constructor needs to be used for `array_agg(string)` and `array_agg(name)`, but their runtime behavior needs to be different.
nvanbenschoten
force-pushed
the
nvanbenschoten/returnStyleAlt
branch
from
15a5e71 to
977e6d5
Compare
This change unifies the builtins for `unnest`. It does so by creating a `returnTyper` type, which is a function that replaces the `returnType` interface. This function defines the type-level function in which a builtin function's return type is determined. It return an out-of-band constant when the arguments provided are not sufficient to determine a return type, which is used during overload resolution. In introducing `dynamicReturnType`, we can remove the previous `fixedReturnType`, `identityReturnType` and `identityArrayReturnType` implementations, which were holdovers from before a more general approach to type-level function was introduced.
nvanbenschoten
force-pushed
the
nvanbenschoten/returnStyleAlt
branch
from
977e6d5 to
5132cd6
Compare
|
@knz thanks for all the input! I've gone ahead and removed the unnecessary specializations of @cuongdo Thanks for testing with this branch. The error you got looks unrelated and implies that this change addressed the current issue. Unfortunately, it also implies that we may need to introduce an |
|
I forgot about the |
|
Awesome. The type computation stuff looks great. LGTM Reviewed 3 of 14 files at r3, 6 of 9 files at r4, 5 of 8 files at r5. Comments from Reviewable |
|
Reviewed 1 of 8 files at r5. Comments from Reviewable |
|
TFTR! @jordanlewis I opened #13434 per your request to continue tracking the |
|
Thanks @nvanbenschoten! |
Supersedes #12644 and addresses all comments.
Fixes #12207.
This change introduces the notion of a
returnTypertype-level function foroverload implementations. The purpose of this is to more clearly express cases
where overloads return types are based on their input argument types.
This was previously possible but was messy and inextensible.
The change also adjusts aggregate constructors take their parameter types.
This is useful for when aggregate behavior needs to change depending on
the input parameters. For instance, the same constructor needs to be
used for
array_agg(string)andarray_agg(name), but their runtimebehavior needs to be different.
This change is