Skip to content

sql: ignore some session variables in RESET ALL #148770

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+144 −4
Open

sql: ignore some session variables in RESET ALL #148770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 21 additions & 0 deletions pkg/ccl/logictestccl/testdata/logic_test/read_committed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,3 +639,24 @@ statement ok
ROLLBACK

subtest end

# Verify that RESET ALL does not change transaction_isolation.

statement ok
BEGIN ISOLATION LEVEL READ COMMITTED

query T
SHOW transaction_isolation
----
read committed

statement ok
RESET ALL

query T
SHOW transaction_isolation
----
read committed

statement ok
COMMIT
1 change: 0 additions & 1 deletion pkg/sql/discard.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ func (n *discardNode) startExec(params runParams) error {

// RESET ALL
if err := params.p.resetAllSessionVars(params.ctx); err != nil {

return err
}

Expand Down
59 changes: 59 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/discard
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,3 +223,62 @@ SELECT currval('discard_seq')
query T rowsort
SELECT table_name FROM [SHOW TABLES FROM pg_temp]
----

# Check that DISCARD still works with follower reads.

query T
SET search_path = bar, public; SHOW search_path
----
bar, public

query T
SET timezone = 'Europe/Amsterdam'; SHOW timezone
----
Europe/Amsterdam

statement ok
PREPARE a AS SELECT 1

statement ok
CREATE SEQUENCE discard_seq2 START WITH 10

statement ok
CREATE TEMP TABLE tempy (a int);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think the DISCARD ALL from line 199 is breaking this, since it resets experimental_enable_temp_tables

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Exactly right. Running into a different problem now, will update in a bit.


query T rowsort
SELECT table_name FROM [SHOW TABLES FROM pg_temp]
----
tempy

statement ok
SET default_transaction_use_follower_reads = on

# DISCARD should be allowed, even with AOST set.
statement ok
DISCARD ALL

# The DISCARD ALL should have reset default_transaction_use_follower_reads.
query T
SHOW default_transaction_use_follower_reads
----
off

query T
SHOW search_path
----
"$user", public

query T
SHOW timezone
----
UTC

statement error prepared statement \"a\" does not exist
DEALLOCATE a

statement error pgcode 55000 pq: currval of sequence "test.public.discard_seq2" is not yet defined in this session
SELECT currval('discard_seq2')

query T rowsort
SELECT table_name FROM [SHOW TABLES FROM pg_temp]
----
39 changes: 39 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/reset
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,42 @@ query T
RESET time zone; SHOW TIME ZONE
----
UTC

# Verify that RESET ALL does not change transaction_read_only.

statement ok
BEGIN TRANSACTION READ ONLY

query T
SHOW transaction_read_only
----
on

statement ok
RESET ALL

query T
SHOW transaction_read_only
----
on

statement ok
COMMIT

# Verify that RESET ALL works with follower reads.

statement ok
SET default_transaction_use_follower_reads = on

query T
SHOW default_transaction_use_follower_reads
----
on

statement ok
RESET ALL

query T
SHOW default_transaction_use_follower_reads
----
off
10 changes: 10 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/set_role
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,16 @@ SHOW is_superuser
----
on

# Verify that RESET ALL does not change is_superuser.

statement ok
RESET ALL

query T
SHOW is_superuser
----
on

# Check testuser cannot transition to other users as it has no privileges.
user testuser

Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/set_var.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,8 @@ func (p *planner) resetAllSessionVars(ctx context.Context) error {
if v.Set == nil && v.RuntimeSet == nil && v.SetWithPlanner == nil {
continue
}
// For Postgres compatibility, Don't reset `role` here.
if varName == "role" {
// For Postgres compatibility, don't reset some settings here.
if v.NoResetAll {
continue
}
hasDefault, defVal := getSessionVarDefaultString(
Expand Down
14 changes: 13 additions & 1 deletion pkg/sql/vars.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ type sessionVar struct {
// Hidden indicates that the variable should not show up in the output of SHOW ALL.
Hidden bool

// NoResetAll indicates that the variable should not be reset by RESET
// ALL. (For the PG equivalent, see the GUC_NO_RESET_ALL flag in
// src/backend/utils/misc/guc_tables.c of PG source.)
NoResetAll bool

// Get returns a string representation of a given variable to be used
// either by SHOW or in the pg_catalog table.
Get func(evalCtx *extendedEvalContext, kv *kv.Txn) (string, error)
Expand Down Expand Up @@ -1214,6 +1219,7 @@ var varGen = map[string]sessionVar{
),

`is_superuser`: {
NoResetAll: true,
Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) {
return formatBoolAsPostgresSetting(evalCtx.SessionData().IsSuperuser), nil
},
Expand Down Expand Up @@ -1378,6 +1384,7 @@ var varGen = map[string]sessionVar{

// See https://www.postgresql.org/docs/current/sql-set-role.html.
`role`: {
NoResetAll: true,
Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) {
if evalCtx.SessionData().SessionUserProto == "" {
return username.NoneRole, nil
Expand Down Expand Up @@ -1618,7 +1625,8 @@ var varGen = map[string]sessionVar{
// See pg sources src/backend/utils/misc/guc.c. The variable is defined
// but is hidden from SHOW ALL.
`session_authorization`: {
Hidden: true,
Hidden: true,
NoResetAll: true,
Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) {
return evalCtx.SessionData().User().Normalized(), nil
},
Expand Down Expand Up @@ -1731,6 +1739,7 @@ var varGen = map[string]sessionVar{
// been executed yet.
// See https://github.com/postgres/postgres/blob/REL_10_STABLE/src/backend/utils/misc/guc.c#L3401-L3409
`transaction_isolation`: {
NoResetAll: true,
Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) {
level := tree.FromKVIsoLevel(evalCtx.Txn.IsoLevel())
return strings.ToLower(level.String()), nil
Expand Down Expand Up @@ -1762,20 +1771,23 @@ var varGen = map[string]sessionVar{

// CockroachDB extension.
`transaction_priority`: {
NoResetAll: true,
Get: func(evalCtx *extendedEvalContext, txn *kv.Txn) (string, error) {
return txn.UserPriority().String(), nil
},
},

// CockroachDB extension.
`transaction_status`: {
NoResetAll: true,
Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) {
return evalCtx.TxnState, nil
},
},

// See https://www.postgresql.org/docs/10/static/hot-standby.html#HOT-STANDBY-USERS
`transaction_read_only`: {
NoResetAll: true,
GetStringVal: makePostgresBoolGetStringValFn("transaction_read_only"),
Set: func(_ context.Context, m sessionDataMutator, s string) error {
b, err := paramparse.ParseBoolVar("transaction_read_only", s)
Expand Down
Loading