Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

schemachanger: Implement ADD UNIQUE WITHOUT INDEX #93824

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.

13 changes: 11 additions & 2 deletions pkg/sql/backfill.go
Original file line number Diff line number Diff line change
Expand Up @@ -1505,8 +1505,6 @@ func (e InvalidIndexesError) Error() string {

// ValidateConstraint validates the constraint against all rows
// in `tbl`.
//
// TODO (xiang): Support validating UNIQUE_WITHOUT_INDEX constraint in this function.
func ValidateConstraint(
ctx context.Context,
tableDesc catalog.TableDescriptor,
Expand Down Expand Up @@ -1558,6 +1556,15 @@ func ValidateConstraint(
indexIDForValidation, txn, ie)
},
)
case catconstants.ConstraintTypeUniqueWithoutIndex:
uwi := constraint.AsUniqueWithoutIndex()
return ie.WithSyntheticDescriptors(
[]catalog.Descriptor{tableDesc},
func() error {
return validateUniqueConstraint(ctx, tableDesc, uwi.GetName(), uwi.CollectKeyColumnIDs().Ordered(),
uwi.GetPredicate(), indexIDForValidation, ie, txn, sessionData.User(), false)
},
)
default:
return errors.AssertionFailedf("validation of unsupported constraint type")
}
Expand Down Expand Up @@ -2049,6 +2056,7 @@ func countIndexRowsAndMaybeCheckUniqueness(
idx.GetName(),
idx.IndexDesc().KeyColumnIDs[idx.ImplicitPartitioningColumnCount():],
idx.GetPredicate(),
0, /* indexIDForValidation */
ie,
txn,
username.NodeUserName(),
Expand Down Expand Up @@ -2737,6 +2745,7 @@ func validateUniqueWithoutIndexConstraintInTxn(
uc.Name,
uc.ColumnIDs,
uc.Predicate,
0, /* indexIDForValidation */
ie,
txn,
user,
Expand Down
14 changes: 14 additions & 0 deletions pkg/sql/catalog/rewrite/rewrite.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ func TableDescs(
}
}

// Rewrite unique_without_index in both `UniqueWithoutIndexConstraints`
// and `Mutations` slice.
origUniqueWithoutIndexConstraints := table.UniqueWithoutIndexConstraints
table.UniqueWithoutIndexConstraints = nil
for _, unique := range origUniqueWithoutIndexConstraints {
Expand All @@ -186,6 +188,18 @@ func TableDescs(
"UniqueWithoutIndexConstraint %d was not found", table.Name, unique.TableID)
}
}
for idx := range table.Mutations {
if c := table.Mutations[idx].GetConstraint(); c != nil &&
c.ConstraintType == descpb.ConstraintToUpdate_UNIQUE_WITHOUT_INDEX {
uwi := &c.UniqueWithoutIndexConstraint
if rewrite, ok := descriptorRewrites[uwi.TableID]; ok {
uwi.TableID = rewrite.ID
} else {
return errors.AssertionFailedf("cannot restore %q because referenced table ID in "+
"UniqueWithoutIndexConstraint %d was not found", table.Name, uwi.TableID)
}
}
}

