Skip to content

Commit

Permalink
schemachanger: Turn on ADD UNIQUE WITHOUT INDEX by default and add …
Browse files Browse the repository at this point in the history
…tests
  • Loading branch information
Xiang-Gu committed Jan 12, 2023
1 parent 80f9e6c commit f354bd5
Show file tree
Hide file tree
Showing 15 changed files with 612 additions and 70 deletions.
5 changes: 5 additions & 0 deletions pkg/ccl/schemachangerccl/backup_base_generated_test.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions pkg/sql/logictest/testdata/logic_test/alter_table
Original file line number Diff line number Diff line change
Expand Up @@ -1560,7 +1560,9 @@ statement ok
INSERT INTO unique_without_index (e, f) VALUES (1, 1), (1, 2)

# But trying to add a unique constraint now fails.
statement error pgcode 23505 pq: could not create unique constraint "my_unique_e"\nDETAIL: Key \(e\)=\(1\) is duplicated\.
# Note that we omit the constraint name in the expected error message because if the declarative schema changer is used,
# the constraint name, at the time of validation failure, is still a place-holder name.
statement error pgcode 23505 pq: could not create unique constraint ".*"\nDETAIL: Key \(e\)=\(1\) is duplicated\.
ALTER TABLE unique_without_index ADD CONSTRAINT my_unique_e UNIQUE WITHOUT INDEX (e)

# We can create not-valid constraints, however.
Expand Down Expand Up @@ -1603,11 +1605,14 @@ SELECT usage_count
WHERE feature_name = 'sql.schema_changer.errors.constraint_violation';

# Trying to add a partial unique constraint fails when there are duplicates.
statement error pgcode 23505 pq: could not create unique constraint "uniq_a_1"\nDETAIL: Key \(a\)=\(1\) is duplicated\.
statement error pgcode 23505 pq: could not create unique constraint ".*"\nDETAIL: Key \(a\)=\(1\) is duplicated\.
ALTER TABLE unique_without_index_partial ADD CONSTRAINT uniq_a_1 UNIQUE WITHOUT INDEX (a) WHERE b > 0 OR c > 0

# Sanity: Check the number of user errors and
# database errors in the test.
# We only do this check for the legacy schema changer because the declarative schema changer does not increment
# exactly the same counters.
onlyif config local-legacy-schema-changer
query B
SELECT usage_count > $constraint_violations_before
FROM crdb_internal.feature_usage
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/schema_changer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5627,6 +5627,7 @@ func TestTableValidityWhileAddingUniqueConstraint(t *testing.T) {
if _, err := sqlDB.Exec(`
CREATE DATABASE t;
CREATE TABLE t.tab (a INT PRIMARY KEY, b INT, c INT);
SET use_declarative_schema_changer = off;
`); err != nil {
t.Fatal(err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ var supportedAlterTableStatements = map[reflect.Type]supportedAlterTableCommand{
// Support ALTER TABLE ... ADD PRIMARY KEY
if d, ok := t.ConstraintDef.(*tree.UniqueConstraintTableDef); ok && d.PrimaryKey && t.ValidationBehavior == tree.ValidationDefault {
return true
} else if ok && d.WithoutIndex && t.ValidationBehavior == tree.ValidationDefault {
return true
}

// Support ALTER TABLE ... ADD CONSTRAINT CHECK
Expand All @@ -75,9 +77,10 @@ var supportedAlterTableStatements = map[reflect.Type]supportedAlterTableCommand{
// They key is constructed as "ADD" + constraint type + validation behavior, joined with "_".
// E.g. "ADD_PRIMARY_KEY_DEFAULT", "ADD_CHECK_SKIP", "ADD_FOREIGN_KEY_DEFAULT", etc.
var alterTableAddConstraintMinSupportedClusterVersion = map[string]clusterversion.Key{
"ADD_PRIMARY_KEY_DEFAULT": clusterversion.V22_2Start,
"ADD_CHECK_DEFAULT": clusterversion.V23_1Start,
"ADD_FOREIGN_KEY_DEFAULT": clusterversion.V23_1Start,
"ADD_PRIMARY_KEY_DEFAULT": clusterversion.V22_2Start,
"ADD_CHECK_DEFAULT": clusterversion.V23_1Start,
"ADD_FOREIGN_KEY_DEFAULT": clusterversion.V23_1Start,
"ADD_UNIQUE_WITHOUT_INDEX_DEFAULT": clusterversion.V23_1Start,
}

func init() {
Expand Down Expand Up @@ -166,6 +169,8 @@ func alterTableAddConstraintSupportedInCurrentClusterVersion(
case *tree.UniqueConstraintTableDef:
if d.PrimaryKey {
cmdKey = "ADD_PRIMARY_KEY"
} else if d.WithoutIndex {
cmdKey = "ADD_UNIQUE_WITHOUT_INDEX"
}
case *tree.CheckConstraintTableDef:
cmdKey = "ADD_CHECK"
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/schemachanger/scplan/internal/rules/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ func isIndexDependent(e scpb.Element) bool {
// when we properly support adding/dropping them in the new schema changer.
func isSupportedNonIndexBackedConstraint(e scpb.Element) bool {
switch e.(type) {
case *scpb.CheckConstraint, *scpb.ForeignKeyConstraint:
case *scpb.CheckConstraint, *scpb.ForeignKeyConstraint, *scpb.UniqueWithoutIndexConstraint:
return true
}
return false
Expand Down
Loading

0 comments on commit f354bd5

Please sign in to comment.