Skip to content

Commit

Permalink
sql/catalog: eliminate repeated database descriptor lookups
Browse files Browse the repository at this point in the history
Previously, one code path of `ResolveExisting` would repeatedly lookup the
same database descriptor for each schema in the search path. The
`pg_catalog` and `pg_extension` schemas are implicitly included
in the search path, so in the common case where the search path is the
default value of `"$user", public`, there were four lookups for the same
database descriptor.

This commit performs the database descriptor lookup once and reuses it
for looking up objects in each schema of the search path.

Release note: None
  • Loading branch information
mgartner committed Dec 11, 2024
1 parent 3d8a3b5 commit 0c043b9
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 24 deletions.
12 changes: 6 additions & 6 deletions pkg/bench/rttanalysis/testdata/benchmark_expectations
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,12 @@ exp,benchmark
1,SystemDatabaseQueries/select_system.users_with_empty_database_Name
1,SystemDatabaseQueries/select_system.users_with_schema_Name
1,SystemDatabaseQueries/select_system.users_without_schema_Name
20,Truncate/truncate_1_column_0_rows
20,Truncate/truncate_1_column_1_row
20,Truncate/truncate_1_column_2_rows
20,Truncate/truncate_2_column_0_rows
20,Truncate/truncate_2_column_1_rows
20,Truncate/truncate_2_column_2_rows
18,Truncate/truncate_1_column_0_rows
18,Truncate/truncate_1_column_1_row
18,Truncate/truncate_1_column_2_rows
18,Truncate/truncate_2_column_0_rows
18,Truncate/truncate_2_column_1_rows
18,Truncate/truncate_2_column_2_rows
3,UDFResolution/select_from_udf
5,UseManyRoles/use_2_roles
3,UseManyRoles/use_50_roles
Expand Down
12 changes: 6 additions & 6 deletions pkg/ccl/benchccl/rttanalysisccl/testdata/benchmark_expectations
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ exp,benchmark
14,AlterSurvivalGoals/alter_empty_database_from_zone_to_region
24,AlterSurvivalGoals/alter_populated_database_from_region_to_zone
24,AlterSurvivalGoals/alter_populated_database_from_zone_to_region
25,AlterTableLocality/alter_from_global_to_rbr
16,AlterTableLocality/alter_from_global_to_regional_by_table
13,AlterTableLocality/alter_from_rbr_to_global
13,AlterTableLocality/alter_from_rbr_to_regional_by_table
16,AlterTableLocality/alter_from_regional_by_table_to_global
25,AlterTableLocality/alter_from_regional_by_table_to_rbr
28,AlterTableLocality/alter_from_global_to_rbr
19,AlterTableLocality/alter_from_global_to_regional_by_table
18,AlterTableLocality/alter_from_rbr_to_global
18,AlterTableLocality/alter_from_rbr_to_regional_by_table
19,AlterTableLocality/alter_from_regional_by_table_to_global
28,AlterTableLocality/alter_from_regional_by_table_to_rbr
27 changes: 15 additions & 12 deletions pkg/sql/catalog/resolver/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -378,26 +378,29 @@ func ResolveExisting(
return found, prefix, result, err
}

db, err := r.LookupDatabase(ctx, curDb)
if err != nil {
return false, prefix, nil, err
}
if curDb != "" && db == nil {
// If we have a database, and we didn't find it, then we're never going
// to find it because it must not exist. This error return path is a bit
// of a rough edge, but it preserves backwards compatibility and makes
// sure we return a database does not exist error in cases where the
// current database definitely does not exist.
return false, prefix, nil, sqlerrors.NewUndefinedDatabaseError(curDb)
}

// This is a naked object name. Use the search path.
iter := searchPath.Iter()
foundDatabase := false
for next, ok := iter.Next(); ok; next, ok = iter.Next() {
if found, prefix, result, err = r.LookupObject(
ctx, lookupFlags, curDb, next, u.Object(),
if found, prefix, result, err = r.LookupObjectInDatabase(
ctx, lookupFlags, db, next, u.Object(),
); found || err != nil {
return found, prefix, result, err
}
foundDatabase = foundDatabase || prefix.Database != nil
}

// If we have a database, and we didn't find it, then we're never going to
// find it because it must not exist. This error return path is a bit of
// a rough edge, but it preserves backwards compatibility and makes sure
// we return a database does not exist error in cases where the current
// database definitely does not exist.
if curDb != "" && !foundDatabase {
return false, prefix, nil, sqlerrors.NewUndefinedDatabaseError(curDb)
}
return false, prefix, nil, nil
}

Expand Down

0 comments on commit 0c043b9

Please sign in to comment.