Skip to content

Commit

Permalink
settings: Add syntax for cluster settings
Browse files Browse the repository at this point in the history
Before this commit, there was no syntax to SET or SHOW cluster settings which
exist for a given tenant. This commit adds the following syntax:

* ALTER TENANT <id> SET CLUSTER SETTING <setting> = <value>
* ALTER TENANT ALL SET CLUSTER SETTING <setting> = <value>
* ALTER TENANT <id> RESET CLUSTER SETTING <setting>
* ALTER TENANT ALL RESET CLUSTER SETTING <setting>
* SHOW CLUSTER SETTING <setting> FOR TENANT <id>
* SHOW [ALL] CLUSTER SETTINGS FOR TENANT <id>

Note that the syntax is added but the underlying commands are currently
unimplemented. The implementation of these commands will come with a subsequent
commit.

Release note (sql change): Added syntax for modifying cluster settings at the
tenant level.
  • Loading branch information
ajstorm committed Feb 24, 2022
1 parent 9132eac commit e2d0312
Show file tree
Hide file tree
Showing 23 changed files with 596 additions and 157 deletions.
1 change: 1 addition & 0 deletions docs/generated/sql/bnf/alter_stmt.bnf
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
alter_stmt ::=
alter_ddl_stmt
| alter_role_stmt
| alter_tenant_csetting_stmt
5 changes: 5 additions & 0 deletions docs/generated/sql/bnf/alter_tenant_csetting_stmt.bnf
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
alter_tenant_csetting_stmt ::=
'ALTER' 'TENANT' iconst64 'SET' 'CLUSTER' 'SETTING' var_name to_or_eq var_value
| 'ALTER' 'TENANT' 'ALL' 'SET' 'CLUSTER' 'SETTING' var_name to_or_eq var_value
| 'ALTER' 'TENANT' iconst64 'RESET' 'CLUSTER' 'SETTING' var_name
| 'ALTER' 'TENANT' 'ALL' 'RESET' 'CLUSTER' 'SETTING' var_name
4 changes: 4 additions & 0 deletions docs/generated/sql/bnf/show_cluster_setting.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,7 @@ show_csettings_stmt ::=
| 'SHOW' 'ALL' 'CLUSTER' 'SETTINGS'
| 'SHOW' 'CLUSTER' 'SETTINGS'
| 'SHOW' 'PUBLIC' 'CLUSTER' 'SETTINGS'
| 'SHOW' 'CLUSTER' 'SETTING' var_name 'FOR' 'TENANT' iconst64
| 'SHOW' 'ALL' 'CLUSTER' 'SETTINGS' 'FOR' 'TENANT' iconst64
| 'SHOW' 'CLUSTER' 'SETTINGS' 'FOR' 'TENANT' iconst64
| 'SHOW' 'PUBLIC' 'CLUSTER' 'SETTINGS' 'FOR' 'TENANT' iconst64
49 changes: 30 additions & 19 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ close_cursor_stmt ::=
alter_stmt ::=
alter_ddl_stmt
| alter_role_stmt
| alter_tenant_csetting_stmt

backup_stmt ::=
'BACKUP' opt_backup_targets 'INTO' sconst_or_placeholder 'IN' string_or_placeholder_opt_list opt_as_of_clause opt_with_backup_options
Expand Down Expand Up @@ -409,6 +410,12 @@ alter_role_stmt ::=
| 'ALTER' 'ROLE_ALL' 'ALL' opt_in_database set_or_reset_clause
| 'ALTER' 'USER_ALL' 'ALL' opt_in_database set_or_reset_clause

alter_tenant_csetting_stmt ::=
'ALTER' 'TENANT' iconst64 'SET' 'CLUSTER' 'SETTING' var_name to_or_eq var_value
| 'ALTER' 'TENANT' 'ALL' 'SET' 'CLUSTER' 'SETTING' var_name to_or_eq var_value
| 'ALTER' 'TENANT' iconst64 'RESET' 'CLUSTER' 'SETTING' var_name
| 'ALTER' 'TENANT' 'ALL' 'RESET' 'CLUSTER' 'SETTING' var_name

opt_backup_targets ::=
targets

Expand Down Expand Up @@ -668,6 +675,10 @@ show_csettings_stmt ::=
| 'SHOW' 'ALL' 'CLUSTER' 'SETTINGS'
| 'SHOW' 'CLUSTER' 'SETTINGS'
| 'SHOW' 'PUBLIC' 'CLUSTER' 'SETTINGS'
| 'SHOW' 'CLUSTER' 'SETTING' var_name 'FOR' 'TENANT' iconst64
| 'SHOW' 'ALL' 'CLUSTER' 'SETTINGS' 'FOR' 'TENANT' iconst64
| 'SHOW' 'CLUSTER' 'SETTINGS' 'FOR' 'TENANT' iconst64
| 'SHOW' 'PUBLIC' 'CLUSTER' 'SETTINGS' 'FOR' 'TENANT' iconst64

