Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: remove round-trips from common DISCARD ALL #95876

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/bench/rttanalysis/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ go_test(
"alter_table_bench_test.go",
"bench_test.go",
"create_alter_role_bench_test.go",
"discard_bench_test.go",
"drop_bench_test.go",
"generate_objects_bench_test.go",
"grant_revoke_bench_test.go",
Expand Down
53 changes: 53 additions & 0 deletions pkg/bench/rttanalysis/discard_bench_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright 2020 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 rttanalysis

import "testing"

func BenchmarkDiscard(b *testing.B) { reg.Run(b) }
func init() {
reg.Register("Discard", []RoundTripBenchTestCase{
{
Name: "DISCARD ALL, no tables",
Setup: `DROP DATABASE IF EXISTS db CASCADE;
CREATE DATABASE db;`,
Stmt: "DISCARD ALL",
},
{
Name: "DISCARD ALL, 1 tables in 1 db",
Setup: `
SET experimental_enable_temp_tables = true;
DROP DATABASE IF EXISTS db CASCADE;
CREATE DATABASE db;
USE db;
CREATE TEMPORARY TABLE foo(i int);
`,
Stmt: "DISCARD ALL",
},
{
Name: "DISCARD ALL, 2 tables in 2 dbs",
Setup: `
SET experimental_enable_temp_tables = true;
DROP DATABASE IF EXISTS db CASCADE;
CREATE DATABASE db;
USE db;
CREATE TEMPORARY TABLE foo(i int);
CREATE TEMPORARY TABLE bar(i int);
DROP DATABASE IF EXISTS db2 CASCADE;
CREATE DATABASE db2;
USE db2;
CREATE TEMPORARY TABLE foo(i int);
CREATE TEMPORARY TABLE bar(i int);
`,
Stmt: "DISCARD ALL",
},
})
}
7 changes: 5 additions & 2 deletions pkg/bench/rttanalysis/testdata/benchmark_expectations
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ exp,benchmark
15,CreateRole/create_role_with_2_options
18,CreateRole/create_role_with_3_options
13,CreateRole/create_role_with_no_options
12,"Discard/DISCARD_ALL,_1_tables_in_1_db"
15,"Discard/DISCARD_ALL,_2_tables_in_2_dbs"
0,"Discard/DISCARD_ALL,_no_tables"
10,DropDatabase/drop_database_0_tables
11,DropDatabase/drop_database_1_table
11,DropDatabase/drop_database_2_tables
Expand All @@ -47,11 +50,11 @@ exp,benchmark
12,DropView/drop_1_view
13,DropView/drop_2_views
13,DropView/drop_3_views
5,GenerateObjects/generate_50000_tables
5,GenerateObjects/generate_1000_tables_-_this_test_should_use_the_same_number_of_RTTs_as_for_10_tables
5,GenerateObjects/generate_10_tables
12,GenerateObjects/generate_100_tables_from_template
5,GenerateObjects/generate_10_tables
16,GenerateObjects/generate_10x10_schemas_and_tables_in_existing_db
5,GenerateObjects/generate_50000_tables
9,Grant/grant_all_on_1_table
9,Grant/grant_all_on_2_tables
9,Grant/grant_all_on_3_tables
Expand Down
12 changes: 12 additions & 0 deletions pkg/sql/discard.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,26 @@ func (n *discardNode) startExec(params runParams) error {
}

func deleteTempTables(ctx context.Context, p *planner) error {
// If this session has no temp schemas, then there is nothing to do here.
// This is the common case.
if len(p.SessionData().DatabaseIDToTempSchemaID) == 0 {
return nil
}
codec := p.execCfg.Codec
descCol := p.Descriptors()
// Note: grabbing all the databases here is somewhat suspect. It appears
// that the logic related to maintaining the set of database temp schemas
// is somewhat incomplete, so there can be temp schemas in the sessiondata
// map which don't exist any longer.
allDbDescs, err := descCol.GetAllDatabaseDescriptors(ctx, p.Txn())
if err != nil {
return err
}
g := p.byNameGetterBuilder().MaybeGet()
for _, db := range allDbDescs {
if _, ok := p.SessionData().DatabaseIDToTempSchemaID[uint32(db.GetID())]; !ok {
continue
}
sc, err := g.Schema(ctx, db, p.TemporarySchemaName())
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ func (p *planner) setRole(ctx context.Context, local bool, s username.SQLUsernam
sessionUser := p.SessionData().SessionUser()
becomeUser := sessionUser
// Check the role exists - if so, populate becomeUser.
if !s.IsNoneRole() {
if !s.IsNoneRole() && s != sessionUser {
becomeUser = s

exists, err := p.RoleExists(ctx, becomeUser)
Expand Down