-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sql: include DB name in create_statement in create_type_statements #137580
Conversation
7757d42
to
e71e158
Compare
crdb_internal.create_type_statements
This commit updates how we populate `crdb_internal.create_type_statements` virtual table so that `create_statement` column included the DB name (previously only the schema and the UDT name were included). This will make it easier to reproduce stmt bundles with cross-DB references. Also, this commit fixes how we use this virtual table during bundle creation - we now properly use `"".crdb_internal` search path and cast the type oid to `OID` type. The same fix is applied for `create_function_statements` and `create_procedure_statements` vtables. Release note: None
e71e158
to
644c458
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! thanks for this improvement. is there anything remaining to resolve #137579 now?
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Dedej-Bergin)
TFTR and for the suggestion! For #137579 - I think there is still unexpected behavior when we use the virtual index without OID cast - I'd assume this should return 0 rows: SELECT * FROM crdb_internal.create_type_statements WHERE descriptor_id = (SELECT 'db.public.enum'::REGTYPE::INT - 100000);
-- 100 | defaultdb | public | 108 | enum | CREATE TYPE public.enum AS ENUM ('x', 'y', 'z') | {x,y,z} But I'm not sure if it's important and whether we should do anything about it. I'm no longer blocked on that issue, and if you think we wouldn't want to change the behavior, I'm ok closing it. bors r+ |
Hm, yeah, that's true - we now will see the DB name in the output of |
This commit updates how we populate
crdb_internal.create_type_statements
virtual table so thatcreate_statement
column included the DB name (previously only the schema and the UDT name were included). This will make it easier to reproduce stmt bundles with cross-DB references.Also, this commit fixes how we use this virtual table during bundle creation - we now properly use
"".crdb_internal
search path and cast the type oid toOID
type. The same fix is applied forcreate_function_statements
andcreate_procedure_statements
vtables.Epic: None
Release note: None