Skip to content

Commit

Permalink
Merge #86342 #86388
Browse files Browse the repository at this point in the history
86342: sql: add feature flag checking for UDF statements r=chengxiong-ruan a=chengxiong-ruan

Release note: None
Release justification: low risk feature flags for DDL statements.

86388: roachtest: deflake kv0/enc=false/../no-admission r=irfansharif a=irfansharif

Fixes #86299 (speculatively). It doesn't seem this perf test cared about
failing the entire test run in the face of errors (the errors tripping
the roachtest were as a result of circuit breakers tripping, possible
without admission control which this test disabled). While here, we also
switch off admission.kv.pause_replication_io_threshold for
configurations where admission is disabled -- it gives us a fuller
comparison point.

Release justification: stability work
Release note: None

Co-authored-by: Chengxiong Ruan <[email protected]>
Co-authored-by: irfan sharif <[email protected]>
  • Loading branch information
3 people committed Aug 18, 2022
3 parents 04c6a1a + 4e01fb1 + 2cfc958 commit 2cdb9e2
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/tests/kv.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func registerKV(r registry.Registry) {
envFlags = " " + e
}

cmd := fmt.Sprintf("./workload run kv --init"+
cmd := fmt.Sprintf("./workload run kv --tolerate-errors --init"+
histograms+concurrency+splits+duration+readPercent+batchSize+blockSize+sequential+envFlags+
" {pgurl:1-%d}", nodes)
c.Run(ctx, c.Node(nodes+1), cmd)
Expand Down
6 changes: 6 additions & 0 deletions pkg/cmd/roachtest/tests/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,4 +118,10 @@ func SetAdmissionControl(ctx context.Context, t test.Test, c cluster.Cluster, en
t.Fatalf("failed to set admission control to %t: %v", enabled, err)
}
}
if !enabled {
if _, err := db.ExecContext(
ctx, "SET CLUSTER SETTING admission.kv.pause_replication_io_threshold = 0.0"); err != nil {
t.Fatalf("failed to set admission control to %t: %v", enabled, err)
}
}
}
32 changes: 32 additions & 0 deletions pkg/sql/alter_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ type alterFunctionDepExtensionNode struct {
func (p *planner) AlterFunctionOptions(
ctx context.Context, n *tree.AlterFunctionOptions,
) (planNode, error) {
if err := checkSchemaChangeEnabled(
ctx,
p.ExecCfg(),
"ALTER FUNCTION",
); err != nil {
return nil, err
}

return &alterFunctionOptionsNode{n: n}, nil
}

Expand Down Expand Up @@ -90,6 +98,14 @@ func (n *alterFunctionOptionsNode) Close(ctx context.Context) {}
func (p *planner) AlterFunctionRename(
ctx context.Context, n *tree.AlterFunctionRename,
) (planNode, error) {
if err := checkSchemaChangeEnabled(
ctx,
p.ExecCfg(),
"ALTER FUNCTION",
); err != nil {
return nil, err
}

return &alterFunctionRenameNode{n: n}, nil
}

Expand Down Expand Up @@ -141,6 +157,14 @@ func (n *alterFunctionRenameNode) Close(ctx context.Context) {}
func (p *planner) AlterFunctionSetOwner(
ctx context.Context, n *tree.AlterFunctionSetOwner,
) (planNode, error) {
if err := checkSchemaChangeEnabled(
ctx,
p.ExecCfg(),
"ALTER FUNCTION",
); err != nil {
return nil, err
}

return &alterFunctionSetOwnerNode{n: n}, nil
}

Expand Down Expand Up @@ -181,6 +205,14 @@ func (n *alterFunctionSetOwnerNode) Close(ctx context.Context) {}
func (p *planner) AlterFunctionSetSchema(
ctx context.Context, n *tree.AlterFunctionSetSchema,
) (planNode, error) {
if err := checkSchemaChangeEnabled(
ctx,
p.ExecCfg(),
"ALTER FUNCTION",
); err != nil {
return nil, err
}

return &alterFunctionSetSchemaNode{n: n}, nil
}

Expand Down
8 changes: 8 additions & 0 deletions pkg/sql/drop_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ type dropFunctionNode struct {
func (p *planner) DropFunction(
ctx context.Context, n *tree.DropFunction,
) (ret planNode, err error) {
if err := checkSchemaChangeEnabled(
ctx,
p.ExecCfg(),
"DROP FUNCTION",
); err != nil {
return nil, err
}

if n.DropBehavior == tree.DropCascade {
// TODO(chengxiong): remove this check when drop function cascade is supported.
return nil, unimplemented.Newf("DROP FUNCTION...CASCADE", "drop function cascade not supported")
Expand Down
27 changes: 27 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/schema_change_feature_flags
Original file line number Diff line number Diff line change
Expand Up @@ -310,3 +310,30 @@ COMMENT ON TABLE t IS 'comment'
# Reset feature flag to true so that test objects can be dropped.
statement ok
SET CLUSTER SETTING feature.schema_change.enabled = TRUE

statement ok
SET CLUSTER SETTING feature.schema_change.enabled = FALSE

statement error pq: feature CREATE FUNCTION is part of the schema change category, which was disabled by the database administrator
CREATE FUNCTION f() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$;

statement error pq: feature CREATE FUNCTION is part of the schema change category, which was disabled by the database administrator
CREATE OR REPLACE FUNCTION f() RETURNS INT LANGUAGE SQL AS $$ SELECT 1 $$;

statement error pq: feature DROP FUNCTION is part of the schema change category, which was disabled by the database administrator
DROP FUNCTION f()

statement error pq: feature ALTER FUNCTION is part of the schema change category, which was disabled by the database administrator
ALTER FUNCTION f() IMMUTABLE;

statement error pq: feature ALTER FUNCTION is part of the schema change category, which was disabled by the database administrator
ALTER FUNCTION f() RENAME TO g;

statement error pq: feature ALTER FUNCTION is part of the schema change category, which was disabled by the database administrator
ALTER FUNCTION f() OWNER TO new_owner;

statement error pq: feature ALTER FUNCTION is part of the schema change category, which was disabled by the database administrator
ALTER FUNCTION f() SET SCHEMA new_schema;

statement ok
SET CLUSTER SETTING feature.schema_change.enabled = TRUE

0 comments on commit 2cdb9e2

Please sign in to comment.