sql/catalog/resolver: remove an allocation, add a benchmark

pkg/sql/catalog/resolver/BUILD.bazel

@@ -27,9 +27,18 @@ go_library(
 
 go_test(
     name = "resolver_test",
-    srcs = ["resolver_test.go"],
+    srcs = [
+        "main_test.go",
+        "resolver_bench_test.go",
+        "resolver_test.go",
+    ],
     deps = [
         ":resolver",
+        "//pkg/base",
+        "//pkg/security",
+        "//pkg/security/securitytest",
+        "//pkg/server",
+        "//pkg/sql",
         "//pkg/sql/catalog",
         "//pkg/sql/catalog/catconstants",
         "//pkg/sql/catalog/dbdesc",
@@ -39,8 +48,15 @@ go_test(
         "//pkg/sql/parser",
         "//pkg/sql/sem/tree",
         "//pkg/sql/sessiondata",
+        "//pkg/sql/sessiondatapb",
         "//pkg/testutils",
+        "//pkg/testutils/serverutils",
+        "//pkg/testutils/sqlutils",
+        "//pkg/testutils/testcluster",
         "//pkg/util/leaktest",
         "//pkg/util/log",
+        "//pkg/util/protoutil",
+        "//pkg/util/randutil",
+        "@com_github_stretchr_testify//require",
     ],
 )

pkg/sql/catalog/resolver/main_test.go

@@ -0,0 +1,33 @@
+// Copyright 2021 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package resolver_test
+
+import (
+	"os"
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/security"
+	"github.com/cockroachdb/cockroach/pkg/security/securitytest"
+	"github.com/cockroachdb/cockroach/pkg/server"
+	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
+	"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
+	"github.com/cockroachdb/cockroach/pkg/util/randutil"
+)
+
+//go:generate ../../../util/leaktest/add-leaktest.sh *_test.go
+
+func TestMain(m *testing.M) {
+	security.SetAssetLoader(securitytest.EmbeddedAssets)
+	randutil.SeedForTests()
+	serverutils.InitTestServerFactory(server.TestServerFactory)
+	serverutils.InitTestClusterFactory(testcluster.TestClusterFactory)
+	os.Exit(m.Run())
+}

pkg/sql/catalog/resolver/resolver.go

@@ -215,19 +215,22 @@ func ResolveExistingObject(
 		}
 		return nil, prefix, nil
 	}
-	resolvedTn := tree.MakeTableNameFromPrefix(prefix.NamePrefix(), tree.Name(un.Object()))
+	getResolvedTn := func() *tree.TableName {
+		tn := tree.MakeTableNameFromPrefix(prefix.NamePrefix(), tree.Name(un.Object()))
+		return &tn
+	}
 
 	switch lookupFlags.DesiredObjectKind {
 	case tree.TypeObject:
 		typ, isType := obj.(catalog.TypeDescriptor)
 		if !isType {
-			return nil, prefix, sqlerrors.NewUndefinedTypeError(&resolvedTn)
+			return nil, prefix, sqlerrors.NewUndefinedTypeError(getResolvedTn())
 		}
 		return typ, prefix, nil
 	case tree.TableObject:
 		table, ok := obj.(catalog.TableDescriptor)
 		if !ok {
-			return nil, prefix, sqlerrors.NewUndefinedRelationError(&resolvedTn)
+			return nil, prefix, sqlerrors.NewUndefinedRelationError(getResolvedTn())
 		}
 		goodType := true
 		switch lookupFlags.DesiredTableDescKind {
@@ -241,7 +244,7 @@ func ResolveExistingObject(
 			goodType = table.IsSequence()
 		}
 		if !goodType {
-			return nil, prefix, sqlerrors.NewWrongObjectTypeError(&resolvedTn, lookupFlags.DesiredTableDescKind.String())
+			return nil, prefix, sqlerrors.NewWrongObjectTypeError(getResolvedTn(), lookupFlags.DesiredTableDescKind.String())
 		}
 
 		// If the table does not have a primary key, return an error

pkg/sql/catalog/resolver/resolver_bench_test.go

@@ -0,0 +1,114 @@
+// Copyright 2021 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package resolver_test
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/base"
+	"github.com/cockroachdb/cockroach/pkg/security"
+	"github.com/cockroachdb/cockroach/pkg/sql"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/resolver"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
+	"github.com/cockroachdb/cockroach/pkg/sql/sessiondatapb"
+	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
+	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
+	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
+	"github.com/stretchr/testify/require"
+)
+
+// BenchmarkResolveExistingObject exercises resolver code to ensure that
+func BenchmarkResolveExistingObject(b *testing.B) {
+	defer leaktest.AfterTest(b)()
+	defer log.Scope(b).Close(b)
+	for _, tc := range []struct {
+		setup      []string
+		name       tree.UnresolvedName
+		flags      tree.ObjectLookupFlags
+		searchPath string
+	}{
+		{
+			setup: []string{
+				"CREATE TABLE foo ()",
+			},
+			name:  tree.MakeUnresolvedName("foo"),
+			flags: tree.ObjectLookupFlagsWithRequired(),
+		},
+		{
+			setup: []string{
+				"CREATE SCHEMA sc",
+				"CREATE TABLE sc.foo ()",
+			},
+			name:  tree.MakeUnresolvedName("sc", "foo"),
+			flags: tree.ObjectLookupFlagsWithRequired(),
+		},
+		{
+			setup: []string{
+				"CREATE SCHEMA sc",
+				"CREATE TABLE sc.foo ()",
+			},
+			name:       tree.MakeUnresolvedName("foo"),
+			flags:      tree.ObjectLookupFlagsWithRequired(),
+			searchPath: "public,$user,sc",
+		},
+	} {
+		b.Run(strings.Join(tc.setup, ";")+tc.name.String(), func(b *testing.B) {
+			ctx := context.Background()
+			s, sqlDB, kvDB := serverutils.StartServer(b, base.TestServerArgs{})
+			defer s.Stopper().Stop(ctx)
+			tDB := sqlutils.MakeSQLRunner(sqlDB)
+			for _, stmt := range tc.setup {
+				tDB.Exec(b, stmt)
+			}
+
+			var sessionData sessiondatapb.SessionData
+			{
+				var sessionSerialized []byte
+				tDB.QueryRow(b, "SELECT crdb_internal.serialize_session()").Scan(&sessionSerialized)
+				require.NoError(b, protoutil.Unmarshal(sessionSerialized, &sessionData))
+			}
+
+			execCfg := s.ExecutorConfig().(sql.ExecutorConfig)
+			txn := kvDB.NewTxn(ctx, "test")
+			p, cleanup := sql.NewInternalPlanner("asdf", txn, security.RootUserName(), &sql.MemoryMetrics{}, &execCfg, sessionData)
+			defer cleanup()
+
+			// The internal planner overrides the database to "system", here we
+			// change it back.
+			{
+				ec := p.(interface{ EvalContext() *tree.EvalContext }).EvalContext()
+				require.NoError(b, ec.SessionAccessor.SetSessionVar(
+					ctx, "database", "defaultdb", false,
+				))
+
+				if tc.searchPath != "" {
+					require.NoError(b, ec.SessionAccessor.SetSessionVar(
+						ctx, "search_path", tc.searchPath, false,
+					))
+				}
+			}
+
+			rs := p.(resolver.SchemaResolver)
+			uon, err := tc.name.ToUnresolvedObjectName(0)
+			require.NoError(b, err)
+			b.ResetTimer()
+			for i := 0; i < b.N; i++ {
+				desc, _, err := resolver.ResolveExistingObject(ctx, rs, uon, tc.flags)
+				require.NoError(b, err)
+				require.NotNil(b, desc)
+			}
+		})
+	}
+}