Merge pull request #58739 from rafiss/backport20.2-58671

release-20.2: sql: fix performance regression in user authn

pkg/bench/ddl_analysis/ddl_analysis_bench.go

@@ -102,14 +102,6 @@ func RunRoundTripBenchmark(b *testing.B, tests []RoundTripBenchTestCase) {
 // counting each "txn coordinator send" operation.
 func countKvBatchRequestsInRecording(r tracing.Recording) int {
 	root := r[0]
-
-	// Find the topmost "flow" span to start traversing from.
-	for _, sp := range r {
-		if sp.ParentSpanID == root.SpanID && sp.Operation == "flow" {
-			return countKvBatchRequestsInSpan(r, sp)
-		}
-	}
-
 	return countKvBatchRequestsInSpan(r, root)
 }
 

pkg/bench/ddl_analysis/system_bench_test.go

@@ -0,0 +1,41 @@
+// 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 bench
+
+import "testing"
+
+func BenchmarkSystemDatabaseQueries(b *testing.B) {
+	tests := []RoundTripBenchTestCase{
+		// This query performs 1 extra lookup since the executor first tries to
+		// lookup the name `current_db.system.users`.
+		{
+			name: "select system.users without schema name",
+			stmt: `SELECT username, "hashedPassword" FROM system.users WHERE username = 'root'`,
+		},
+		// This query performs 4 extra lookup since the executor tries to
+		// lookup the name `"".system.users`. Since the "" database doesn't exist,
+		// it also falls back to looking up that database name in the deprecated
+		// namespace table.
+		{
+			name:  "select system.users with empty database name",
+			setup: `SET sql_safe_updates = false; USE "";`,
+			stmt:  `SELECT username, "hashedPassword"  FROM system.users WHERE username = 'root'`,
+		},
+		// This query performs 2 lookups: getting the descriptor ID by name, then
+		// fetching the system table descriptor.
+		{
+			name: "select system.users with schema name",
+			stmt: `SELECT username, "hashedPassword" FROM system.public.users WHERE username = 'root'`,
+		},
+	}
+
+	RunRoundTripBenchmark(b, tests)
+}

pkg/sql/user.go

@@ -111,7 +111,8 @@ func retrieveUserAndPassword(
 
 	// Perform the lookup with a timeout.
 	err = runFn(func(ctx context.Context) error {
-		const getHashedPassword = `SELECT "hashedPassword" FROM system.users ` +
+		// Use fully qualified table name to avoid looking up "".system.users.
+		const getHashedPassword = `SELECT "hashedPassword" FROM system.public.users ` +
 			`WHERE username=$1`
 		values, err := ie.QueryRowEx(
 			ctx, "get-hashed-pwd", nil, /* txn */
@@ -131,7 +132,8 @@ func retrieveUserAndPassword(
 			return nil
 		}
 
-		getLoginDependencies := `SELECT option, value FROM system.role_options ` +
+		// Use fully qualified table name to avoid looking up "".system.role_options.
+		getLoginDependencies := `SELECT option, value FROM system.public.role_options ` +
 			`WHERE username=$1 AND option IN ('NOLOGIN', 'VALID UNTIL')`
 
 		loginDependencies, err := ie.QueryEx(