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.

Sign up for GitHub

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

sql: add checks after all referenced columns have been backfilled #35300

Merged
merged 1 commit into from
Mar 13, 2019
Merged

sql: add checks after all referenced columns have been backfilled #35300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 2 additions & 9 deletions pkg/sql/alter_table.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,7 @@ func (n *alterTableNode) startExec(params runParams) error {
return err
}
ck.Validity = sqlbase.ConstraintValidity_Validating
n.tableDesc.Checks = append(n.tableDesc.Checks, ck)
descriptorChanged = true

n.tableDesc.AddCheckValidationMutation(ck.Name)
n.tableDesc.AddCheckValidationMutation(ck)

case *tree.ForeignKeyConstraintTableDef:
for _, colName := range d.FromCols {
Expand Down Expand Up @@ -399,7 +396,7 @@ func (n *alterTableNode) startExec(params runParams) error {

// Drop check constraints which reference the column.
validChecks := n.tableDesc.Checks[:0]
for _, check := range n.tableDesc.Checks {
for _, check := range n.tableDesc.AllActiveAndInactiveChecks() {
if used, err := check.UsesColumn(n.tableDesc.TableDesc(), col.ID); err != nil {
return err
} else if used {
Expand Down Expand Up @@ -482,10 +479,6 @@ func (n *alterTableNode) startExec(params runParams) error {
}
}
if !found {
panic("constraint returned by GetConstraintInfo not found")
}

if n.tableDesc.Checks[idx].Validity == sqlbase.ConstraintValidity_Validating {
return fmt.Errorf("constraint %q in the middle of being added, try again later", t.Constraint)
}

Expand Down
70 changes: 64 additions & 6 deletions pkg/sql/backfill.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ func (sc *SchemaChanger) runBackfill(
var droppedIndexDescs []sqlbase.IndexDescriptor
var addedIndexDescs []sqlbase.IndexDescriptor

var checksToValidate []sqlbase.ConstraintToValidate
var addedChecks []*sqlbase.TableDescriptor_CheckConstraint
var checksToValidate []sqlbase.ConstraintToUpdate

var tableDesc *sqlbase.TableDescriptor
if err := sc.db.Txn(ctx, func(ctx context.Context, txn *client.Txn) error {
Expand Down Expand Up @@ -160,7 +161,8 @@ func (sc *SchemaChanger) runBackfill(
addedIndexDescs = append(addedIndexDescs, *t.Index)
case *sqlbase.DescriptorMutation_Constraint:
switch t.Constraint.ConstraintType {
case sqlbase.ConstraintToValidate_CHECK:
case sqlbase.ConstraintToUpdate_CHECK:
addedChecks = append(addedChecks, &t.Constraint.Check)
checksToValidate = append(checksToValidate, *t.Constraint)
default:
return errors.Errorf("unsupported constraint type: %d", t.Constraint.ConstraintType)
Expand All @@ -178,14 +180,19 @@ func (sc *SchemaChanger) runBackfill(
droppedIndexDescs = append(droppedIndexDescs, *t.Index)
}
case *sqlbase.DescriptorMutation_Constraint:
// Only possible during a rollback
if !m.Rollback {
return errors.Errorf(
"trying to drop constraint through schema changer outside of a rollback: %+v", t)
}
// no-op
default:
return errors.Errorf("unsupported mutation: %+v", m)
}
}
}

// First drop indexes, then add/drop columns, and only then add indexes.
// First drop indexes, then add/drop columns, and only then add indexes and constraints.

// Drop indexes not to be removed by `ClearRange`.
if len(droppedIndexDescs) > 0 {
Expand All @@ -209,6 +216,21 @@ func (sc *SchemaChanger) runBackfill(
}
}

// Add check constraints, publish the new version of the table descriptor,
// and wait until the entire cluster is on the new version. This is basically
// a state transition for the schema change, which must happen after the
// columns are backfilled and before constraint validation begins. This
// ensures that 1) all columns are writable and backfilled when the constraint
// starts being enforced on insert/update (which is relevant in the case where
// a constraint references both public and non-public columns), and 2) the
// validation occurs only when the entire cluster is already enforcing the
// constraint on insert/update.
if len(addedChecks) > 0 {
if err := sc.addChecks(ctx, addedChecks); err != nil {
return err
}
}

thoszhang marked this conversation as resolved.
Show resolved Hide resolved
// Validate check constraints.
if len(checksToValidate) > 0 {
if err := sc.validateChecks(ctx, evalCtx, lease, checksToValidate); err != nil {
Expand All @@ -218,11 +240,46 @@ func (sc *SchemaChanger) runBackfill(
return nil
}

// addChecks publishes a new version of the given table descriptor with the
// given check constraint added to it, and waits until the entire cluster is on
// the new version of the table descriptor.
func (sc *SchemaChanger) addChecks(
ctx context.Context, addedChecks []*sqlbase.TableDescriptor_CheckConstraint,
) error {
_, err := sc.leaseMgr.Publish(ctx, sc.tableID,
func(desc *sqlbase.MutableTableDescriptor) error {
for i, added := range addedChecks {
found := false
for _, c := range desc.Checks {
if c.Name == added.Name {
log.VEventf(
ctx, 2,
"backfiller tried to add constraint %+v but found existing constraint %+v, presumably due to a retry",
added, c,
)
found = true
break
}
}
if !found {
desc.Checks = append(desc.Checks, addedChecks[i])
}
}
return nil
},
nil,
)
if err != nil {
return err
}
return sc.waitToUpdateLeases(ctx, sc.tableID)
}

func (sc *SchemaChanger) validateChecks(
ctx context.Context,
evalCtx *extendedEvalContext,
lease *sqlbase.TableDescriptor_SchemaChangeLease,
checks []sqlbase.ConstraintToValidate,
checks []sqlbase.ConstraintToUpdate,
) error {
if testDisableTableLeases {
return nil
Expand Down Expand Up @@ -916,7 +973,7 @@ func runSchemaChangesInTxn(
// all column mutations.
doneColumnBackfill := false
// Checks are validated after all other mutations have been applied.
var checksToValidate []sqlbase.ConstraintToValidate
var checksToValidate []sqlbase.ConstraintToUpdate

for _, m := range tableDesc.Mutations {
immutDesc := sqlbase.NewImmutableTableDescriptor(*tableDesc.TableDesc())
Expand All @@ -939,7 +996,8 @@ func runSchemaChangesInTxn(

case *sqlbase.DescriptorMutation_Constraint:
switch t.Constraint.ConstraintType {
case sqlbase.ConstraintToValidate_CHECK:
case sqlbase.ConstraintToUpdate_CHECK:
tableDesc.Checks = append(tableDesc.Checks, &t.Constraint.Check)
checksToValidate = append(checksToValidate, *t.Constraint)
default:
return errors.Errorf("unsupported constraint type: %d", t.Constraint.ConstraintType)
Expand Down
Loading