sql: fix pg_type_is_visible and ::regtype cast to handle UDTs#62181
sql: fix pg_type_is_visible and ::regtype cast to handle UDTs#62181craig[bot] merged 3 commits intocockroachdb:masterfrom
Conversation
|
This change is |
| query B | ||
| SELECT pg_type_is_visible(t.oid) | ||
| FROM pg_type t | ||
| WHERE t.typname = 'int8' |
There was a problem hiding this comment.
maybe:
query B
SELECT t.typname pg_type_is_visible(t.oid)
FROM pg_type t
WHERE t.typname IN ( 'int8', '_date', ...)
pkg/sql/resolver.go
Outdated
| if _, ok := types.OidToType[typeID]; ok { | ||
| return true, true, nil | ||
| } | ||
| // TODO(ajwerner): look at this error and only no-op if it is |
There was a problem hiding this comment.
any reason we can't do this now
There was a problem hiding this comment.
doesn't seem there's a reason we can't do it now -- i meant to ask @ajwerner about this comment (which was just added recently). it does look like there's a few more errors than just ErrDescriptorNotFound that we need to check for
There was a problem hiding this comment.
i guess my concern is if it fails due to some transient error it will return False instead of error. arguably this is bad...
There was a problem hiding this comment.
Right, also, if it is a transaction restart I think it could expose some other anomalies.
FWIW cleaning up the errors here and probably making these methods return clear booleans and deal with flags properly to declare desired behavior is a major roadmap item for schema this release.
This code is actually a nightmare right now. I'm trying to untangle all of the errors you can get and it's making my angry. I'm 75% sure that the condition at the moment here is:
errors.Is(err, catalog.ErrDescriptorNotFound) || errors.Is(err, catalog.ErrDescriptorDropped) || pgcode.GetPGCode(err) == pgcode.UndefinedObject
Sorry 😓
There was a problem hiding this comment.
thank you for that list and the roadmap item!
Release note: None
ajwerner
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @otan, @pbardea, and @rafiss)
pkg/sql/resolver.go, line 262 at r2 (raw file):
Previously, rafiss (Rafi Shamim) wrote…
thank you for that list and the roadmap item!
Gah
pkg/sql/resolver.go, line 220 at r5 (raw file):
if errors.Is(err, catalog.ErrDescriptorNotFound) || errors.Is(err, catalog.ErrDescriptorDropped) || pgerror.GetPGCode(err) == pgcode.UndefinedObject {
This one needs pgcode.UndefinedTable 🎉
Release note (bug fix): Previously, the pg_type_is_visible builtin function did not correctly handle user-defined types. This is fixed now.
Release note (bug fix): Previously, casting an OID to a regtype did not work for user-defined types. This is now fixed.
|
ty for the tip! bors r=otan,ajwerner |
|
Build succeeded: |
fixes #61216
fixes #60008
The 1st commit is a simple refactor.
Release note (bug fix): Previously, casting an OID to a regtype did not
work for user-defined types. This is now fixed.
Release note (bug fix): Previously, the pg_type_is_visible builtin
function did not correctly handle user-defined types. This is fixed now.