Skip to content

Commit

Permalink
catalog,tabledesc: tighten Constraint interface definition
Browse files Browse the repository at this point in the history 
Release note: None
  • Loading branch information
Marius Posta committed Nov 15, 2022
1 parent 47c7b3a commit f08db43
Show file tree
Hide file tree
Showing 13 changed files with 458 additions and 474 deletions.
156 changes: 81 additions & 75 deletions pkg/sql/backfill.go
View file

Large diffs are not rendered by default.

9 changes: 2 additions & 7 deletions pkg/sql/backfill_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,9 @@ type constraintToUpdateForTest struct {
desc *descpb.ConstraintToUpdate
}

// IsCheck returns true iff this is an update for a check constraint.
func (c constraintToUpdateForTest) IsCheck() bool {
return c.desc.ConstraintType == descpb.ConstraintToUpdate_CHECK
}

// Check returns the underlying check constraint, if there is one.
func (c constraintToUpdateForTest) Check() descpb.TableDescriptor_CheckConstraint {
return c.desc.Check
func (c constraintToUpdateForTest) AsCheck() *descpb.TableDescriptor_CheckConstraint {
return &c.desc.Check
}

func TestShouldSkipConstraintValidation(t *testing.T) {
Expand Down
190 changes: 0 additions & 190 deletions pkg/sql/catalog/descriptor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
package catalog_test

import (
"sort"
"testing"

"github.com/cockroachdb/cockroach/pkg/sql/catalog"
Expand Down Expand Up @@ -67,192 +66,3 @@ func TestFormatSafeDescriptorProperties(t *testing.T) {
})
}
}

