sql: add not visible index to optimizer

This commit adds the logic of the invisible index feature to the optimizer.
After this commit has been merged, the invisible index feature should be fully
functional with `CREATE INDEX` and `CREATE TABLE`.

Assists: cockroachdb#72576

See also: cockroachdb#85239

Release note (sql change): creating a not visible index using `CREATE TABLE …
(INDEX … NOT VISIBLE)` or `CREATE INDEX … NOT VISIBLE` is now supported.

pkg/sql/alter_table.go

@@ -859,6 +859,16 @@ func (n *alterTableNode) startExec(params runParams) error {
 			return err
 		}
 	}
+
+	// Warn against dropping an index if there exists a NotVisible index that may
+	// be used for constraint check behind the scene.
+	if notVisibleIndexNotice := tabledesc.ValidateNotVisibleIndexWithinTable(n.tableDesc); notVisibleIndexNotice != nil {
+		params.p.BufferClientNotice(
+			params.ctx,
+			notVisibleIndexNotice,
+		)
+	}
+
 	// Were some changes made?
 	//
 	// This is only really needed for the unittests that add dummy mutations

pkg/sql/catalog/descpb/index.go

@@ -64,6 +64,15 @@ func (desc *IndexDescriptor) FillColumns(elems tree.IndexElemList) error {
 	return nil
 }
 
