Skip to content
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.

Sign up for GitHub

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.2: sql: fix including all related FK tables into the stmt bundle #137792

Merged
merged 1 commit into from
Dec 20, 2024
Merged

release-24.2: sql: fix including all related FK tables into the stmt bundle #137792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions pkg/sql/explain_bundle_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,11 +276,18 @@ CREATE TABLE users(id UUID DEFAULT gen_random_uuid() PRIMARY KEY, promo_id INT R
})

t.Run("foreign keys", func(t *testing.T) {
// All tables should be included in the stmt bundle, regardless of which
// one we query because all of them are considered "related" (even
// though we don't specify ON DELETE and ON UPDATE actions).
tableNames := []string{"parent", "child1", "child2", "grandchild1", "grandchild2"}
r.Exec(t, "CREATE TABLE parent (pk INT PRIMARY KEY, v INT);")
r.Exec(t, "CREATE TABLE child (pk INT PRIMARY KEY, fk INT REFERENCES parent(pk));")
r.Exec(t, "CREATE TABLE child1 (pk INT PRIMARY KEY, fk INT REFERENCES parent(pk));")
r.Exec(t, "CREATE TABLE child2 (pk INT PRIMARY KEY, fk INT REFERENCES parent(pk));")
r.Exec(t, "CREATE TABLE grandchild1 (pk INT PRIMARY KEY, fk INT REFERENCES child1(pk));")
r.Exec(t, "CREATE TABLE grandchild2 (pk INT PRIMARY KEY, fk INT REFERENCES child2(pk));")
contentCheck := func(name, contents string) error {
if name == "schema.sql" {
for _, tableName := range []string{"parent", "child"} {
for _, tableName := range tableNames {
if regexp.MustCompile("CREATE TABLE defaultdb.public."+tableName).FindString(contents) == "" {
return errors.Newf(
"could not find 'CREATE TABLE defaultdb.public.%s' in schema.sql:\n%s", tableName, contents)
Expand All @@ -289,12 +296,12 @@ CREATE TABLE users(id UUID DEFAULT gen_random_uuid() PRIMARY KEY, promo_id INT R
}
return nil
}
for _, tableName := range []string{"parent", "child"} {
for _, tableName := range tableNames {
rows := r.QueryStr(t, "EXPLAIN ANALYZE (DEBUG) SELECT * FROM "+tableName)
checkBundle(
t, fmt.Sprint(rows), "child", contentCheck, false, /* expectErrors */
base, plans, "stats-defaultdb.public.parent.sql", "stats-defaultdb.public.child.sql",
"distsql.html vec.txt vec-v.txt",
base, plans, "stats-defaultdb.public.parent.sql", "stats-defaultdb.public.child1.sql", "stats-defaultdb.public.child2.sql",
"stats-defaultdb.public.grandchild1.sql", "stats-defaultdb.public.grandchild2.sql", "distsql.html vec.txt vec-v.txt",
)
}
})
Expand Down
64 changes: 29 additions & 35 deletions pkg/sql/opt/metadata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -995,49 +995,43 @@ func (md *Metadata) getAllReferenceTables(
var tableSet intsets.Fast
var tableList []cat.DataSource
var addForeignKeyReferencedTables func(tab cat.Table)
var addForeignKeyReferencingTables func(tab cat.Table)
// handleRelatedTables is a helper function that processes the given table
// if it hasn't been handled yet by adding all referenced and referencing
// table of the given one, including via transient (recursive) FK
// relationships.
handleRelatedTables := func(tabID cat.StableID) {
if !tableSet.Contains(int(tabID)) {
tableSet.Add(int(tabID))
ds, _, err := catalog.ResolveDataSourceByID(ctx, cat.Flags{}, tabID)
if err != nil {
// This is a best-effort attempt to get all the tables, so don't
// error.
return
}
refTab, ok := ds.(cat.Table)
if !ok {
// This is a best-effort attempt to get all the tables, so don't
// error.
return
}
// We want to include all tables that we reference before adding
// ourselves, followed by all tables that reference us.
addForeignKeyReferencedTables(refTab)
tableList = append(tableList, ds)
addForeignKeyReferencingTables(refTab)
}
}
addForeignKeyReferencedTables = func(tab cat.Table) {
for i := 0; i < tab.OutboundForeignKeyCount(); i++ {
tabID := tab.OutboundForeignKey(i).ReferencedTableID()
if !tableSet.Contains(int(tabID)) {
tableSet.Add(int(tabID))
ds, _, err := catalog.ResolveDataSourceByID(ctx, cat.Flags{}, tabID)
if err != nil {
// This is a best-effort attempt to get all the tables, so don't error.
continue
}
refTab, ok := ds.(cat.Table)
if !ok {
// This is a best-effort attempt to get all the tables, so don't error.
continue
}
// We want to include all tables that we reference before adding
// ourselves.
addForeignKeyReferencedTables(refTab)
tableList = append(tableList, ds)
}
handleRelatedTables(tabID)
}
}
var addForeignKeyReferencingTables func(tab cat.Table)
addForeignKeyReferencingTables = func(tab cat.Table) {
for i := 0; i < tab.InboundForeignKeyCount(); i++ {
tabID := tab.InboundForeignKey(i).OriginTableID()
if !tableSet.Contains(int(tabID)) {
tableSet.Add(int(tabID))
ds, _, err := catalog.ResolveDataSourceByID(ctx, cat.Flags{}, tabID)
if err != nil {
// This is a best-effort attempt to get all the tables, so don't error.
continue
}
refTab, ok := ds.(cat.Table)
if !ok {
// This is a best-effort attempt to get all the tables, so don't error.
continue
}
// We want to include ourselves before all tables that reference
// us.
tableList = append(tableList, ds)
addForeignKeyReferencingTables(refTab)
}
handleRelatedTables(tabID)
}
}
for i := range md.tables {
Expand Down