// TestConstraintRetrieval tests the following three method inside catalog.TableDescriptor interface.
// - AllConstraints() []Constraint
// - AllActiveAndInactiveConstraints() []Constraint
// - AllActiveConstraints() []Constraint
func TestConstraintRetrieval(t *testing.T) {
// Construct a table with the following constraints:
// - Primary Index: [ID_1:validated]
// - Indexes: [a non-unique index, ID_4:validated]
// - Checks: [ID_2:validated], [ID_3:unvalidated]
// - OutboundFKs: [ID_6:validated]
// - InboundFKs: [ID_5:validated]
// - UniqueWithoutIndexConstraints: [ID_7:dropping]
// - mutation slice: [ID_7:dropping:UniqueWithoutIndex, ID_8:validating:Check, ID_9:validating:UniqueIndex, a non-unique index]
primaryIndex := descpb.IndexDescriptor{
Unique: true,
ConstraintID: 1,
EncodingType: descpb.PrimaryIndexEncoding,
}

indexes := make([]descpb.IndexDescriptor, 2)
indexes[0] = descpb.IndexDescriptor{
Unique: false,
}
indexes[1] = descpb.IndexDescriptor{
Unique: true,
ConstraintID: 4,
}

checks := make([]*descpb.TableDescriptor_CheckConstraint, 2)
checks[0] = &descpb.TableDescriptor_CheckConstraint{
Validity: descpb.ConstraintValidity_Validated,
ConstraintID: 2,
}
checks[1] = &descpb.TableDescriptor_CheckConstraint{
Validity: descpb.ConstraintValidity_Unvalidated,
ConstraintID: 3,
}

outboundFKs := make([]descpb.ForeignKeyConstraint, 1)
outboundFKs[0] = descpb.ForeignKeyConstraint{
Validity: descpb.ConstraintValidity_Validated,
ConstraintID: 6,
}

inboundFKs := make([]descpb.ForeignKeyConstraint, 1)
inboundFKs[0] = descpb.ForeignKeyConstraint{
Validity: descpb.ConstraintValidity_Validated,
ConstraintID: 5,
}

uniqueWithoutIndexConstraints := make([]descpb.UniqueWithoutIndexConstraint, 1)
uniqueWithoutIndexConstraints[0] = descpb.UniqueWithoutIndexConstraint{
Validity: descpb.ConstraintValidity_Dropping,
ConstraintID: 7,
}

mutations := make([]descpb.DescriptorMutation, 4)
mutations[0] = descpb.DescriptorMutation{
Descriptor_: &descpb.DescriptorMutation_Constraint{
Constraint: &descpb.ConstraintToUpdate{
ConstraintType: descpb.ConstraintToUpdate_UNIQUE_WITHOUT_INDEX,
Name: "unique_on_k_without_index",
UniqueWithoutIndexConstraint: uniqueWithoutIndexConstraints[0],
},
},
State: descpb.DescriptorMutation_DELETE_ONLY,
Direction: descpb.DescriptorMutation_DROP,
}
mutations[1] = descpb.DescriptorMutation{
Descriptor_: &descpb.DescriptorMutation_Constraint{
Constraint: &descpb.ConstraintToUpdate{
ConstraintType: descpb.ConstraintToUpdate_CHECK,
Check: descpb.TableDescriptor_CheckConstraint{
Validity: descpb.ConstraintValidity_Validating,
ConstraintID: 8,
},
}},
State: descpb.DescriptorMutation_DELETE_ONLY,
Direction: descpb.DescriptorMutation_ADD,
}
mutations[2] = descpb.DescriptorMutation{
Descriptor_: &descpb.DescriptorMutation_Index{
Index: &descpb.IndexDescriptor{
Unique: true,
ConstraintID: 9,
}},
State: descpb.DescriptorMutation_DELETE_ONLY,
Direction: descpb.DescriptorMutation_ADD,
}
mutations[3] = descpb.DescriptorMutation{
Descriptor_: &descpb.DescriptorMutation_Index{
Index: &descpb.IndexDescriptor{
Unique: false,
}},
State: descpb.DescriptorMutation_DELETE_ONLY,
Direction: descpb.DescriptorMutation_ADD,
}

tableDesc := tabledesc.NewBuilder(&descpb.TableDescriptor{
PrimaryIndex: primaryIndex,
Indexes: indexes,
Checks: checks,
OutboundFKs: outboundFKs,
InboundFKs: inboundFKs,
UniqueWithoutIndexConstraints: uniqueWithoutIndexConstraints,
Mutations: mutations,
}).BuildImmutable().(catalog.TableDescriptor)

t.Run("test-AllConstraints", func(t *testing.T) {
all := tableDesc.AllConstraints()
sort.Slice(all, func(i, j int) bool {
return all[i].GetConstraintID() < all[j].GetConstraintID()
})
require.Equal(t, len(all), 9)
checkIndexBackedConstraint(t, all[0], 1, descpb.ConstraintValidity_Validated, descpb.PrimaryIndexEncoding)
checkNonIndexBackedConstraint(t, all[1], 2, descpb.ConstraintValidity_Validated, descpb.ConstraintToUpdate_CHECK)
checkNonIndexBackedConstraint(t, all[2], 3, descpb.ConstraintValidity_Unvalidated, descpb.ConstraintToUpdate_CHECK)
checkIndexBackedConstraint(t, all[3], 4, descpb.ConstraintValidity_Validated, descpb.SecondaryIndexEncoding)
checkNonIndexBackedConstraint(t, all[4], 5, descpb.ConstraintValidity_Validated, descpb.ConstraintToUpdate_FOREIGN_KEY)
checkNonIndexBackedConstraint(t, all[5], 6, descpb.ConstraintValidity_Validated, descpb.ConstraintToUpdate_FOREIGN_KEY)
checkNonIndexBackedConstraint(t, all[6], 7, descpb.ConstraintValidity_Dropping, descpb.ConstraintToUpdate_UNIQUE_WITHOUT_INDEX)
checkNonIndexBackedConstraint(t, all[7], 8, descpb.ConstraintValidity_Validating, descpb.ConstraintToUpdate_CHECK)
checkIndexBackedConstraint(t, all[8], 9, descpb.ConstraintValidity_Validating, descpb.SecondaryIndexEncoding)
})

t.Run("test-AllActiveAndInactiveConstraints", func(t *testing.T) {
allActiveAndInactive := tableDesc.AllActiveAndInactiveConstraints()
sort.Slice(allActiveAndInactive, func(i, j int) bool {
return allActiveAndInactive[i].GetConstraintID() < allActiveAndInactive[j].GetConstraintID()
})
require.Equal(t, len(allActiveAndInactive), 8)
checkIndexBackedConstraint(t, allActiveAndInactive[0], 1, descpb.ConstraintValidity_Validated, descpb.PrimaryIndexEncoding)
checkNonIndexBackedConstraint(t, allActiveAndInactive[1], 2, descpb.ConstraintValidity_Validated, descpb.ConstraintToUpdate_CHECK)
checkNonIndexBackedConstraint(t, allActiveAndInactive[2], 3, descpb.ConstraintValidity_Unvalidated, descpb.ConstraintToUpdate_CHECK)
checkIndexBackedConstraint(t, allActiveAndInactive[3], 4, descpb.ConstraintValidity_Validated, descpb.SecondaryIndexEncoding)
checkNonIndexBackedConstraint(t, allActiveAndInactive[4], 5, descpb.ConstraintValidity_Validated, descpb.ConstraintToUpdate_FOREIGN_KEY)
checkNonIndexBackedConstraint(t, allActiveAndInactive[5], 6, descpb.ConstraintValidity_Validated, descpb.ConstraintToUpdate_FOREIGN_KEY)
checkNonIndexBackedConstraint(t, allActiveAndInactive[6], 8, descpb.ConstraintValidity_Validating, descpb.ConstraintToUpdate_CHECK)
checkIndexBackedConstraint(t, allActiveAndInactive[7], 9, descpb.ConstraintValidity_Validating, descpb.SecondaryIndexEncoding)
})

t.Run("test-AllActiveConstraints", func(t *testing.T) {
allActive := tableDesc.AllActiveConstraints()
sort.Slice(allActive, func(i, j int) bool {
return allActive[i].GetConstraintID() < allActive[j].GetConstraintID()
})
require.Equal(t, len(allActive), 5)
checkIndexBackedConstraint(t, allActive[0], 1, descpb.ConstraintValidity_Validated, descpb.PrimaryIndexEncoding)
checkNonIndexBackedConstraint(t, allActive[1], 2, descpb.ConstraintValidity_Validated, descpb.ConstraintToUpdate_CHECK)
checkIndexBackedConstraint(t, allActive[2], 4, descpb.ConstraintValidity_Validated, descpb.SecondaryIndexEncoding)
checkNonIndexBackedConstraint(t, allActive[3], 5, descpb.ConstraintValidity_Validated, descpb.ConstraintToUpdate_FOREIGN_KEY)
checkNonIndexBackedConstraint(t, allActive[4], 6, descpb.ConstraintValidity_Validated, descpb.ConstraintToUpdate_FOREIGN_KEY)
})
}

