Skip to content

Commit e94cda6

Browse files
committed
opt: track distinct object references in metadata
Prior to this commit, all referenced object names in a query, including tables, UDTs, and UDFs, were appended to slices in a memo's metadata. These names are later re-resolved to determine if a memo is stale. Because this list could contain duplicate entries, the same name could be re-resolved multiple times during the staleness check. Now, the distinct set of reference object names are maintained, eliminating duplicate object resolution. The `tree.UnresolvedObjectNameSet` type has been added to facilitate this. Fixes #153800 Release note: None
1 parent dd67cb8 commit e94cda6

File tree

5 files changed

+165
-16
lines changed

5 files changed

+165
-16
lines changed

pkg/sql/opt/metadata.go

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ type Metadata struct {
132132
// objectRefsByName stores each unique name that the query uses to reference
133133
// each object. It is needed because changes to the search path may change
134134
// which object a given name refers to; for example, switching the database.
135-
objectRefsByName map[cat.StableID][]*tree.UnresolvedObjectName
135+
objectRefsByName map[cat.StableID]tree.UnresolvedObjectNameSet
136136

137137
// privileges stores the privileges needed to access each object that the
138138
// query depends on.
@@ -204,7 +204,7 @@ func (md *Metadata) Init() {
204204

205205
objectRefsByName := md.objectRefsByName
206206
if objectRefsByName == nil {
207-
objectRefsByName = make(map[cat.StableID][]*tree.UnresolvedObjectName)
207+
objectRefsByName = make(map[cat.StableID]tree.UnresolvedObjectNameSet)
208208
}
209209
for id := range md.objectRefsByName {
210210
delete(md.objectRefsByName, id)
@@ -306,10 +306,12 @@ func (md *Metadata) CopyFrom(from *Metadata, copyScalarFn func(Expr) Expr) {
306306

307307
for id, names := range from.objectRefsByName {
308308
if md.objectRefsByName == nil {
309-
md.objectRefsByName = make(map[cat.StableID][]*tree.UnresolvedObjectName)
309+
md.objectRefsByName = make(map[cat.StableID]tree.UnresolvedObjectNameSet)
310+
}
311+
newNames := tree.MakeUnresolvedObjectNameSet(names.Len())
312+
for i, n := 0, names.Len(); i < n; i++ {
313+
newNames.Add(names.Get(i))
310314
}
311-
newNames := make([]*tree.UnresolvedObjectName, len(names))
312-
copy(newNames, names)
313315
md.objectRefsByName[id] = newNames
314316
}
315317

@@ -371,7 +373,9 @@ func (md *Metadata) AddDependency(name MDDepName, ds cat.DataSource, priv privil
371373
md.privileges[id] = md.privileges[id] | (1 << priv)
372374
if name.byID == 0 {
373375
// This data source was referenced by name.
374-
md.objectRefsByName[id] = append(md.objectRefsByName[id], name.byName.ToUnresolvedObjectName())
376+
names := md.objectRefsByName[id]
377+
names.Add(name.byName.ToUnresolvedObjectName())
378+
md.objectRefsByName[id] = names
375379
}
376380
}
377381

@@ -465,8 +469,8 @@ func (md *Metadata) CheckDependencies(
465469
var toCheck cat.DataSource
466470
if names, ok := md.objectRefsByName[id]; ok {
467471
// The data source was referenced by name at least once.
468-
for _, name := range names {
469-
tableName := name.ToTableName()
472+
for i, n := 0, names.Len(); i < n; i++ {
473+
tableName := names.Get(i).ToTableName()
470474
toCheck, _, err = optCatalog.ResolveDataSource(ctx, cat.Flags{}, &tableName)
471475
if err != nil || !dataSource.Equals(toCheck) {
472476
return false, maybeSwallowMetadataResolveErr(err)
@@ -485,8 +489,8 @@ func (md *Metadata) CheckDependencies(
485489
for _, typ := range md.AllUserDefinedTypes() {
486490
id := cat.StableID(catid.UserDefinedOIDToID(typ.Oid()))
487491
if names, ok := md.objectRefsByName[id]; ok {
488-
for _, name := range names {
489-
toCheck, err := optCatalog.ResolveType(ctx, name)
492+
for i, n := 0, names.Len(); i < n; i++ {
493+
toCheck, err := optCatalog.ResolveType(ctx, names.Get(i))
490494
if err != nil || typ.Oid() != toCheck.Oid() ||
491495
typ.TypeMeta.Version != toCheck.TypeMeta.Version {
492496
return false, maybeSwallowMetadataResolveErr(err)
@@ -505,7 +509,8 @@ func (md *Metadata) CheckDependencies(
505509
for id, dep := range md.routineDeps {
506510
overload := dep.overload
507511
if names, ok := md.objectRefsByName[id]; ok {
508-
for _, name := range names {
512+
for i, n := 0, names.Len(); i < n; i++ {
513+
name := names.Get(i)
509514
definition, err := optCatalog.ResolveFunction(
510515
ctx, tree.MakeUnresolvedFunctionName(name.ToUnresolvedName()),
511516
&evalCtx.SessionData().SearchPath,
@@ -677,7 +682,9 @@ func (md *Metadata) AddUserDefinedType(typ *types.T, name *tree.UnresolvedObject
677682
}
678683
if name != nil {
679684
id := cat.StableID(catid.UserDefinedOIDToID(typ.Oid()))
680-
md.objectRefsByName[id] = append(md.objectRefsByName[id], name)
685+
names := md.objectRefsByName[id]
686+
names.Add(name)
687+
md.objectRefsByName[id] = names
681688
}
682689
}
683690

@@ -707,7 +714,9 @@ func (md *Metadata) AddUserDefinedRoutine(
707714
invocationTypes: invocationTypes,
708715
}
709716
if name != nil {
710-
md.objectRefsByName[id] = append(md.objectRefsByName[id], name)
717+
names := md.objectRefsByName[id]
718+
names.Add(name)
719+
md.objectRefsByName[id] = names
711720
}
712721
}
713722

@@ -1171,7 +1180,7 @@ func (md *Metadata) TestingRoutineDepsEqual(other *Metadata) bool {
11711180
}
11721181

11731182
// TestingObjectRefsByName exposes the objectRefsByName for testing.
1174-
func (md *Metadata) TestingObjectRefsByName() map[cat.StableID][]*tree.UnresolvedObjectName {
1183+
func (md *Metadata) TestingObjectRefsByName() map[cat.StableID]tree.UnresolvedObjectNameSet {
11751184
return md.objectRefsByName
11761185
}
11771186

pkg/sql/opt/metadata_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,8 @@ func TestMetadata(t *testing.T) {
182182
newNamesByID, oldNamesByID := mdNew.TestingObjectRefsByName(), md.TestingObjectRefsByName()
183183
for id, names := range oldNamesByID {
184184
newNames := newNamesByID[id]
185-
for i, name := range names {
186-
if newNames[i] != name {
185+
for i, n := 0, names.Len(); i < n; i++ {
186+
if newNames.Get(i) != names.Get(i) {
187187
t.Fatalf("expected object name to be copied")
188188
}
189189
}

pkg/sql/sem/tree/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,7 @@ go_test(
222222
"main_test.go",
223223
"name_part_test.go",
224224
"name_resolution_test.go",
225+
"object_name_test.go",
225226
"operators_test.go",
226227
"overload_test.go",
227228
"parse_array_test.go",

pkg/sql/sem/tree/object_name.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,61 @@ func (u *UnresolvedObjectName) HasExplicitCatalog() bool {
335335
return u.NumParts >= 3
336336
}
337337

338+
// equals returns true if the unresolved object name is equal to the given name,
339+
// that is, they have the same number of parts and all parts are equal.
340+
// Annotations are ignored.
341+
func (u *UnresolvedObjectName) equals(other *UnresolvedObjectName) bool {
342+
if u.NumParts != other.NumParts {
343+
return false
344+
}
345+
for i := 0; i < u.NumParts; i++ {
346+
if u.Parts[i] != other.Parts[i] {
347+
return false
348+
}
349+
}
350+
return true
351+
}
352+
353+
// UnresolvedObjectNameSet is a set of distinct unresolved object names. Two
354+
// unresolved object names are considered non-distinct if they have the same
355+
// number of parts and all parts are equal. Annotations are ignored.
356+
//
357+
// UnresolvedObjectNameSet is designed for small sets. Add is has O(n)
358+
// complexity where n is the number of names in the set and all the names in the
359+
// set must be examined, using Len and Get, to test for containment in the set.
360+
type UnresolvedObjectNameSet struct {
361+
names []*UnresolvedObjectName
362+
}
363+
364+
// MakeUnresolvedObjectNameSet creates an UnresolvedObjectNameSet with the
365+
// given initial capacity.
366+
func MakeUnresolvedObjectNameSet(cap int) UnresolvedObjectNameSet {
367+
return UnresolvedObjectNameSet{
368+
names: make([]*UnresolvedObjectName, 0, cap),
369+
}
370+
}
371+
372+
// Add adds the given name to the set. No-op if the name is already in the set.
373+
// The complexity is O(n) where n = u.Len().
374+
func (u *UnresolvedObjectNameSet) Add(name *UnresolvedObjectName) {
375+
for _, n := range u.names {
376+
if n.equals(name) {
377+
return
378+
}
379+
}
380+
u.names = append(u.names, name)
381+
}
382+
383+
// Len returns the number of names in the set.
384+
func (u *UnresolvedObjectNameSet) Len() int {
385+
return len(u.names)
386+
}
387+
388+
// Get returns the i-th name in the set. Panics if i is out of bounds.
389+
func (u *UnresolvedObjectNameSet) Get(i int) *UnresolvedObjectName {
390+
return u.names[i]
391+
}
392+
338393
// UnresolvedRoutineName is an unresolved function or procedure name. The two
339394
// implementations of this interface are used to differentiate between the two
340395
// types of routines for things like error messages.
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package tree
7+
8+
import (
9+
"testing"
10+
11+
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
12+
"github.com/cockroachdb/cockroach/pkg/util/log"
13+
)
14+
15+
func TestUnresolvedObjectNameSet(t *testing.T) {
16+
defer leaktest.AfterTest(t)()
17+
defer log.Scope(t).Close(t)
18+
19+
name := func(parts ...string) *UnresolvedObjectName {
20+
var partsArr [3]string
21+
copy(partsArr[:], parts)
22+
n, err := NewUnresolvedObjectName(len(parts), partsArr, NoAnnotation)
23+
if err != nil {
24+
t.Fatal(err)
25+
}
26+
return n
27+
}
28+
29+
testCases := []struct {
30+
add *UnresolvedObjectName
31+
expectedLen int
32+
}{
33+
{name("foo"), 1},
34+
{name("bar"), 2},
35+
{name("foo", "bar"), 3},
36+
{name("bar", "foo"), 4},
37+
{name("foo", "bar", "baz"), 5},
38+
{name("foo", "bar"), 5},
39+
{name("bar"), 5},
40+
{name("foo"), 5},
41+
{name("baz"), 6},
42+
}
43+
44+
var s UnresolvedObjectNameSet
45+
if s.Len() != 0 {
46+
t.Error("expected set to be empty")
47+
}
48+
49+
// Add each test case to the set and check the length.
50+
for _, tc := range testCases {
51+
s.Add(tc.add)
52+
if l := s.Len(); l != tc.expectedLen {
53+
t.Errorf("after adding %v, expected length of %d, got %d", tc.add, tc.expectedLen, l)
54+
}
55+
}
56+
57+
// Every name should be in the set.
58+
for _, tc := range testCases {
59+
inSet := false
60+
for i := 0; i < s.Len(); i++ {
61+
if s.Get(i).equals(tc.add) {
62+
inSet = true
63+
break
64+
}
65+
}
66+
if !inSet {
67+
t.Errorf("expected %v to be in the set", tc.add)
68+
}
69+
}
70+
71+
// There should be no names in the set that were never added.
72+
for i := 0; i < s.Len(); i++ {
73+
added := false
74+
for _, tc := range testCases {
75+
if s.Get(i).equals(tc.add) {
76+
added = true
77+
break
78+
}
79+
}
80+
if !added {
81+
t.Errorf("%v was never added to the set", s.Get(i))
82+
}
83+
}
84+
}

0 commit comments

Comments
 (0)