Skip to content

Commit

Permalink
sql: fix current_setting(..., true) for custom options
Browse files Browse the repository at this point in the history
Release note (bug fix): The `current_setting` builtin function now
properly does not result in an error when checking a custom session
setting that does not exist and the `missing_ok` argument is true.
  • Loading branch information
rafiss committed Sep 19, 2022
1 parent 48654d6 commit e6d8c31
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 0 deletions.
28 changes: 28 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/pg_builtins
Expand Up @@ -268,6 +268,34 @@ SELECT current_setting('statement_timeout'), current_setting('search_path')
query error unrecognized configuration parameter
SELECT pg_catalog.current_setting('woo', false)

# Check that current_setting handles custom settings correctly.
query T
SELECT current_setting('my.custom', true)
----
NULL

statement ok
PREPARE check_custom AS SELECT current_setting('my.custom', true)

query T
EXECUTE check_custom
----
NULL

statement ok
BEGIN;
SET LOCAL my.custom = 'foo'

# Check that the existence of my.custom is checked depending on the execution
# context, and not at PREPARE time.
query T
EXECUTE check_custom
----
foo

statement ok
COMMIT

# check error on unsupported session var.
query error configuration setting.*not supported
SELECT current_setting('vacuum_cost_delay', false)
Expand Down
14 changes: 14 additions & 0 deletions pkg/sql/vars.go
Expand Up @@ -79,6 +79,11 @@ type sessionVar struct {
// either by SHOW or in the pg_catalog table.
Get func(evalCtx *extendedEvalContext, kv *kv.Txn) (string, error)

// Exists returns true if this custom session option exists in the current
// context. It's needed to support the current_setting builtin for custom
// options. It is only defined for custom options.
Exists func(evalCtx *extendedEvalContext, kv *kv.Txn) bool

// GetFromSessionData returns a string representation of a given variable to
// be used by BufferParamStatus. This is only required if the variable
// is expected to send updates through ParamStatusUpdate in pgwire.
Expand Down Expand Up @@ -2607,6 +2612,10 @@ func getCustomOptionSessionVar(varName string) (sv sessionVar, isCustom bool) {
}
return v, nil
},
Exists: func(evalCtx *extendedEvalContext, _ *kv.Txn) bool {
_, ok := evalCtx.SessionData().CustomOptions[varName]
return ok
},
Set: func(ctx context.Context, m sessionDataMutator, val string) error {
// TODO(#72026): do some memory accounting.
m.SetCustomOption(varName, val)
Expand All @@ -2629,6 +2638,11 @@ func (p *planner) GetSessionVar(
if err != nil || !ok {
return ok, "", err
}
if existsFn := v.Exists; existsFn != nil {
if missingOk && !existsFn(&p.extendedEvalCtx, p.Txn()) {
return false, "", nil
}
}
val, err := v.Get(&p.extendedEvalCtx, p.Txn())
return true, val, err
}
Expand Down

0 comments on commit e6d8c31

Please sign in to comment.