show_databases_stmt ::=
'SHOW' 'DATABASES' with_comment
Expand Down Expand Up @@ -1406,6 +1417,18 @@ set_or_reset_clause ::=
| 'RESET_ALL' 'ALL'
| 'RESET' session_var

var_name ::=
name
| name attrs

to_or_eq ::=
'='
| 'TO'

var_value ::=
a_expr
| extra_var_value

as_of_clause ::=
'AS' 'OF' 'SYSTEM' 'TIME' a_expr

Expand Down Expand Up @@ -1576,10 +1599,6 @@ session_var ::=
| 'LC_CTYPE'
| 'TIME' 'ZONE'

var_name ::=
name
| name attrs

restore_options_list ::=
( restore_options ) ( ( ',' restore_options ) )*

Expand Down Expand Up @@ -1621,14 +1640,6 @@ set_rest_more ::=
set_rest ::=
generic_set

to_or_eq ::=
'='
| 'TO'

var_value ::=
a_expr
| extra_var_value

with_comment ::=
'WITH' 'COMMENT'
|
Expand Down Expand Up @@ -1889,6 +1900,13 @@ alter_changefeed_cmds ::=
role_options ::=
( role_option ) ( ( role_option ) )*

attrs ::=
( '.' unrestricted_name ) ( ( '.' unrestricted_name ) )*

extra_var_value ::=
'ON'
| cockroachdb_extra_reserved_keyword

backup_options ::=
'ENCRYPTION_PASSPHRASE' '=' string_or_placeholder
| 'REVISION_HISTORY'
Expand Down Expand Up @@ -2123,9 +2141,6 @@ column_name ::=
session_var_parts ::=
( '.' 'identifier' ) ( ( '.' 'identifier' ) )*

attrs ::=
( '.' unrestricted_name ) ( ( '.' unrestricted_name ) )*

restore_options ::=
'ENCRYPTION_PASSPHRASE' '=' string_or_placeholder
| 'KMS' '=' string_or_placeholder_opt_list
Expand Down Expand Up @@ -2169,10 +2184,6 @@ offset_clause ::=
generic_set ::=
var_name to_or_eq var_list

extra_var_value ::=
'ON'
| cockroachdb_extra_reserved_keyword

targets_roles ::=
'ROLE' role_spec_list
| 'SCHEMA' schema_name_list
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ go_library(
"alter_table_locality.go",
"alter_table_owner.go",
"alter_table_set_schema.go",
"alter_tenant.go",
"alter_type.go",
"analyze_expr.go",
"apply_join.go",
Expand Down
84 changes: 84 additions & 0 deletions pkg/sql/alter_tenant.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// Copyright 2022 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 sql

import (
"context"
"strings"

"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/settings"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
"github.com/cockroachdb/errors"
)

// alterTenantSetClusterSettingNode represents an
// ALTER TENANT ... SET CLUSTER SETTING statement.
type alterTenantSetClusterSettingNode struct {
name string
tenantID roachpb.TenantID
tenantAll bool
st *cluster.Settings
setting settings.NonMaskedSetting
// If value is nil, the setting should be reset.
value tree.TypedExpr
}

// AlterTenantSetClusterSetting sets tenant level session variables.
// Privileges: super user.
func (p *planner) AlterTenantSetClusterSetting(
ctx context.Context, n *tree.AlterTenantSetClusterSetting,
) (planNode, error) {
name := strings.ToLower(n.Name)
st := p.EvalContext().Settings
v, ok := settings.Lookup(name, settings.LookupForLocalAccess, p.ExecCfg().Codec.ForSystemTenant())
if !ok {
return nil, errors.Errorf("unknown cluster setting '%s'", name)
}

if err := checkPrivilegesForSetting(ctx, p, name, "set"); err != nil {
return nil, err
}

setting, ok := v.(settings.NonMaskedSetting)
if !ok {
return nil, errors.AssertionFailedf("expected writable setting, got %T", v)
}

if setting.Class() == settings.SystemOnly && !p.execCfg.Codec.ForSystemTenant() {
return nil, pgerror.Newf(pgcode.InsufficientPrivilege,
"setting %s is only settable in the system tenant", name)
}

value, err := p.getAndValidateTypedClusterSetting(ctx, name, n.Value, setting)
if err != nil {
return nil, err
}

node := alterTenantSetClusterSettingNode{
name: name, tenantID: n.TenantID, tenantAll: n.TenantAll, st: st,
setting: setting, value: value,
}
return &node, nil
}

func (n *alterTenantSetClusterSettingNode) startExec(params runParams) error {
return unimplemented.NewWithIssue(73857,
`unimplemented: tenant-level cluster settings not supported`)
}

func (n *alterTenantSetClusterSettingNode) Next(_ runParams) (bool, error) { return false, nil }
func (n *alterTenantSetClusterSettingNode) Values() tree.Datums { return nil }
func (n *alterTenantSetClusterSettingNode) Close(_ context.Context) {}
5 changes: 5 additions & 0 deletions pkg/sql/delegate/show_all_cluster_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,16 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/roleoption"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
)

