Skip to content

Commit 50c74c2

Browse files
Guilherme1Rochayuzefovich
Guilherme1Rocha
authored and
yuzefovich
committed
sql: report unit for some session variables
Previously, we never populated the `unit` column of `pg_settings` virtual table deviating from PostgreSQL. We now report the unit for time-based and memory-based settings. AFAIU the unit defines the default unit of the value which allows these settings take in integer numbers with the unit providing the necessary measurement dimension. Release note: None
1 parent 8f3ba4a commit 50c74c2

File tree

5 files changed

+95
-12
lines changed

5 files changed

+95
-12
lines changed

‎pkg/sql/logictest/testdata/logic_test/pg_catalog‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3180,7 +3180,7 @@ cost_scans_with_default_col_size off N
31803180
create_table_with_schema_locked off NULL user NULL off off
31813181
database test NULL user NULL · test
31823182
datestyle ISO, MDY NULL user NULL ISO, MDY ISO, MDY
3183-
deadlock_timeout 0 NULL user NULL 0s 0s
3183+
deadlock_timeout 0 ms user NULL 0s 0s
31843184
declare_cursor_statement_timeout_enabled on NULL user NULL on on
31853185
default_int_size 8 NULL user NULL 8 8
31863186
default_table_access_method heap NULL user NULL heap heap
@@ -3231,18 +3231,18 @@ experimental_hash_group_join_enabled off N
32313231
extra_float_digits 1 NULL user NULL 1 1
32323232
force_savepoint_restart off NULL user NULL off off
32333233
foreign_key_cascades_limit 10000 NULL user NULL 10000 10000
3234-
idle_in_transaction_session_timeout 0 NULL user NULL 0s 0s
3235-
idle_session_timeout 0 NULL user NULL 0s 0s
3236-
index_join_streamer_batch_size 8.0 MiB NULL user NULL 8.0 MiB 8.0 MiB
3234+
idle_in_transaction_session_timeout 0 ms user NULL 0s 0s
3235+
idle_session_timeout 0 ms user NULL 0s 0s
3236+
index_join_streamer_batch_size 8.0 MiB B user NULL 8.0 MiB 8.0 MiB
32373237
index_recommendations_enabled off NULL user NULL on false
32383238
inject_retry_errors_enabled off NULL user NULL off off
32393239
inject_retry_errors_on_commit_enabled off NULL user NULL off off
32403240
integer_datetimes on NULL user NULL on on
32413241
intervalstyle postgres NULL user NULL postgres postgres
32423242
is_superuser on NULL user NULL on on
3243-
join_reader_index_join_strategy_batch_size 4.0 MiB NULL user NULL 4.0 MiB 4.0 MiB
3244-
join_reader_no_ordering_strategy_batch_size 2.0 MiB NULL user NULL 2.0 MiB 2.0 MiB
3245-
join_reader_ordering_strategy_batch_size 100 KiB NULL user NULL 100 KiB 100 KiB
3243+
join_reader_index_join_strategy_batch_size 4.0 MiB B user NULL 4.0 MiB 4.0 MiB
3244+
join_reader_no_ordering_strategy_batch_size 2.0 MiB B user NULL 2.0 MiB 2.0 MiB
3245+
join_reader_ordering_strategy_batch_size 100 KiB B user NULL 100 KiB 100 KiB
32463246
large_full_scan_rows 0 NULL user NULL 0 0
32473247
lc_collate C.UTF-8 NULL user NULL C.UTF-8 C.UTF-8
32483248
lc_ctype C.UTF-8 NULL user NULL C.UTF-8 C.UTF-8
@@ -3253,7 +3253,7 @@ lc_time C.UTF-8 N
32533253
legacy_varchar_typing off NULL user NULL off off
32543254
locality region=test,dc=dc1 NULL user NULL region=test,dc=dc1 region=test,dc=dc1
32553255
locality_optimized_partitioned_index_scan on NULL user NULL on on
3256-
lock_timeout 0 NULL user NULL 0s 0s
3256+
lock_timeout 0 ms user NULL 0s 0s
32573257
log_timezone UTC NULL user NULL UTC UTC
32583258
max_connections -1 NULL user NULL -1 -1
32593259
max_identifier_length 128 NULL user NULL 128 128
@@ -3305,7 +3305,7 @@ pg_trgm.similarity_threshold 0.3 N
33053305
plan_cache_mode auto NULL user NULL auto auto
33063306
plpgsql_use_strict_into off NULL user NULL off off
33073307
prefer_lookup_joins_for_fks off NULL user NULL off off
3308-
prepared_statements_cache_size 0 B NULL user NULL 0 B 0 B
3308+
prepared_statements_cache_size 0 B B user NULL 0 B 0 B
33093309
propagate_admission_header_to_leaf_transactions on NULL user NULL on on
33103310
propagate_input_ordering off NULL user NULL off off
33113311
recursion_depth_limit 1000 NULL user NULL 1000 1000
@@ -3324,7 +3324,7 @@ show_primary_key_constraint_on_not_visible_columns on N
33243324
sql_safe_updates off NULL user NULL off off
33253325
ssl on NULL user NULL on on
33263326
standard_conforming_strings on NULL user NULL on on
3327-
statement_timeout 0 NULL user NULL 0s 0s
3327+
statement_timeout 0 ms user NULL 0s 0s
33283328
streamer_always_maintain_ordering off NULL user NULL off off
33293329
streamer_enabled on NULL user NULL on on
33303330
streamer_head_of_line_only_fraction 0.8 NULL user NULL 0.8 0.8
@@ -3350,7 +3350,7 @@ transaction_rows_read_log 0 N
33503350
transaction_rows_written_err 0 NULL user NULL 0 0
33513351
transaction_rows_written_log 0 NULL user NULL 0 0
33523352
transaction_status NoTxn NULL user NULL NoTxn NoTxn
3353-
transaction_timeout 0 NULL user NULL 0s 0s
3353+
transaction_timeout 0 ms user NULL 0s 0s
33543354
troubleshooting_mode off NULL user NULL off off
33553355
unbounded_parallel_scans off NULL user NULL off off
33563356
unconstrained_non_covering_index_scan_enabled off NULL user NULL off off

