sql: handle implicit record return types better #96696
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @mgartner)
rharding6373
force-pushed
the
20230131_udf_implicitrecord
branch
2 times, most recently
from
66aede2
to
86386f9
Compare
|
TFTR! bors r=DrewKimball |
There was a problem hiding this comment.
Reviewed 1 of 3 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status:
pkg/sql/opt/optbuilder/scalar.go line 694 at r2 (raw file):
// defined in the function. rtyp := f.ResolvedType() if rtyp.UserDefined() {
Is there any downside to performing this check for non-user-defined types?
Can you move this check outside the loop (probably above) because it doesn't reference any specific stmts within the body.
|
bors r- |
|
Canceled. |
rharding6373
force-pushed
the
20230131_udf_implicitrecord
branch
from
86386f9
to
cea454b
Compare
This PR validates the UDF return type at build time if it is a user-defined type. If the return type is no longer compatible with what the UDF body returns, we return an error instead. This is more in line with postgres behavior. Fixes: 95558 Epic: CRDB-19496 Release note (sql change): UDFs with implicit record return types will return an error when called if the return type has been altered and is no longer compatible with the body of the UDF.
rharding6373
force-pushed
the
20230131_udf_implicitrecord
branch
from
cea454b
to
bed5677
Compare
There was a problem hiding this comment.
TFTR!
Reviewable status:
pkg/sql/opt/optbuilder/scalar.go line 694 at r2 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
Is there any downside to performing this check for non-user-defined types?
Can you move this check outside the loop (probably above) because it doesn't reference any specific stmts within the body.
Good catch, moved to the beginning of the function.
Unfortunately there is some assumption under the hood of ResolveTypeByOID that it's a user defined type. Apparently this has to do with the fact that only UDTs need to be hydrated.
|
Thanks all! bors r+ |
|
Build succeeded: |
This PR validates the UDF return type at build time if it is a user-defined type. If the return type is no longer compatible with what the UDF body returns, we return an error instead. This is more in line with postgres behavior.
Fixes: #95558
Epic: CRDB-19496
Release note (sql change): UDFs with implicit record return types will return an error when called if the return type has been altered and is no longer compatible with the body of the UDF.