func (d *delegator) delegateShowClusterSettingList(
stmt *tree.ShowClusterSettingList,
) (tree.Statement, error) {
if stmt.TenantID.IsSet() {
return nil, unimplemented.NewWithIssue(73857,
`unimplemented: tenant-level cluster settings not supported`)
}
hasModify, err := d.catalog.HasRoleOption(d.ctx, roleoption.MODIFYCLUSTERSETTING)
if err != nil {
return nil, err
Expand Down
35 changes: 35 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/cluster_settings
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ SET CLUSTER SETTING sql.conn.max_read_buffer_message_size = '1b'
statement ok
SET CLUSTER SETTING sql.conn.max_read_buffer_message_size = '64MB'

statement ok
RESET CLUSTER SETTING sql.conn.max_read_buffer_message_size

# Test permissions for modifying cluster settings.

user testuser
Expand Down Expand Up @@ -116,3 +119,35 @@ SET CLUSTER SETTING kv.snapshot_rebalance.max_rate = '10Mib'
onlyif config 3node-tenant
statement error unknown setting
SHOW CLUSTER SETTING kv.snapshot_rebalance.max_rate

subtest tenant-cluster-settings
statement error unimplemented
ALTER TENANT 10 SET CLUSTER SETTING admission.kv.enabled=true

statement error unimplemented
ALTER TENANT ALL SET CLUSTER SETTING admission.kv.enabled=true

statement error unimplemented
ALTER TENANT 10 RESET CLUSTER SETTING admission.kv.enabled

statement error unimplemented
ALTER TENANT ALL RESET CLUSTER SETTING admission.kv.enabled

statement error pq: at or near "EOF": syntax error: invalid tenant ID
ALTER TENANT 0 SET CLUSTER SETTING admission.kv.enabled=true

statement error pq: at or near "EOF": syntax error: invalid tenant ID
ALTER TENANT 0 RESET CLUSTER SETTING admission.kv.enabled

statement error unimplemented
SHOW CLUSTER SETTING admission.kv.enabled FOR TENANT 10

statement error unimplemented
SHOW CLUSTER SETTINGS FOR TENANT 10

statement error unimplemented
SHOW PUBLIC CLUSTER SETTINGS FOR TENANT 10

statement error unimplemented
SHOW ALL CLUSTER SETTINGS FOR TENANT 10

3 changes: 3 additions & 0 deletions pkg/sql/opaque.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ func planOpaque(ctx context.Context, p *planner, stmt tree.Statement) (planNode,
return p.AlterTableOwner(ctx, n)
case *tree.AlterTableSetSchema:
return p.AlterTableSetSchema(ctx, n)
case *tree.AlterTenantSetClusterSetting:
return p.AlterTenantSetClusterSetting(ctx, n)
case *tree.AlterType:
return p.AlterType(ctx, n)
case *tree.AlterRole:
Expand Down Expand Up @@ -242,6 +244,7 @@ func init() {
&tree.AlterTableLocality{},
&tree.AlterTableOwner{},
&tree.AlterTableSetSchema{},
&tree.AlterTenantSetClusterSetting{},
&tree.AlterType{},
&tree.AlterSequence{},
&tree.AlterRole{},
Expand Down
9 changes: 9 additions & 0 deletions pkg/sql/parser/help_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ func TestContextualHelp(t *testing.T) {
{`ALTER TABLE blah RENAME TO blih ??`, `ALTER TABLE`},
{`ALTER TABLE blah SPLIT AT (SELECT 1) ??`, `ALTER TABLE`},

{`ALTER TENANT 1 SET CLUSTER SETTING ???`, `ALTER TENANT`},
{`ALTER TENANT 1 RESET CLUSTER SETTING ???`, `ALTER TENANT`},
{`ALTER TENANT ALL SET CLUSTER SETTING ???`, `ALTER TENANT`},
{`ALTER TENANT ALL RESET CLUSTER SETTING ???`, `ALTER TENANT`},

{`ALTER TYPE ??`, `ALTER TYPE`},
{`ALTER TYPE t ??`, `ALTER TYPE`},
{`ALTER TYPE t ADD VALUE ??`, `ALTER TYPE`},
Expand Down Expand Up @@ -315,6 +320,10 @@ func TestContextualHelp(t *testing.T) {

{`SHOW CLUSTER SETTING all ??`, `SHOW CLUSTER SETTING`},
{`SHOW ALL CLUSTER ??`, `SHOW CLUSTER SETTING`},
{`SHOW CLUSTER SETTING a FOR TENANT ??`, `SHOW CLUSTER SETTING`},
{`SHOW ALL CLUSTER SETTINGS FOR TENANT ??`, `SHOW CLUSTER SETTING`},
{`SHOW CLUSTER SETTINGS FOR TENANT ??`, `SHOW CLUSTER SETTING`},
{`SHOW PUBLIC CLUSTER SETTINGS FOR TENANT ??`, `SHOW CLUSTER SETTING`},

{`SHOW COLUMNS FROM ??`, `SHOW COLUMNS`},
{`SHOW COLUMNS FROM foo ??`, `SHOW COLUMNS`},
Expand Down
Loading

0 comments on commit e2d0312

Please sign in to comment.