if table.IsSequence() && table.SequenceOpts.HasOwner() {
if ownerRewrite, ok := descriptorRewrites[table.SequenceOpts.SequenceOwner.OwnerTableID]; ok {
Expand Down
36 changes: 32 additions & 4 deletions pkg/sql/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,8 +368,15 @@ func validateForeignKey(
// The pred argument is a partial unique constraint predicate, which filters the
// subset of rows that are guaranteed unique by the constraint. If the unique
// constraint is not partial, pred should be empty.
//
// `indexIDForValidation`, if non-zero, will be used to force the sql query to
// use this particular index by hinting the query.
func duplicateRowQuery(
srcTbl catalog.TableDescriptor, columnIDs []descpb.ColumnID, pred string, limitResults bool,
srcTbl catalog.TableDescriptor,
columnIDs []descpb.ColumnID,
pred string,
indexIDForValidation descpb.IndexID,
limitResults bool,
) (sql string, colNames []string, _ error) {
colNames, err := srcTbl.NamesForColumnIDs(columnIDs)
if err != nil {
Expand Down Expand Up @@ -397,13 +404,24 @@ func duplicateRowQuery(
if limitResults {
limit = " LIMIT 1"
}
return fmt.Sprintf(
query := fmt.Sprintf(
`SELECT %[1]s FROM [%[2]d AS tbl] WHERE %[3]s GROUP BY %[1]s HAVING count(*) > 1 %[4]s`,
strings.Join(srcCols, ", "), // 1
srcTbl.GetID(), // 2
strings.Join(srcWhere, " AND "), // 3
limit, // 4
), colNames, nil
)
if indexIDForValidation != 0 {
query = fmt.Sprintf(
`SELECT %[1]s FROM [%[2]d AS tbl]@[%[3]d] WHERE %[4]s GROUP BY %[1]s HAVING count(*) > 1 %[5]s`,
strings.Join(srcCols, ", "), // 1
srcTbl.GetID(), // 2
indexIDForValidation, // 3
strings.Join(srcWhere, " AND "), // 4
limit, // 5
)
}
return query, colNames, nil
}

// RevalidateUniqueConstraintsInCurrentDB verifies that all unique constraints
Expand Down Expand Up @@ -473,6 +491,7 @@ func (p *planner) RevalidateUniqueConstraint(
index.GetName(),
index.IndexDesc().KeyColumnIDs[index.ImplicitPartitioningColumnCount():],
index.GetPredicate(),
0, /* indexIDForValidation */
p.ExecCfg().InternalExecutor,
p.Txn(),
p.User(),
Expand All @@ -493,6 +512,7 @@ func (p *planner) RevalidateUniqueConstraint(
uc.GetName(),
uc.CollectKeyColumnIDs().Ordered(),
uc.GetPredicate(),
0, /* indexIDForValidation */
p.ExecCfg().InternalExecutor,
p.Txn(),
p.User(),
Expand Down Expand Up @@ -557,6 +577,7 @@ func RevalidateUniqueConstraintsInTable(
index.GetName(),
index.IndexDesc().KeyColumnIDs[index.ImplicitPartitioningColumnCount():],
index.GetPredicate(),
0, /* indexIDForValidation */
ie,
txn,
user,
Expand All @@ -577,6 +598,7 @@ func RevalidateUniqueConstraintsInTable(
uc.GetName(),
uc.CollectKeyColumnIDs().Ordered(),
uc.GetPredicate(),
0, /* indexIDForValidation */
ie,
txn,
user,
Expand All @@ -595,6 +617,11 @@ func RevalidateUniqueConstraintsInTable(
// validateUniqueConstraint verifies that all the rows in the srcTable
// have unique values for the given columns.
//
// `indexIDForValidation`, if non-zero, will be used to force validation
// against this particular index. This is used to facilitate the declarative
// schema changer when the validation should be against a yet non-public
// primary index.
//
// It operates entirely on the current goroutine and is thus able to
// reuse an existing kv.Txn safely.
//
Expand All @@ -606,13 +633,14 @@ func validateUniqueConstraint(
constraintName string,
columnIDs []descpb.ColumnID,
pred string,
indexIDForValidation descpb.IndexID,
ie sqlutil.InternalExecutor,
txn *kv.Txn,
user username.SQLUsername,
preExisting bool,
) error {
query, colNames, err := duplicateRowQuery(
srcTable, columnIDs, pred, true, /* limitResults */
srcTable, columnIDs, pred, indexIDForValidation, true, /* limitResults */
)
if err != nil {
return err
Expand Down
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
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ func alterTableAddConstraint(
case *tree.UniqueConstraintTableDef:
if d.PrimaryKey && t.ValidationBehavior == tree.ValidationDefault {
alterTableAddPrimaryKey(b, tn, tbl, t)
} else if d.WithoutIndex && t.ValidationBehavior == tree.ValidationDefault {
alterTableAddUniqueWithoutIndex(b, tn, tbl, t)
}
case *tree.CheckConstraintTableDef:
if t.ValidationBehavior == tree.ValidationDefault {
Expand Down Expand Up @@ -373,9 +375,110 @@ func alterTableAddForeignKey(
})
}

// getFullyResolvedColNames returns fully resolved column names.
// For each name in `colNames`, its fully resolved name will be "db.sc.tbl.col".
// The order of column names in the return is in syc with the input `colNames`.
// alterTableAddUniqueWithoutIndex contains logic for building ALTER TABLE ... ADD CONSTRAINT ... UNIQUE WITHOUT INDEX.
// It assumes `t` is such a command.
func alterTableAddUniqueWithoutIndex(
b BuildCtx, tn *tree.TableName, tbl *scpb.Table, t *tree.AlterTableAddConstraint,
) {
d := t.ConstraintDef.(*tree.UniqueConstraintTableDef)

// 1. A bunch of checks.
if !b.SessionData().EnableUniqueWithoutIndexConstraints {
panic(pgerror.New(pgcode.FeatureNotSupported,
"unique constraints without an index are not yet supported",
))
}
if len(d.Storing) > 0 {
panic(pgerror.New(pgcode.FeatureNotSupported,
"unique constraints without an index cannot store columns",
))
}
if d.PartitionByIndex.ContainsPartitions() {
panic(pgerror.New(pgcode.FeatureNotSupported,
"partitioned unique constraints without an index are not supported",
))
}
if d.NotVisible {
// Theoretically, this should never happen because this is not supported by
// the parser. This is just a safe check.
panic(pgerror.Newf(pgcode.FeatureNotSupported,
"creating a unique constraint using UNIQUE WITH NOT VISIBLE INDEX is not supported",
))
}

// 2. Check that columns that we want to have uniqueness should have no duplicate.
var colSet catalog.TableColSet
var colIDs []catid.ColumnID
var colNames []string
for _, col := range d.Columns {
colID := getColumnIDFromColumnName(b, tbl.TableID, col.Column)
if colSet.Contains(colID) {
panic(pgerror.Newf(pgcode.DuplicateColumn,
"column %q appears twice in unique constraint", col.Column))
}
colSet.Add(colID)
colIDs = append(colIDs, colID)
colNames = append(colNames, string(col.Column))
}

// 3. If a name is provided, check that this name is not used; Otherwise, generate
// a unique name for it.
if skip, err := validateConstraintNameIsNotUsed(b, tn, tbl, t); err != nil {
panic(err)
} else if skip {
return
}
if d.Name == "" {
d.Name = tree.Name(tabledesc.GenerateUniqueName(
fmt.Sprintf("unique_%s", strings.Join(colNames, "_")),
func(name string) bool {
return constraintNameInUse(b, tbl.TableID, name)
},
))
}

// 4. If there is a predicate, validate it.
if d.Predicate != nil {
predicate, _, _, err := schemaexpr.DequalifyAndValidateExprImpl(b, d.Predicate, types.Bool, "unique without index predicate", b.SemaCtx(), volatility.Immutable, tn,
func() colinfo.ResultColumns {
return getNonDropResultColumns(b, tbl.TableID)
},
func(columnName tree.Name) (exists bool, accessible bool, id catid.ColumnID, typ *types.T) {
return columnLookupFn(b, tbl.TableID, columnName)
},
)
if err != nil {
panic(err)
}
typedPredicate, err := parser.ParseExpr(predicate)
if err != nil {
panic(err)
}
d.Predicate = typedPredicate
}

// 5. (Finally!) Add a UniqueWithoutIndex, ConstraintName element to builder state.
constraintID := b.NextTableConstraintID(tbl.TableID)
uwi := &scpb.UniqueWithoutIndexConstraint{
TableID: tbl.TableID,
ConstraintID: constraintID,
ColumnIDs: colIDs,
IndexIDForValidation: getIndexIDForValidationForConstraint(b, tbl.TableID),
}
if d.Predicate != nil {
uwi.Predicate = b.WrapExpression(tbl.TableID, d.Predicate)
}
b.Add(uwi)
b.Add(&scpb.ConstraintWithoutIndexName{
TableID: tbl.TableID,
ConstraintID: constraintID,
Name: string(d.Name),
})
}

// getFullyResolvedColNames returns fully resolved column names for `colNames`.
// For each column name in `colNames`, its fully resolved name will be "db.sc.tbl.col".
// The order of column names in the return is in syc with that in the input `colNames`.
func getFullyResolvedColNames(
b BuildCtx, tableID catid.DescID, colNames tree.NameList,
) (ret tree.NameList) {
Expand Down Expand Up @@ -433,7 +536,8 @@ func validateConstraintNameIsNotUsed(
name = d.Name
ifNotExists = d.IfNotExists
case *tree.UniqueConstraintTableDef:
panic(scerrors.NotImplementedErrorf(t, "UNIQUE constraint %v not yet implemented", d.Name))
name = d.Name
ifNotExists = d.IfNotExists
default:
return false, errors.AssertionFailedf(
"unsupported constraint: %T", t.ConstraintDef)
Expand Down
Loading