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

@@ -166,6 +166,10 @@ func (n *alterTableNode) startExec(params runParams) error {
 			"%q was not resolved as a table but is %T", resolved, resolved)
 	}
 
+	// This flag is created to avoid sending multiple notices if user is adding
+	// multiple FK constraints.
+	// TODO (wenyihu6): update buffer notice to perform deduplication of notices
+	sentNotVisibleMsgAlready := new(bool)
 	for i, cmd := range n.n.Cmds {
 		telemetry.Inc(sqltelemetry.SchemaChangeAlterCounterWithExtra("table", cmd.TelemetryName()))
 
@@ -423,6 +427,7 @@ func (n *alterTableNode) startExec(params runParams) error {
 						tableState,
 						t.ValidationBehavior,
 						params.p.EvalContext(),
+						sentNotVisibleMsgAlready,
 					)
 				})
 				if err != nil {

pkg/sql/create_index.go

@@ -737,9 +737,19 @@ 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 dropping an index if user is creating a unique invisible
+		// index or creating an invisible index in a child table. Invisible indexes
+		// may still be used to police unique or foreign key constraint check behind
+		// the scene. In this case, dropping the index may behave differently than
+		// marking the index invisible.
+		if n.n.Unique || len(n.tableDesc.OutboundFKs) != 0 {
+			params.p.BufferClientNotice(
+				params.ctx,
+				pgnotice.Newf("invisible indexes may still be used for unique or "+
+					"foreign key constraint check, so the query plan may be different from "+
+					"dropping the index completely."),
+			)
+		}
 	}
 
 	// Warn against creating a non-partitioned index on a partitioned table,

pkg/sql/create_table.go

@@ -861,6 +861,7 @@ func ResolveFK(
 	ts TableState,
 	validationBehavior tree.ValidationBehavior,
 	evalCtx *eval.Context,
+	sentNotVisibleMsgAlready *bool,
 ) error {
 	var originColSet catalog.TableColSet
 	originCols := make([]catalog.Column, len(d.FromCols))
@@ -1048,6 +1049,26 @@ func ResolveFK(
 		return err
 	}
 
+	// Warn against dropping an index if this is adding a FK constraint to a table
+	// with invisible indexes. Invisible indexes may still be used to police
+	// unique or foreign key constraint check behind the scene. In this case,
+	// dropping the index may behave differently than marking the index invisible.
+	if sentNotVisibleMsgAlready != nil && !*sentNotVisibleMsgAlready {
+		for _, idx := range tbl.Indexes {
+			if idx.NotVisible {
+				*sentNotVisibleMsgAlready = true
+				evalCtx.ClientNoticeSender.BufferClientNotice(
+					ctx,
+					pgnotice.Newf("invisible 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 there are several invisible indexes, send the notice only once.
+				break
+			}
+		}
+	}
+
 	var validity descpb.ConstraintValidity
 	if ts != NewTable {
 		if validationBehavior == tree.ValidationSkip {
@@ -1769,6 +1790,9 @@ func NewTableDesc(
 		}
 	}
 
+	// This flag is created to avoid sending multiple notices if user is adding
+	// multiple FK constraints or multiple unique invisible indexes.
+	sentNotVisibleMsgAlready := new(bool)
 	for _, def := range n.Defs {
 		switch d := def.(type) {
 		case *tree.ColumnTableDef, *tree.LikeTableDef:
@@ -1783,11 +1807,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
 			}
@@ -1892,10 +1912,19 @@ 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")
+
+			// Warn against dropping an index if user is creating a unique invisible
+			// index. Invisible indexes may still be used to police unique or foreign
+			// key constraint check behind the scene. In this case, dropping the index
+			// may behave differently than marking the index invisible.
+			if d.NotVisible && !*sentNotVisibleMsgAlready {
+				*sentNotVisibleMsgAlready = true
+				evalCtx.ClientNoticeSender.BufferClientNotice(
+					ctx,
+					pgnotice.Newf("invisible 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 the index is named, ensure that the name is unique. Unnamed
 			// indexes will be given a unique auto-generated name later on when
@@ -2201,7 +2230,7 @@ func NewTableDesc(
 		case *tree.ForeignKeyConstraintTableDef:
 			if err := ResolveFK(
 				ctx, txn, fkResolver, db, sc, &desc, d, affected, NewTable,
-				tree.ValidationDefault, evalCtx,
+				tree.ValidationDefault, evalCtx, sentNotVisibleMsgAlready,
 			); err != nil {
 				return nil, err
 			}

pkg/sql/importer/read_import_mysql.go

@@ -602,7 +602,7 @@ func addDelayedFKs(
 		if err := sql.ResolveFK(
 			ctx, nil, &resolver, def.db, def.sc, def.tbl, def.def,
 			backrefs, sql.NewTable,
-			tree.ValidationDefault, evalCtx,
+			tree.ValidationDefault, evalCtx, nil,
 		); err != nil {
 			return err
 		}

pkg/sql/importer/read_import_pgdump.go

@@ -437,7 +437,7 @@ func resolvePostgresFKs(
 				evalCtx.Ctx(), nil /* txn */, &fks.resolver,
 				parentDB, schema, desc,
 				constraint, backrefs, sql.NewTable,
-				tree.ValidationDefault, evalCtx,
+				tree.ValidationDefault, evalCtx, nil,
 			); err != nil {
 				return err
 			}