‎pkg/sql/logictest/testdata/logic_test/set‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -940,3 +940,22 @@ subtest end
940940
# Regression test for incorrectly marking this variable as boolean.
941941
statement ok
942942
SET copy_num_retries_per_batch = 5;
943+
944+
# Tests for a byte-size setting where integer values are provided (which are
945+
# treated as number of bytes).
946+
statement ok
947+
SET distsql_workmem = 1048576
948+
949+
query T
950+
SHOW distsql_workmem
951+
----
952+
1.0 MiB
953+
954+
statement error distsql_workmem cannot be set to a negative value
955+
SET distsql_workmem = -1
956+
957+
statement error distsql_workmem can only be set to a value greater than 1
958+
SET distsql_workmem = 1
959+
960+
statement ok
961+
RESET distsql_workmem

‎pkg/sql/pg_catalog.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3076,6 +3076,10 @@ https://www.postgresql.org/docs/9.5/catalog-pg-settings.html`,
30763076
return err
30773077
}
30783078
valueDatum := tree.NewDString(value)
3079+
var valueUnit = tree.DNull
3080+
if gen.Unit != "" {
3081+
valueUnit = tree.NewDString(gen.Unit)
3082+
}
30793083
var bootDatum tree.Datum = tree.DNull
30803084
var resetDatum tree.Datum = tree.DNull
30813085
if gen.Set == nil && gen.RuntimeSet == nil && gen.SetWithPlanner == nil {
@@ -3099,7 +3103,7 @@ https://www.postgresql.org/docs/9.5/catalog-pg-settings.html`,
30993103
if err := addRow(
31003104
tree.NewDString(strings.ToLower(vName)), // name
31013105
valueDatum, // setting
3102-
tree.DNull, // unit
3106+
valueUnit, // unit
31033107
tree.DNull, // category
31043108
tree.DNull, // short_desc
31053109
tree.DNull, // extra_desc

‎pkg/sql/set_var.go‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
2727
"github.com/cockroachdb/errors"
2828
"github.com/cockroachdb/redact"
29+
"github.com/dustin/go-humanize"
2930
)
3031

3132
// setVarNode represents a SET {SESSION | LOCAL} statement.
@@ -368,6 +369,8 @@ func makeTimeoutVarGetter(
368369
}
369370
case *tree.DInt:
370371
timeout = time.Duration(*v) * time.Millisecond
372+
default:
373+
return "", newVarValueError(varName, values[0].String())
371374
}
372375
return timeout.String(), nil
373376
}
@@ -522,3 +525,33 @@ func newCannotChangeParameterError(varName string) error {
522525
return pgerror.Newf(pgcode.CantChangeRuntimeParam,
523526
"parameter %q cannot be changed", varName)
524527
}
528+
529+
func makeByteSizeVarGetter(
530+
varName string,
531+
) func(
532+
ctx context.Context, evalCtx *extendedEvalContext, values []tree.TypedExpr, txn *kv.Txn) (string, error) {
533+
return func(
534+
ctx context.Context, evalCtx *extendedEvalContext, values []tree.TypedExpr, txn *kv.Txn,
535+
) (string, error) {
536+
if len(values) != 1 {
537+
return "", newSingleArgVarError(varName)
538+
}
539+
d, err := eval.Expr(ctx, &evalCtx.Context, values[0])
540+
if err != nil {
541+
return "", err
542+
}
543+
544+
switch v := eval.UnwrapDatum(ctx, &evalCtx.Context, d).(type) {
545+
case *tree.DString:
546+
return string(*v), nil
547+
case *tree.DInt:
548+
size := int64(*v)
549+
if size < 0 {
550+
return "", errors.Newf("%s cannot be set to a negative value", varName)
551+
}
552+
return humanize.IBytes(uint64(size)), nil
553+
default:
554+
return "", newVarValueError(varName, values[0].String())
555+
}
556+
}
557+
}

