-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
107815: sql/schemachanger: disable CREATE SEQUENCE / CREATE SCHEMA by default in declarative schema changer r=fqazi a=fqazi For 23.2 and keeping things more conservative, we will keep CREATE SEQUENCE / CREATE SCHEMA running in the older declarative schema changer. This patch will do the following: 1. CREATE SEQUENCE / CREATE SCHEMA are disabled by default 2. A new cluster setting sql.schema.force_declarative_statements can now be used to enable/disable declarative features in a more granular matter. Fixes: #107734 Co-authored-by: Faizan Qazi <[email protected]>
- Loading branch information
Showing
16 changed files
with
319 additions
and
86 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
53 changes: 53 additions & 0 deletions
53
pkg/sql/schemachanger/scbuild/internal/scbuildstmt/process_test.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| // Copyright 2023 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 scbuildstmt | ||
|
|
||
| import ( | ||
| "reflect" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/cockroachdb/cockroach/pkg/settings" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestSupportedStatements(t *testing.T) { | ||
| sv := &settings.Values{} | ||
| // Non-existent tags should error out. | ||
| require.Error(t, schemaChangerDisabledStatements.Validate(sv, "FAKE STATEMENT")) | ||
| // Generate the full set of statements | ||
| allTags := strings.Builder{} | ||
| noTags := strings.Builder{} | ||
| first := true | ||
| for typ, stmt := range supportedStatements { | ||
| require.Greaterf(t, len(stmt.statementTag), 0, "statement tag is missing %v %v", typ, stmt) | ||
| // Validate tags matches the statement tag | ||
| typTag, found := typ.MethodByName("StatementTag") | ||
| require.True(t, found, "unable to find stmt: %v %v", typ, stmt) | ||
| ret := typTag.Func.Call([]reflect.Value{reflect.New(typ.Elem())}) | ||
| require.Equal(t, ret[0].String(), stmt.statementTag, "statement tag is different in AST") | ||
| // Validate all tags are supported. | ||
| require.NoError(t, schemaChangerDisabledStatements.Validate(sv, "+"+stmt.statementTag)) | ||
| require.NoError(t, schemaChangerDisabledStatements.Validate(sv, "!"+stmt.statementTag)) | ||
| // Validate all of them can be specified at once. | ||
| if !first { | ||
| allTags.WriteString(",") | ||
| noTags.WriteString(",") | ||
| } | ||
| first = false | ||
| allTags.WriteString("+") | ||
| allTags.WriteString(stmt.statementTag) | ||
| noTags.WriteString("!") | ||
| noTags.WriteString(stmt.statementTag) | ||
| } | ||
| require.NoError(t, schemaChangerDisabledStatements.Validate(sv, allTags.String())) | ||
| require.NoError(t, schemaChangerDisabledStatements.Validate(sv, noTags.String())) | ||
| } |
100 changes: 100 additions & 0 deletions
100
pkg/sql/schemachanger/scbuild/internal/scbuildstmt/statement_control.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| // Copyright 2023 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 scbuildstmt | ||
|
|
||
| import ( | ||
| "strings" | ||
|
|
||
| "github.com/cockroachdb/cockroach/pkg/settings" | ||
| "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scerrors" | ||
| "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" | ||
| "github.com/cockroachdb/errors" | ||
| ) | ||
|
|
||
| // schemaStatementControl track if a statement tag is enabled or disabled | ||
| // forcefully by the user. | ||
| type schemaStatementControl map[string]bool | ||
|
|
||
| // schemaChangerDisabledStatements statements which are disabled | ||
| // for the declarative schema changer. Users can specify statement | ||
| // tags for each statement and a "!" symbol in front can have the opposite | ||
| // effect to force enable fully unimplemented features. | ||
| var schemaChangerDisabledStatements = func() *settings.StringSetting { | ||
| return settings.RegisterValidatedStringSetting( | ||
| settings.TenantWritable, | ||
| "sql.schema.force_declarative_statements", | ||
| "allows force enabling / disabling declarative schema changer for specific statements", | ||
| "", | ||
| func(values *settings.Values, s string) error { | ||
| if s == "" { | ||
| return nil | ||
| } | ||
| // First split the string into individual tags. | ||
| tags := strings.Split(s, ",") | ||
| for _, tag := range tags { | ||
| tag = strings.ToUpper(strings.TrimSpace(tag)) | ||
| if len(tag) > 0 && (tag[0] == '+' || tag[0] == '!') { | ||
| tag = tag[1:] | ||
| } else { | ||
| return errors.Errorf("tag is not properly formatted, must start with '+' or '!' (%s)", tag) | ||
| } | ||
| if _, ok := supportedStatementTags[tag]; !ok { | ||
| return errors.Errorf("statement tag %q is not controlled by the declarative schema changer", tag) | ||
| } | ||
| } | ||
| return nil | ||
| }) | ||
| }() | ||
|
|
||
| // CheckStatementControl if a statement is forced to disabled or enabled. If a | ||
| // statement is disabled then an not implemented error will be panicked. Otherwise, | ||
| // a flag is returned indicating if this statement has been *forced* to be enabled. | ||
| func (c schemaStatementControl) CheckStatementControl(n tree.Statement) (forceEnabled bool) { | ||
| // This map is only created *if* any force flags are set. | ||
| if c == nil { | ||
| return false | ||
| } | ||
| enabledOrDisabled, found := c[n.StatementTag()] | ||
| if !found { | ||
| return false | ||
| } | ||
| if !enabledOrDisabled { | ||
| panic(scerrors.NotImplementedErrorf(n, | ||
| "statement has been disabled via cluster setting")) | ||
| } | ||
| return enabledOrDisabled | ||
| } | ||
|
|
||
| // GetSchemaChangerStatementControl returns a map of statements that | ||
| // are explicitly disabled by administrators for the declarative schema | ||
| // changer. | ||
| func getSchemaChangerStatementControl(sv *settings.Values) schemaStatementControl { | ||
| statements := schemaChangerDisabledStatements.Get(sv) | ||
| var statementMap schemaStatementControl | ||
| for _, tag := range strings.Split(statements, ",") { | ||
| tag = strings.ToUpper(strings.TrimSpace(tag)) | ||
| if len(tag) == 0 { | ||
| continue | ||
| } | ||
| enabledOrDisabled := true | ||
| tagStart := tag[0] | ||
| tag = tag[1:] | ||
| // If the tag starts with a ! its disabled. | ||
| if tagStart == '!' { | ||
| enabledOrDisabled = false | ||
| } | ||
| if statementMap == nil { | ||
| statementMap = make(schemaStatementControl) | ||
| } | ||
| statementMap[tag] = enabledOrDisabled | ||
| } | ||
| return statementMap | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.