+// IsHelpfulOriginIndex returns whether the index may become a helpful index for
+// foreign key constraint check on the child table. Given the originColIDs for
+// foreign key constraint, the index could be useful for FK check if it has
+// overlaps (intersection) with the originColIDs.
+// TODO(wenyihu6): check if checking HasIntersection is correct here.
+func (desc *IndexDescriptor) IsHelpfulOriginIndex(originColIDs ColumnIDs) bool {
+	return !desc.IsPartial() && ColumnIDs(desc.KeyColumnIDs).HasIntersection(originColIDs)
+}
+
 // IsValidOriginIndex returns whether the index can serve as an origin index for a foreign
 // key constraint with the provided set of originColIDs.
 func (desc *IndexDescriptor) IsValidOriginIndex(originColIDs ColumnIDs) bool {

pkg/sql/catalog/descpb/structured.go

@@ -131,6 +131,28 @@ func (c ColumnIDs) HasPrefix(input ColumnIDs) bool {
 	return true
 }
 
+// UtilFastIntSetOperation takes in a list of ColumnIDs and performs the given
+// operation op after transforming ColumnIDs to FastIntSet.
+func (c ColumnIDs) UtilFastIntSetOperation(
+	input ColumnIDs, op func(util.FastIntSet, util.FastIntSet) bool,
+) bool {
+	ourColsSet := util.MakeFastIntSet()
+	for _, col := range c {
+		ourColsSet.Add(int(col))
+	}
+
+	inputColsSet := util.MakeFastIntSet()
+	for _, inputCol := range input {
+		inputColsSet.Add(int(inputCol))
+	}
+	return op(ourColsSet, inputColsSet)
+}
+
+// HasIntersection returns true if the input list has intersection with columnIDs.
+func (c ColumnIDs) HasIntersection(input ColumnIDs) bool {
+	return c.UtilFastIntSetOperation(input, util.FastIntSet.Intersects)
+}
+
 // Equals returns true if the input list is equal to this list.
 func (c ColumnIDs) Equals(input ColumnIDs) bool {
 	if len(input) != len(c) {
@@ -147,17 +169,7 @@ func (c ColumnIDs) Equals(input ColumnIDs) bool {
 // PermutationOf returns true if this list and the input list contain the same
 // set of column IDs in any order. Duplicate ColumnIDs have no effect.
 func (c ColumnIDs) PermutationOf(input ColumnIDs) bool {
-	ourColsSet := util.MakeFastIntSet()
-	for _, col := range c {
-		ourColsSet.Add(int(col))
-	}
-
-	inputColsSet := util.MakeFastIntSet()
-	for _, inputCol := range input {
-		inputColsSet.Add(int(inputCol))
-	}
-
-	return inputColsSet.Equals(ourColsSet)
+	return c.UtilFastIntSetOperation(input, util.FastIntSet.Equals)
 }
 
 // Contains returns whether this list contains the input ID.

pkg/sql/catalog/table_elements.go

@@ -151,6 +151,7 @@ type Index interface {
 	GetShardColumnName() string
 
 	IsValidOriginIndex(originColIDs descpb.ColumnIDs) bool
+	IsHelpfulOriginIndex(originColIDs descpb.ColumnIDs) bool
 	IsValidReferencedUniqueConstraint(referencedColIDs descpb.ColumnIDs) bool
 
 	GetPartitioning() Partitioning

pkg/sql/catalog/tabledesc/BUILD.bazel

@@ -40,6 +40,7 @@ go_library(
         "//pkg/sql/parser",
         "//pkg/sql/pgwire/pgcode",
         "//pkg/sql/pgwire/pgerror",
+        "//pkg/sql/pgwire/pgnotice",
         "//pkg/sql/privilege",
         "//pkg/sql/rowenc",
         "//pkg/sql/schemachanger/scpb",

pkg/sql/catalog/tabledesc/index.go

@@ -142,6 +142,12 @@ func (w index) IsValidOriginIndex(originColIDs descpb.ColumnIDs) bool {
 	return w.desc.IsValidOriginIndex(originColIDs)
 }
 
+// IsHelpfulOriginIndex returns whether the index may become a helpful index for
+// foreign key constraint check on the child table.
+func (w index) IsHelpfulOriginIndex(originColIDs descpb.ColumnIDs) bool {
+	return w.desc.IsHelpfulOriginIndex(originColIDs)
+}
+
 // IsValidReferencedUniqueConstraint returns whether the index can serve as a
 // referenced index for a foreign  key constraint with the provided set of
 // referencedColumnIDs.

pkg/sql/catalog/tabledesc/validate.go

@@ -28,6 +28,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/parser"
 	"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/rowenc"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
@@ -792,6 +793,53 @@ func (desc *wrapper) ValidateSelf(vea catalog.ValidationErrorAccumulator) {
 	ValidateOnUpdate(desc, vea.Report)
 }
 
+// NotVisible indexes may still be used to police unique or foreign key
+// constraint check behind the scene. Hence, dropping the index might behave
+// different from marking the index invisible. The following two cases are where
+// NotVisible indexes might be used for constraint check:
+// Case 1. if there exists unique and not-visible indexes in the given tableDesc.
+// Case 2. if the given tableDesc is a child table and there exists not visible
+// indexes that could be helpful for FK check in the table.
+// Note that not visible indexes in a parent table may also be used for FK
+// check, but they are not discussed in a separate case here because they will
+// always be unique indexes (having unique indexes or constraint on a parent
+// table was necessary to create the child table).
+func validateNotVisibleIndex(
+	index catalog.Index, tableDesc catalog.TableDescriptor,
+) pgnotice.Notice {
+	noticeStr := "not visible indexes may still be used for unique or " +
+		"foreign key constraint check, so the query plan may be different from " +
+		"dropping the index completely."
+	if index.IsUnique() {
+		return pgnotice.Newf(noticeStr)
+	}
+
+	notice := tableDesc.ForeachOutboundFK(func(fk *descpb.ForeignKeyConstraint) error {
+		// IsHelpfulOriginIndex checks if the index columns has overlaps with the fk
+		// constraint origin columns. If it does, we know that this index might be
+		// helpful for fk checks.
+		if index.IsHelpfulOriginIndex(fk.OriginColumnIDs) {
+			return pgnotice.Newf(noticeStr)
+		}
+		return nil
+	})
+	return notice
+}
+
+// ValidateNotVisibleIndexWithinTable returns a notice when dropping an index
+// may behave differently than marking the index invisible.
+func ValidateNotVisibleIndexWithinTable(tableDesc catalog.TableDescriptor) pgnotice.Notice {
+	for _, idx := range tableDesc.AllIndexes() {
+		if idx.IsNotVisible() {
+			// Perform checks on every NotVisible indexes.
+			if notice := validateNotVisibleIndex(idx, tableDesc); notice != nil {
+				return notice
+			}
+		}
+	}
+	return nil
+}
+
 // ValidateOnUpdate returns an error if there is a column with both a foreign
 // key constraint and an ON UPDATE expression, nil otherwise.
 func ValidateOnUpdate(desc catalog.TableDescriptor, errReportFn func(err error)) {

pkg/sql/create_index.go

@@ -736,12 +736,6 @@ func (n *createIndexNode) startExec(params runParams) error {
 		)
 	}
 
-	if n.n.NotVisible {
-		return unimplemented.Newf(
-			"Not Visible Index",
-			"creating a not visible index is not supported yet")
-	}
-
 	// Warn against creating a non-partitioned index on a partitioned table,
 	// which is undesirable in most cases.
 	// Avoid the warning if we have PARTITION ALL BY as all indexes will implicitly
@@ -813,6 +807,15 @@ func (n *createIndexNode) startExec(params runParams) error {
 		return err
 	}
 
+	// Warn against dropping an index if there exists a NotVisible index that may
+	// be used for constraint check behind the scene.
+	if notVisibleIndexNotice := tabledesc.ValidateNotVisibleIndexWithinTable(n.tableDesc); notVisibleIndexNotice != nil {
+		params.p.BufferClientNotice(
+			params.ctx,
+			notVisibleIndexNotice,
+		)
+	}
+
 	// The index name may have changed as a result of
 	// AllocateIDs(). Retrieve it for the event log below.
 	index := n.tableDesc.Mutations[mutationIdx].GetIndex()

pkg/sql/create_table.go

@@ -1784,11 +1784,7 @@ func NewTableDesc(
 					return nil, pgerror.Newf(pgcode.DuplicateRelation, "duplicate index name: %q", d.Name)
 				}
 			}
-			if d.NotVisible {
-				return nil, unimplemented.Newf(
-					"Not Visible Index",
-					"creating a not visible index is not supported yet")
-			}
+
 			if err := validateColumnsAreAccessible(&desc, d.Columns); err != nil {
 				return nil, err
 			}
@@ -1893,11 +1889,7 @@ func NewTableDesc(
 				// We will add the unique constraint below.
 				break
 			}
-			if d.NotVisible {
-				return nil, unimplemented.Newf(
-					"Not Visible Index",
-					"creating a not visible index is not supported yet")
-			}
+
 			// If the index is named, ensure that the name is unique. Unnamed
 			// indexes will be given a unique auto-generated name later on when
 			// AllocateIDs is called.
@@ -2225,6 +2217,15 @@ func NewTableDesc(
 		return nil, onUpdateErr
 	}
 
+	// Warn against dropping an index if there exists a NotVisible index that may
+	// be used for constraint check behind the scene.
+	if notVisibleIndexNotice := tabledesc.ValidateNotVisibleIndexWithinTable(&desc); notVisibleIndexNotice != nil {
+		evalCtx.ClientNoticeSender.BufferClientNotice(
+			ctx,
+			notVisibleIndexNotice,
+		)
+	}
+
 	// AllocateIDs mutates its receiver. `return desc, desc.AllocateIDs()`
 	// happens to work in gc, but does not work in gccgo.
 	//