From 4e01fb11deb3dd11b12da69e2b6458f2925ca94b Mon Sep 17 00:00:00 2001 From: Chengxiong Ruan Date: Wed, 17 Aug 2022 17:59:21 -0400 Subject: [PATCH 1/2] sql: add feature flag checking for UDF statements Release note: None Release justification: low risk feature flags for DDL statements. --- pkg/sql/alter_function.go | 32 +++++++++++++++++++ pkg/sql/drop_function.go | 8 +++++ .../logic_test/schema_change_feature_flags | 27 ++++++++++++++++ 3 files changed, 67 insertions(+) diff --git a/pkg/sql/alter_function.go b/pkg/sql/alter_function.go index d452b2354388..4a78414ccc32 100644 --- a/pkg/sql/alter_function.go +++ b/pkg/sql/alter_function.go @@ -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 } @@ -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 } @@ -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 } @@ -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 } diff --git a/pkg/sql/drop_function.go b/pkg/sql/drop_function.go index 8a12851bcce8..3f726044fb5d 100644 --- a/pkg/sql/drop_function.go +++ b/pkg/sql/drop_function.go @@ -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") diff --git a/pkg/sql/logictest/testdata/logic_test/schema_change_feature_flags b/pkg/sql/logictest/testdata/logic_test/schema_change_feature_flags index 852921f89f13..567b04cd4c90 100644 --- a/pkg/sql/logictest/testdata/logic_test/schema_change_feature_flags +++ b/pkg/sql/logictest/testdata/logic_test/schema_change_feature_flags @@ -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 From 2cfc958ccb801081a34013007d8125e080cc2b3a Mon Sep 17 00:00:00 2001 From: irfan sharif Date: Thu, 18 Aug 2022 11:07:36 -0400 Subject: [PATCH 2/2] roachtest: deflake kv0/enc=false/../no-admission 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 --- pkg/cmd/roachtest/tests/kv.go | 2 +- pkg/cmd/roachtest/tests/util.go | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/roachtest/tests/kv.go b/pkg/cmd/roachtest/tests/kv.go index dafcfd7700da..22c00b266400 100644 --- a/pkg/cmd/roachtest/tests/kv.go +++ b/pkg/cmd/roachtest/tests/kv.go @@ -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) diff --git a/pkg/cmd/roachtest/tests/util.go b/pkg/cmd/roachtest/tests/util.go index 1f8237cd424b..f5a2bac5ed8a 100644 --- a/pkg/cmd/roachtest/tests/util.go +++ b/pkg/cmd/roachtest/tests/util.go @@ -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) + } + } }