sql: include all schemas into schema.sql in the bundle #91245
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @cucaroach and @michae2)
There was a problem hiding this comment.
Thank you for doing this!
Reviewed all commit messages.
Reviewable status:
pkg/sql/explain_bundle.go line 494 at r1 (raw file):
rows, err := c.ie.QueryBufferedEx( c.ctx, "stmtEnvCollector",
Could we use a more descriptive name? Maybe something about the current statement bundle?
pkg/sql/explain_bundle.go line 738 at r1 (raw file):
} for _, r := range createAllSchemas { if strings.HasPrefix(r, "CREATE SCHEMA "+catconstants.PublicSchemaName) {
Will this work if someone has a schema starting with "public", like CREATE SCHEMA publicfoo?
This commit makes it so that all schemas are added into the `schema.sql` file of the statement bundle using `SHOW CREATE ALL SCHEMAS;` statement (the only exception is the public schema that always exists in CRDB clusters). This will make it more likely that `debug statement-bundle recreate` succeeds on the bundles. Epic: None Release note: None
yuzefovich
force-pushed
the
fix-stmt-bundle
branch
from
a3917af to
205bd84
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/sql/explain_bundle.go line 494 at r1 (raw file):
Previously, michae2 (Michael Erickson) wrote…
Could we use a more descriptive name? Maybe something about the current statement bundle?
It doesn't seem that important (it would only show up as a log tag), and I'd also like it to keep it for consistency with query() method. I guess places that use query() method can also be invoked during EXPLAIN (OPT, ENV) and that's why the opName is not very descriptive there, but the schemas are only used for the bundle.
pkg/sql/explain_bundle.go line 738 at r1 (raw file):
Previously, michae2 (Michael Erickson) wrote…
Will this work if someone has a schema starting with "public", like
CREATE SCHEMA publicfoo?
Good point, made it an exact comparison.
There was a problem hiding this comment.
Reviewed all commit messages.
Reviewable status:
pkg/sql/explain_bundle.go line 494 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
It doesn't seem that important (it would only show up as a log tag), and I'd also like it to keep it for consistency with
query()method. I guess places that usequery()method can also be invoked duringEXPLAIN (OPT, ENV)and that's why the opName is not very descriptive there, but the schemas are only used for the bundle.
I see. That's fine, thanks for checking it out!
|
TFTRs! bors r+ |
|
Build succeeded: |
This commit makes it so that all schemas are added into the
schema.sqlfile of the statement bundle usingSHOW CREATE ALL SCHEMAS;statement (the only exception is the public schema that always exists in CRDB clusters). This will make it more likely thatdebug statement-bundle recreatesucceeds on the bundles.Fixes: #91099.
Epic: CRDB-14510
Release note: None