Skip to content

Commit

Permalink
sql: add support for NOT VALID check constraints
Browse files Browse the repository at this point in the history
Mark constraint as unvalidated if user specifies NOT VALID in their
check constraint. Within backfill, do not add the unvalidated constraints
to the queues for validating.

Release note (sql change): Support NOT VALID for check constraints,
which supports not checking constraints for existing rows.
  • Loading branch information
Tyler314 committed Jun 25, 2019
1 parent 225a320 commit 96dcc88
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 15 deletions.
8 changes: 6 additions & 2 deletions pkg/sql/alter_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,12 @@ func (n *alterTableNode) startExec(params runParams) error {
if err != nil {
return err
}
ck.Validity = sqlbase.ConstraintValidity_Validating
n.tableDesc.AddCheckValidationMutation(ck)
if t.ValidationBehavior == tree.ValidationDefault {
ck.Validity = sqlbase.ConstraintValidity_Validating
} else {
ck.Validity = sqlbase.ConstraintValidity_Unvalidated
}
n.tableDesc.AddCheckMutation(ck)

case *tree.ForeignKeyConstraintTableDef:
for _, colName := range d.FromCols {
Expand Down
26 changes: 21 additions & 5 deletions pkg/sql/backfill.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (sc *SchemaChanger) runBackfill(
var droppedIndexDescs []sqlbase.IndexDescriptor
var addedIndexSpans []roachpb.Span

var constraintsToAdd []sqlbase.ConstraintToUpdate
var constraintsToAddBeforeValidation []sqlbase.ConstraintToUpdate
var constraintsToValidate []sqlbase.ConstraintToUpdate

tableDesc, err := sc.updateJobRunningStatus(ctx, RunningStatusBackfill)
Expand Down Expand Up @@ -156,8 +156,24 @@ func (sc *SchemaChanger) runBackfill(
case *sqlbase.DescriptorMutation_Index:
addedIndexSpans = append(addedIndexSpans, tableDesc.IndexSpan(t.Index.ID))
case *sqlbase.DescriptorMutation_Constraint:
constraintsToAdd = append(constraintsToAdd, *t.Constraint)
constraintsToValidate = append(constraintsToValidate, *t.Constraint)
switch t.Constraint.ConstraintType {
case sqlbase.ConstraintToUpdate_CHECK:
if t.Constraint.Check.Validity == sqlbase.ConstraintValidity_Validating {
constraintsToAddBeforeValidation = append(constraintsToAddBeforeValidation, *t.Constraint)
constraintsToValidate = append(constraintsToValidate, *t.Constraint)
}
// TODO (tyler): we do not yet support teh NOT VALID foreign keys,
// because we don't add the Foreign Key mutations
case sqlbase.ConstraintToUpdate_FOREIGN_KEY:
if t.Constraint.ForeignKey.Validity == sqlbase.ConstraintValidity_Validating {
constraintsToAddBeforeValidation = append(constraintsToAddBeforeValidation, *t.Constraint)
constraintsToValidate = append(constraintsToValidate, *t.Constraint)
}
case sqlbase.ConstraintToUpdate_NOT_NULL:
// NOT NULL constraints are always validated before they can be added
constraintsToAddBeforeValidation = append(constraintsToAddBeforeValidation, *t.Constraint)
constraintsToValidate = append(constraintsToValidate, *t.Constraint)
}
default:
return errors.AssertionFailedf(
"unsupported mutation: %+v", m)
Expand Down Expand Up @@ -218,8 +234,8 @@ func (sc *SchemaChanger) runBackfill(
// 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(constraintsToAdd) > 0 {
if err := sc.AddConstraints(ctx, constraintsToAdd); err != nil {
if len(constraintsToAddBeforeValidation) > 0 {
if err := sc.AddConstraints(ctx, constraintsToAddBeforeValidation); err != nil {
return err
}
}
Expand Down
40 changes: 40 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/alter_table
Original file line number Diff line number Diff line change
Expand Up @@ -972,3 +972,43 @@ t a_auto_not_null1 CHECK CHECK ((a IS NOT NULL)) true

statement ok
DROP TABLE t

# Check for adding constraints NOT VALID
statement ok
CREATE TABLE t (a int);

statement ok
INSERT INTO t VALUES (10), (15), (17)

statement error pq: validation of CHECK "a < 16" failed on row: a=17
ALTER TABLE t ADD CHECK (a < 16)

statement ok
ALTER TABLE t ADD CHECK (a < 100)

statement ok
ALTER TABLE t ADD CHECK (a < 16) NOT VALID

query TTTTB
SHOW CONSTRAINTS FROM t
----
t check_a CHECK CHECK ((a < 100)) true
t check_a1 CHECK CHECK ((a < 16)) false

query error pq: failed to satisfy CHECK constraint \(a < 16\)
INSERT INTO t VALUES (20)

statement error pq: validation of CHECK "a < 16" failed on row: a=17
ALTER TABLE t VALIDATE CONSTRAINT check_a1

statement ok
DELETE FROM t WHERE a = 17

statement ok
ALTER TABLE t VALIDATE CONSTRAINT check_a1

query TTTTB
SHOW CONSTRAINTS FROM t
----
t check_a CHECK CHECK ((a < 100)) true
t check_a1 CHECK CHECK ((a < 16)) true
6 changes: 6 additions & 0 deletions pkg/sql/schema_changer.go
Original file line number Diff line number Diff line change
Expand Up @@ -1502,6 +1502,9 @@ func (sc *SchemaChanger) maybeDropValidatingConstraint(
) error {
switch constraint.ConstraintType {
case sqlbase.ConstraintToUpdate_CHECK, sqlbase.ConstraintToUpdate_NOT_NULL:
if constraint.Check.Validity == sqlbase.ConstraintValidity_Unvalidated {
return nil
}
for j, c := range desc.Checks {
if c.Name == constraint.Check.Name {
desc.Checks = append(desc.Checks[:j], desc.Checks[j+1:]...)
Expand All @@ -1516,6 +1519,9 @@ func (sc *SchemaChanger) maybeDropValidatingConstraint(
)
}
case sqlbase.ConstraintToUpdate_FOREIGN_KEY:
if constraint.ForeignKey.Validity == sqlbase.ConstraintValidity_Unvalidated {
return nil
}
idx, err := desc.FindIndexByID(constraint.ForeignKeyIndex)
if err != nil {
return err
Expand Down
21 changes: 13 additions & 8 deletions pkg/sql/sqlbase/structured.go
Original file line number Diff line number Diff line change
Expand Up @@ -2374,11 +2374,18 @@ func (desc *MutableTableDescriptor) MakeMutationComplete(m DescriptorMutation) e
case *DescriptorMutation_Constraint:
switch t.Constraint.ConstraintType {
case ConstraintToUpdate_CHECK:
for _, c := range desc.Checks {
if c.Name == t.Constraint.Name {
c.Validity = ConstraintValidity_Validated
break
switch t.Constraint.Check.Validity {
case ConstraintValidity_Unvalidated:
desc.Checks = append(desc.Checks, &t.Constraint.Check)
case ConstraintValidity_Validating:
for _, c := range desc.Checks {
if c.Name == t.Constraint.Name {
c.Validity = ConstraintValidity_Validated
break
}
}
default:
return errors.AssertionFailedf("invalid constraint validity state: %d", t.Constraint.Check.Validity)
}
case ConstraintToUpdate_FOREIGN_KEY:
idx, err := desc.FindIndexByID(t.Constraint.ForeignKeyIndex)
Expand Down Expand Up @@ -2414,10 +2421,8 @@ func (desc *MutableTableDescriptor) MakeMutationComplete(m DescriptorMutation) e
return nil
}

// AddCheckValidationMutation adds a check constraint validation mutation to desc.Mutations.
func (desc *MutableTableDescriptor) AddCheckValidationMutation(
ck *TableDescriptor_CheckConstraint,
) {
// AddCheckMutation adds a check constraint mutation to desc.Mutations.
func (desc *MutableTableDescriptor) AddCheckMutation(ck *TableDescriptor_CheckConstraint) {
m := DescriptorMutation{
Descriptor_: &DescriptorMutation_Constraint{
Constraint: &ConstraintToUpdate{
Expand Down

0 comments on commit 96dcc88

Please sign in to comment.