-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
release-24.3: sql: use memo metadata to add routines and UDTs to statement bundles #136124
base: release-24.3
Are you sure you want to change the base?
release-24.3: sql: use memo metadata to add routines and UDTs to statement bundles #136124
Conversation
This commit changes the naming of some methods and data structures used in query plan metadata to refer to "routines" instead of "udfs". This clarifies that stored procedures are also valid targets for metadata tracking. Epic: None Release note: None
This commit updates the statement bundle logic to take advantage of the information stored in the query plan metadata so that only relevant routines are shown in `schema.sql` for a statement bundle. In addition, stored procedures are now tracked in the metadata in addition to UDFs. This has no impact on query-plan caching, since we currently do not cache plans that invoke stored procedures. Fixes cockroachdb#132142 Fixes cockroachdb#104976 Release note (bug fix): Fixed a bug that prevented the create statement for a routine from being shown in a statement bundle. This happened when the routine was created on a schema other than `public`. The bug has existed since v23.1.
This commit modifies the statement bundle collection logic to use the query plan metadata to determine which types to display in `schema.sql`. This ensures that only types which are referenced by the query are shown. Informs cockroachdb#104976 Release note: None
Thanks for opening a backport. Please check the backport criteria before merging:
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
Also, please add a brief release justification to the body of your PR to justify this |
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 there's a couple things you might want to fixup on master before merging these backports.
Reviewed 4 of 4 files at r1, 3 of 3 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball)
pkg/sql/explain_bundle_test.go
line 430 at r2 (raw file):
t, fmt.Sprint(rows), "s.foo", func(name, contents string) error { if name == "schema.sql" { reg := regexp.MustCompile("s.foo")
.
is a wild card match, so you probably want "s\.foo"
instead
pkg/sql/explain_bundle_test.go
line 434 at r2 (raw file):
return errors.Errorf("could not find definition for 's.foo' function in schema.sql") } reg = regexp.MustCompile("^foo")
This will only match "foo" at the start of the line, not "CREATE FUNCTION foo ...", right? And the foo
will be fully qualified in the bundle, so maybe you want something like "public\.foo"
?
pkg/sql/explain_bundle_test.go
line 465 at r2 (raw file):
return errors.Errorf("could not find definition for 's.bar' procedure in schema.sql") } reg = regexp.MustCompile("^bar")
see my comment above
pkg/sql/explain_bundle_test.go
line 469 at r3 (raw file):
t, fmt.Sprint(rows), "s.bar", func(name, contents string) error { if name == "schema.sql" { reg := regexp.MustCompile("s.bar")
"s\.bar"
pkg/sql/explain_bundle.go
line 630 at r2 (raw file):
if mem.Metadata().HasUserDefinedRoutines() { // Get all relevant user-defined routines. var ids intsets.Fast
nit: ids
doesn't seem necessary - why can't the code in the ids.ForEach
anonymous function o in the ForEachUserDefinedRoutine
anonymous function?
Backport 3/3 commits from #132147.
/cc @cockroachdb/release
opt: replace "udf" with "routine" in the metadata
This commit changes the naming of some methods and data structures used
in query plan metadata to refer to "routines" instead of "udfs". This
clarifies that stored procedures are also valid targets for metadata
tracking.
Epic: None
Release note: None
sql: use memo metadata to add routines to statement bundles
This commit updates the statement bundle logic to take advantage of the
information stored in the query plan metadata so that only relevant routines
are shown in
schema.sql
for a statement bundle. In addition, storedprocedures are now tracked in the metadata in addition to UDFs. This has
no impact on query-plan caching, since we currently do not cache plans
that invoke stored procedures.
Fixes #132142
Fixes #104976
Release note (bug fix): Fixed a bug that prevented the create statement
for a routine from being shown in a statement bundle. This happened when
the routine was created on a schema other than
public
. The bug hasexisted since v23.1.
sql: use memo metadata to add UDTs to statement bundle
This commit modifies the statement bundle collection logic to use the
query plan metadata to determine which types to display in
schema.sql
.This ensures that only types which are referenced by the query are shown.
Informs #104976
Release note: None
Release justification: statement bundles improvements