diff --git a/pkg/bench/ddl_analysis/ddl_analysis_bench.go b/pkg/bench/ddl_analysis/ddl_analysis_bench.go index 9cc5a257d224..88bb311f6586 100644 --- a/pkg/bench/ddl_analysis/ddl_analysis_bench.go +++ b/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) } diff --git a/pkg/bench/ddl_analysis/system_bench_test.go b/pkg/bench/ddl_analysis/system_bench_test.go new file mode 100644 index 000000000000..45c1d9e472c4 --- /dev/null +++ b/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) +} diff --git a/pkg/sql/user.go b/pkg/sql/user.go index f96c24791ea2..7718d10e5a66 100644 --- a/pkg/sql/user.go +++ b/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(