-
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: extend SHOW CREATE TABLE to use fully-qualified names #137719
Conversation
This commit extends SHOW CREATE TABLE syntax to use fully-qualified names for tables as well as user-defined types used in the table. This is needed to make the recreation of stmt bundles collected on multi-region tables easier. Note that for UDTs this behavior was already the case up until this Summer when we switched to not showing the DB name for portability. The stmt bundle recreation case needs the exact reproduction since we might be dealing with cross-DB references and things like `crdb_internal_region` type unavailable in the current DB when creating a multi-region table. `SHOW CREATE TABLE` syntax is extended to support `FULLY_QUALIFIED` option. We already had a documented `REDACT` option, so any combination of these two options is now supported. However, at least as of right now, I chose to not document this new option. We now use the new option when building the stmt bundle. It is powered by adding two new columns `create_statement_fq` and `create_redactable_fq` to `crdb_internal.create_statements` virtual table. These columns are only populated for tables and will remain empty strings for views and sequences. The delegated show query is adjusted to use the proper column based on the options. Release note: None
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.
I think it would be a lot simpler to add a USE
statement to bundles. If we go down the route of making CREATE statements fully-qualified, wouldn't we have to add that ability to every type of CREATE statement that could appear in schema.sql?
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @spilchen)
We already use some CREATE statements that use fully-qualified names for bundles (and will use more with #137662), and I think we should embrace this approach fully. I considered using CREATE DATABASE mr1 ...;
CREATE DATABASE mr2 ...;
...
USE mr1;
CREATE TABLE t1 ...;
...
USE mr2;
CREATE TABLE t2 ...; Now let's add user-defined types into the mix. Due to how we populate CREATE DATABASE mr1 ...;
CREATE DATABASE mr2 ...;
...
USE mr1;
CREATE TYPE udt1 ...;
...
USE mr2;
CREATE TYPE udt2 ...;
...
USE mr1;
CREATE TABLE t1 ... <using udt1> ...;
...
USE mr2;
CREATE TABLE t2 ... <using udt2> ...; Effectively we need to teach the bundle building code to be DB-aware for all objects. I actually started going down this path a bit, but then UDTs needed the special handling in addition to tables, and I thought that adjusting the CREATE stmts themselves would be cleaner (so I added #137580 for UDTs). The alternative approach (which is what this PR helps with) is to ensure that all CREATE statements included in Having typed this all out, I don't feel strongly about |
I implemented the USE approach in #138023, and it seems cleaner, so let's hold off on this change. |
We implemented the USE approach in #138023, so this is no longer needed. |
This commit extends SHOW CREATE TABLE syntax to use fully-qualified names for tables as well as user-defined types used in the table. This is needed to make the recreation of stmt bundles collected on multi-region tables easier. Note that for UDTs this behavior was already the case up until this Summer when we switched to not showing the DB name for portability. The stmt bundle recreation case needs the exact reproduction since we might be dealing with cross-DB references and things like
crdb_internal_region
type unavailable in the current DB when creating a multi-region table.SHOW CREATE TABLE
syntax is extended to supportFULLY_QUALIFIED
option. We already had a documentedREDACT
option, so any combination of these two options is now supported. However, at least as of right now, I chose to not document this new option. We now use the new option when building the stmt bundle.It is powered by adding two new columns
create_statement_fq
andcreate_redactable_fq
tocrdb_internal.create_statements
virtual table. These columns are only populated for tables and will remain empty strings for views and sequences. The delegated show query is adjusted to use the proper column based on the options.Informs: #99131.
Fixes: #123897.
Epic: None
Release note: None