// checkIndexBackedConstraint ensures `c` (PRIMARY KEY or UNIQUE)
// has the expected ID, validity, and type.
//
// `expectedEncodingType = secondaryIndexEncoding` is used for
// UNIQUE constraint type.
func checkIndexBackedConstraint(
t *testing.T,
c catalog.Constraint,
expectedID descpb.ConstraintID,
expectedValidity descpb.ConstraintValidity,
expectedEncodingType descpb.IndexDescriptorEncodingType,
) {
require.Equal(t, expectedID, c.GetConstraintID())
require.Equal(t, expectedValidity, c.GetConstraintValidity())
require.Equal(t, expectedEncodingType, c.IndexDesc().EncodingType)
if expectedEncodingType == descpb.SecondaryIndexEncoding {
require.Equal(t, true, c.IndexDesc().Unique)
}
}

// checkNonIndexBackedConstraint ensures `c` (CHECK, FK, or UNIQUE_WITHOUT_INDEX)
// has the expected ID, validity, and type.
func checkNonIndexBackedConstraint(
t *testing.T,
c catalog.Constraint,
expectedID descpb.ConstraintID,
expectedValidity descpb.ConstraintValidity,
expectedType descpb.ConstraintToUpdate_ConstraintType,
) {
require.Equal(t, expectedID, c.GetConstraintID())
require.Equal(t, expectedValidity, c.GetConstraintValidity())
require.Equal(t, expectedType, c.ConstraintToUpdateDesc().ConstraintType)
}
64 changes: 20 additions & 44 deletions pkg/sql/catalog/table_elements.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
package catalog