‎pkg/sql/vars.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,11 @@ type sessionVar struct {
7777
// either by SHOW or in the pg_catalog table.
7878
Get func(evalCtx *extendedEvalContext, kv *kv.Txn) (string, error)
7979

80+
// Unit, if set, is a string representing the unit in which the value of
81+
// this session variable is expressed by default. It can be empty in which
82+
// case the unit is not defined.
83+
Unit string
84+
8085
// Exists returns true if this custom session option exists in the current
8186
// context. It's needed to support the current_setting builtin for custom
8287
// options. It is only defined for custom options.
@@ -97,6 +102,10 @@ type sessionVar struct {
97102
// the Set() method has to operate on strings, because it can be
98103
// invoked at a point where there is no evalContext yet (e.g.
99104
// upon session initialization in pgwire).
105+
//
106+
// Additional use case for this field is when we want to allow values of
107+
// multiple types to be used when setting the variable (e.g. to accept both
108+
// strings and integers).
100109
GetStringVal getStringValFn
101110

102111
// Set performs mutations to effect the change desired by SET commands.
@@ -354,6 +363,7 @@ var varGen = map[string]sessionVar{
354363
ms := evalCtx.SessionData().DeadlockTimeout.Nanoseconds() / int64(time.Millisecond)
355364
return strconv.FormatInt(ms, 10), nil
356365
},
366+
Unit: "ms",
357367
GlobalDefault: func(sv *settings.Values) string {
358368
return "0s"
359369
},
@@ -590,6 +600,8 @@ var varGen = map[string]sessionVar{
590600
Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) {
591601
return string(humanizeutil.IBytes(evalCtx.SessionData().WorkMemLimit)), nil
592602
},
603+
Unit: "B",
604+
GetStringVal: makeByteSizeVarGetter("distsql_workmem"),
593605
GlobalDefault: func(sv *settings.Values) string {
594606
return string(humanizeutil.IBytes(settingWorkMemBytes.Get(sv)))
595607
},
@@ -1260,6 +1272,7 @@ var varGen = map[string]sessionVar{
12601272
ms := evalCtx.SessionData().LockTimeout.Nanoseconds() / int64(time.Millisecond)
12611273
return strconv.FormatInt(ms, 10), nil
12621274
},
1275+
Unit: "ms",
12631276
GlobalDefault: func(sv *settings.Values) string {
12641277
return clusterLockTimeout.String(sv)
12651278
},
@@ -1498,6 +1511,7 @@ var varGen = map[string]sessionVar{
14981511
ms := evalCtx.SessionData().TransactionTimeout.Nanoseconds() / int64(time.Millisecond)
14991512
return strconv.FormatInt(ms, 10), nil
15001513
},
1514+
Unit: "ms",
15011515
GlobalDefault: func(sv *settings.Values) string {
15021516
return "0s"
15031517
},
@@ -1661,6 +1675,7 @@ var varGen = map[string]sessionVar{
16611675
ms := evalCtx.SessionData().StmtTimeout.Nanoseconds() / int64(time.Millisecond)
16621676
return strconv.FormatInt(ms, 10), nil
16631677
},
1678+
Unit: "ms",
16641679
GlobalDefault: func(sv *settings.Values) string {
16651680
return clusterStatementTimeout.String(sv)
16661681
},
@@ -1674,6 +1689,7 @@ var varGen = map[string]sessionVar{
16741689
ms := evalCtx.SessionData().IdleInSessionTimeout.Nanoseconds() / int64(time.Millisecond)
16751690
return strconv.FormatInt(ms, 10), nil
16761691
},
1692+
Unit: "ms",
16771693
GlobalDefault: func(sv *settings.Values) string {
16781694
return clusterIdleInSessionTimeout.String(sv)
16791695
},
@@ -1686,6 +1702,7 @@ var varGen = map[string]sessionVar{
16861702
ms := evalCtx.SessionData().IdleInTransactionSessionTimeout.Nanoseconds() / int64(time.Millisecond)
16871703
return strconv.FormatInt(ms, 10), nil
16881704
},
1705+
Unit: "ms",
16891706
GlobalDefault: func(sv *settings.Values) string {
16901707
return clusterIdleInTransactionSessionTimeout.String(sv)
16911708
},
@@ -2293,6 +2310,8 @@ var varGen = map[string]sessionVar{
22932310
Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) {
22942311
return string(humanizeutil.IBytes(evalCtx.SessionData().JoinReaderOrderingStrategyBatchSize)), nil
22952312
},
2313+
Unit: "B",
2314+
GetStringVal: makeByteSizeVarGetter("join_reader_ordering_strategy_batch_size"),
22962315
GlobalDefault: func(sv *settings.Values) string {
22972316
return string(humanizeutil.IBytes(rowexec.JoinReaderOrderingStrategyBatchSize.Get(sv)))
22982317
},
@@ -2314,6 +2333,8 @@ var varGen = map[string]sessionVar{
23142333
Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) {
23152334
return string(humanizeutil.IBytes(evalCtx.SessionData().JoinReaderNoOrderingStrategyBatchSize)), nil
23162335
},
2336+
Unit: "B",
2337+
GetStringVal: makeByteSizeVarGetter("join_reader_no_ordering_strategy_batch_size"),
23172338
GlobalDefault: func(sv *settings.Values) string {
23182339
return string(humanizeutil.IBytes(rowexec.JoinReaderNoOrderingStrategyBatchSize.Get(sv)))
23192340
},
@@ -2335,6 +2356,8 @@ var varGen = map[string]sessionVar{
23352356
Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) {
23362357
return string(humanizeutil.IBytes(evalCtx.SessionData().JoinReaderIndexJoinStrategyBatchSize)), nil
23372358
},
2359+
Unit: "B",
2360+
GetStringVal: makeByteSizeVarGetter("join_reader_index_join_strategy_batch_size"),
23382361
GlobalDefault: func(sv *settings.Values) string {
23392362
return string(humanizeutil.IBytes(execinfra.JoinReaderIndexJoinStrategyBatchSize.Get(sv)))
23402363
},
@@ -2356,6 +2379,8 @@ var varGen = map[string]sessionVar{
23562379
Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) {
23572380
return string(humanizeutil.IBytes(evalCtx.SessionData().IndexJoinStreamerBatchSize)), nil
23582381
},
2382+
Unit: "B",
2383+
GetStringVal: makeByteSizeVarGetter("index_join_streamer_batch_size"),
23592384
GlobalDefault: func(sv *settings.Values) string {
23602385
return string(humanizeutil.IBytes(colfetcher.IndexJoinStreamerBatchSize.Get(sv)))
23612386
},
@@ -3051,6 +3076,8 @@ var varGen = map[string]sessionVar{
30513076
Get: func(evalCtx *extendedEvalContext, _ *kv.Txn) (string, error) {
30523077
return string(humanizeutil.IBytes(evalCtx.SessionData().PreparedStatementsCacheSize)), nil
30533078
},
3079+
Unit: "B",
3080+
GetStringVal: makeByteSizeVarGetter("prepared_statements_cache_size"),
30543081
GlobalDefault: func(_ *settings.Values) string {
30553082
return string(humanizeutil.IBytes(0))
30563083
},

0 commit comments

Comments
 (0)