Skip to content

Commit

Permalink
sql/schemachanger: version gate dropping constraints / functions
Browse files Browse the repository at this point in the history
Previously, it was possible in a mixed version state
to cleanup constraints (foreign keys, unique without indexes,
function bodies) when dropping columns. This would cause
down level nodes to fail at planning seeing unsupported
status transitions. To address this, this patch adds a version gate
to prevent dropping any of these objects when cleaning up a column.

Fixes: cockroachdb#97576

Release note: None
  • Loading branch information
fqazi committed Feb 27, 2023
1 parent 5d0c6ee commit d427d3b
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ package scbuildstmt
import (
"sort"

"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/colinfo"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
Expand Down Expand Up @@ -246,6 +247,12 @@ func dropColumn(
undroppedSeqBackrefsToCheck.Add(e.SequenceID)
}
case *scpb.FunctionBody:
// Until the appropriate version gate is hit, we still do not allow
// dropping function bodies.
if !b.ClusterSettings().Version.IsActive(b, clusterversion.V23_1) {
panic(scerrors.NotImplementedErrorf(nil, "dropping functions is"+
"not allowed yet."))
}
if behavior != tree.DropCascade {
_, _, fnName := scpb.FindFunctionName(b.QueryByID(e.FunctionID))
panic(sqlerrors.NewDependentObjectErrorf(
Expand All @@ -255,6 +262,12 @@ func dropColumn(
}
dropCascadeDescriptor(b, e.FunctionID)
case *scpb.UniqueWithoutIndexConstraint:
// Until the appropriate version gate is hit, we still do not allow
// dropping unique without index constraints.
if !b.ClusterSettings().Version.IsActive(b, clusterversion.V23_1) {
panic(scerrors.NotImplementedErrorf(nil, "dropping without"+
"index constraints is not allowed."))
}
constraintElems := b.QueryByID(e.TableID).Filter(hasConstraintIDAttrFilter(e.ConstraintID))
_, _, constraintName := scpb.FindConstraintWithoutIndexName(constraintElems.Filter(publicTargetFilter))
alterTableDropConstraint(b, tn, tbl, &tree.AlterTableDropConstraint{
Expand All @@ -263,6 +276,12 @@ func dropColumn(
DropBehavior: behavior,
})
case *scpb.UniqueWithoutIndexConstraintUnvalidated:
// Until the appropriate version gate is hit, we still do not allow
// dropping unique without index constraints.
if !b.ClusterSettings().Version.IsActive(b, clusterversion.V23_1) {
panic(scerrors.NotImplementedErrorf(nil, "dropping without"+
"index constraints is not allowed."))
}
constraintElems := b.QueryByID(e.TableID).Filter(hasConstraintIDAttrFilter(e.ConstraintID))
_, _, constraintName := scpb.FindConstraintWithoutIndexName(constraintElems.Filter(publicTargetFilter))
alterTableDropConstraint(b, tn, tbl, &tree.AlterTableDropConstraint{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ package scbuildstmt
import (
"fmt"

"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgnotice"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scerrors"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
Expand Down Expand Up @@ -235,6 +237,12 @@ func maybeDropDependentFKConstraints(
// shouldDropFK returns true if it is a dependent FK and no uniqueness-providing
// replacement can be found.
shouldDropFK := func(fkReferencedColIDs []catid.ColumnID) bool {
// Until the appropriate version gate is hit, we still do not allow
// dropping foreign keys in any the context of secondary indexes.
if !b.ClusterSettings().Version.IsActive(b, clusterversion.V23_1) {
panic(scerrors.NotImplementedErrorf(nil, "dropping FK constraints"+
" as a result of `DROP INDEX CASCADE` is not supported yet."))
}
return canToBeDroppedConstraintServeFK(fkReferencedColIDs) &&
!hasColsUniquenessConstraintOtherThan(b, tableID, fkReferencedColIDs, toBeDroppedConstraintID)
}
Expand Down

0 comments on commit d427d3b

Please sign in to comment.