import (
"fmt"
"time"

"github.com/cockroachdb/cockroach/pkg/geo/geoindex"
Expand Down Expand Up @@ -399,63 +400,38 @@ type Column interface {
// Constraint is an interface around a constraint.
type Constraint interface {
TableElementMaybeMutation
fmt.Stringer

// ConstraintToUpdateDesc returns the underlying protobuf descriptor
// for non-index-backed-constraints (CHECK, FK, or UNIQUE_WITHOUT_INDEX).
ConstraintToUpdateDesc() *descpb.ConstraintToUpdate
// GetConstraintID returns the ID for the constraint.
GetConstraintID() descpb.ConstraintID

// IndexDesc returns the underlying protobuf descriptor for
// index-backed-constraints (PRIMARY KEY or UNIQUE).
IndexDesc() *descpb.IndexDescriptor
// GetConstraintValidity returns the validity of this constraint.
GetConstraintValidity() descpb.ConstraintValidity

// GetName returns the name of this constraint update mutation.
GetName() string

// IsCheck returns true iff this is an update for a check constraint.
IsCheck() bool

// IsForeignKey returns true iff this is an update for a fk constraint.
IsForeignKey() bool

// IsNotNull returns true iff this is an update for a not-null constraint.
IsNotNull() bool

// IsUniqueWithoutIndex returns true iff this is an update for a unique
// without index constraint.
IsUniqueWithoutIndex() bool

// IsPrimaryKey returns true iff this is an index-backed PRIMARY KEY constraint.
IsPrimaryKey() bool

// IsUniqueConstraint returns true iff this is an index-backed UNIQUE constraint.
IsUniqueConstraint() bool

// Check returns the underlying check constraint, if there is one.
Check() descpb.TableDescriptor_CheckConstraint

// ForeignKey returns the underlying fk constraint, if there is one.
ForeignKey() descpb.ForeignKeyConstraint

// NotNullColumnID returns the underlying not-null column ID, if there is one.
NotNullColumnID() descpb.ColumnID

// UniqueWithoutIndex returns the underlying unique without index constraint, if
// there is one.
UniqueWithoutIndex() descpb.UniqueWithoutIndexConstraint
// AsCheck returns the underlying check constraint, if there is one.
AsCheck() *descpb.TableDescriptor_CheckConstraint

// PrimaryKey returns the index descriptor backing the PRIMARY KEY constraint,
// if there is one.
PrimaryKey() Index
// AsForeignKey returns the underlying foreign key constraint, if there is
// one.
AsForeignKey() *descpb.ForeignKeyConstraint

// Unique returns the index descriptor backing the UNIQUE constraint,
// if there is one.
Unique() Index
// AsUniqueWithoutIndex returns the underlying unique without index
// constraint, if there is one.
AsUniqueWithoutIndex() *descpb.UniqueWithoutIndexConstraint

// GetConstraintID returns the ID for the constraint.
GetConstraintID() descpb.ConstraintID
// AsPrimaryKey returns the index descriptor backing the PRIMARY KEY
// constraint, if there is one.
AsPrimaryKey() Index

// GetConstraintValidity returns the validity of this constraint.
GetConstraintValidity() descpb.ConstraintValidity
// AsUnique returns the index descriptor backing the UNIQUE constraint,
// if there is one.
AsUnique() Index
}

// PrimaryKeySwap is an interface around a primary key swap mutation.
Expand Down
1 change: 1 addition & 0 deletions pkg/sql/catalog/tabledesc/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ go_test(
name = "tabledesc_test",
size = "small",
srcs = [
"constraint_test.go",
"helpers_test.go",
"index_test.go",
"main_test.go",
Expand Down
4 changes: 3 additions & 1 deletion pkg/sql/catalog/tabledesc/constraint.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,9 @@ func newConstraintCache(desc *descpb.TableDescriptor, mutations *mutationCache)
mutationState: mutationState(cstMutation),
mutationIsRollback: cstMutation.IsRollback(),
},
desc: cst.ConstraintToUpdateDesc(),
}
if cc, ok := cst.(*constraint); ok {
backingStruct.desc = cc.desc
}
allConstraints = addIfNotExists(backingStruct, allConstraints, constraintIDs)
}
Expand Down
Loading

0 comments on commit f08db43

Please sign in to comment.