sql: support ALTER INDEX … NOT VISIBLE

This commit adds support to execute ALTER INDEX … [VISIBLE | NOT VISIBLE].

Assists: cockroachdb#72576

See also: cockroachdb#85473

Release note (sql change): Altering an index to visible or not visible using
ALTER INDEX … VISIBLE | NOT VISIBLE is now supported.

docs/generated/eventlog.md

@@ -639,6 +639,31 @@ An event of type `alter_index` is recorded when an index is altered.
 | `MutationID` | The mutation ID for the asynchronous job that is processing the index update. | no |
 
 
+#### Common fields
+
+| Field | Description | Sensitive |
+|--|--|--|
+| `Timestamp` | The timestamp of the event. Expressed as nanoseconds since the Unix epoch. | no |
+| `EventType` | The type of the event. | no |
+| `Statement` | A normalized copy of the SQL statement that triggered the event. The statement string contains a mix of sensitive and non-sensitive details (it is redactable). | partially |
+| `Tag` | The statement tag. This is separate from the statement string, since the statement string can contain sensitive information. The tag is guaranteed not to. | no |
+| `User` | The user account that triggered the event. The special usernames `root` and `node` are not considered sensitive. | depends |
+| `DescriptorID` | The primary object descriptor affected by the operation. Set to zero for operations that don't affect descriptors. | no |
+| `ApplicationName` | The application name for the session where the event was emitted. This is included in the event to ease filtering of logging output by application. Application names starting with a dollar sign (`$`) are not considered sensitive. | no |
+| `PlaceholderValues` | The mapping of SQL placeholders to their values, for prepared statements. | yes |
+
+### `alter_index_visible`
+
+AlterIndex is recorded when an index visibility is altered.
+
+
+| Field | Description | Sensitive |
+|--|--|--|
+| `TableName` | The name of the table containing the affected index. | yes |
+| `IndexName` | The name of the affected index. | yes |
+| `NotVisible` | Set true if index is not visible. | no |
+
+
 #### Common fields
 
 | Field | Description | Sensitive |

pkg/sql/alter_index_visible.go

@@ -14,9 +14,13 @@ import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
+	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
-	"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
+	"github.com/cockroachdb/cockroach/pkg/util/log/eventpb"
 )
 
 type alterIndexVisibleNode struct {
@@ -28,17 +32,77 @@ type alterIndexVisibleNode struct {
 func (p *planner) AlterIndexVisible(
 	ctx context.Context, n *tree.AlterIndexVisible,
 ) (planNode, error) {
-	return nil, unimplemented.Newf(
-		"Not Visible Index",
-		"altering an index to visible or not visible is not supported yet")
+	if err := checkSchemaChangeEnabled(
+		ctx,
+		p.ExecCfg(),
+		"ALTER INDEX VISIBILITY",
+	); err != nil {
+		return nil, err
+	}
+
+	// Check if the table actually exists. expandMutableIndexName returns the
+	// underlying table.
+	_, tableDesc, err := expandMutableIndexName(ctx, p, &n.Index, !n.IfExists /* requireTable */)
+	if err != nil {
+		// Error if no table is found and IfExists is false.
+		return nil, err
+	}
+
+	if tableDesc == nil {
+		// No error if no table but IfExists is true.
+		return newZeroNode(nil /* columns */), nil
+	}
+
+	// Check if the index actually exists. FindIndexWithName returns the first
+	// catalog.Index in tableDesc.AllIndexes().
+	idx, err := tableDesc.FindIndexWithName(string(n.Index.Index))
+	if err != nil {
+		if n.IfExists {
+			// Nothing needed if no index exists and IfExists is true.
+			return newZeroNode(nil /* columns */), nil
+		}
+		// Error if no index exists and IfExists is not specified.
+		return nil, pgerror.WithCandidateCode(err, pgcode.UndefinedObject)
+	}
+
+	if err := p.CheckPrivilege(ctx, tableDesc, privilege.CREATE); err != nil {
+		return nil, err
+	}
+
+	return &alterIndexVisibleNode{n: n, tableDesc: tableDesc, index: idx}, nil
 }
 
 func (n *alterIndexVisibleNode) ReadingOwnWrites() {}
 
 func (n *alterIndexVisibleNode) startExec(params runParams) error {
-	return unimplemented.Newf(
-		"Not Visible Index",
-		"altering an index to visible or not visible is not supported yet")
+	if n.n.NotVisible && n.index.Primary() {
+		return pgerror.Newf(pgcode.FeatureNotSupported, "primary index cannot be invisible")
+	}
+
+	if n.index.IsNotVisible() == n.n.NotVisible {
+		// Nothing needed if the index is already what they want.
+		return nil
+	}
+
+	n.index.IndexDesc().NotVisible = n.n.NotVisible
+
+	if err := validateDescriptor(params.ctx, params.p, n.tableDesc); err != nil {
+		return err
+	}
+
+	if err := params.p.writeSchemaChange(
+		params.ctx, n.tableDesc, descpb.InvalidMutationID, tree.AsStringWithFQNames(n.n, params.Ann()),
+	); err != nil {
+		return err
+	}
+
+	return params.p.logEvent(params.ctx,
+		n.tableDesc.ID,
+		&eventpb.AlterIndexVisible{
+			TableName:  n.n.Index.Table.FQString(),
+			IndexName:  n.index.GetName(),
+			NotVisible: n.n.NotVisible,
+		})
 }
 func (n *alterIndexVisibleNode) Next(runParams) (bool, error) { return false, nil }
 func (n *alterIndexVisibleNode) Values() tree.Datums          { return tree.Datums{} }

pkg/sql/opt/exec/execbuilder/testdata/not_visible_index

@@ -545,7 +545,6 @@ DROP TABLE t1
 statement ok
 DROP TABLE t2
 
-
 ##################################################################################
 # Check Invisible Inverted Index and Partial Inverted Index.
 ##################################################################################
@@ -1155,24 +1154,171 @@ statement ok
 DROP TABLE parent
 
 ############################################################################
-# The following tests check for ALTER INDEX ... VISIBLE | NOT VISIBLE. 
+# The following tests check for ALTER INDEX ... VISIBLE | NOT VISIBLE.
+# - Error when IF EXISTS is not specified
+# - Error when altering primary index to not visible happens
+# - Check alter index visibility using SHOW INDEX and EXPLAIN
 ############################################################################
 subtest alter_index_visibility
 
+# The following tests check error and notices.
 statement ok
-CREATE TABLE t (p INT PRIMARY KEY, INDEX idx (p) VISIBLE)
+CREATE TABLE t1 (c INT PRIMARY KEY, other INT NOT NULL, INDEX idx_visible (other) VISIBLE, INDEX idx_invisible (other) NOT VISIBLE)
 
-query TTB
-SELECT index_name, column_name, visible FROM [SHOW INDEX FROM t] ORDER BY index_name, seq_in_index
+query TTBITTBBB colnames
+SELECT * FROM [SHOW INDEX FROM t1]
+----
+table_name  index_name     non_unique  seq_in_index  column_name  direction  storing  implicit  visible
+t1          t1_pkey        false       1             c            ASC        false    false     true
+t1          t1_pkey        false       2             other        N/A        true     false     true
+t1          idx_visible    true        1             other        ASC        false    false     true
+t1          idx_visible    true        2             c            ASC        false    true      true
+t1          idx_invisible  true        1             other        ASC        false    false     false
+t1          idx_invisible  true        2             c            ASC        false    true      false
+
+# Primary index cannot be invisible.
+statement error pq: primary index cannot be invisible
+ALTER INDEX t1_pkey NOT VISIBLE
+
+# Altering primary index to visible is fine.
+statement ok
+ALTER INDEX t1_pkey VISIBLE
+
+# The following test check ALTER PRIMARY KEY.
+# Changing a column with invisible index to primary key is fine: 1. This will
+# result in a new primary index (visible) on the column and the secondary index
+# on the column stay not visible. 2. The old primary key index now becomes a
+# secondary index, and this secondary index can also be changed invisible.
+statement ok
+ALTER TABLE t1 ALTER PRIMARY KEY USING COLUMNS (other);
+
+# A new primary index t1_pkey is created on the column other. The old primary
+# index becomes a secondary index t1_c_key on c. idx_invisible remains
+# invisible.
+query TTBITTBBB colnames
+SELECT * FROM [SHOW INDEX FROM t1]
+----
+table_name  index_name     non_unique  seq_in_index  column_name  direction  storing  implicit  visible
+t1          t1_pkey        false       1             other        ASC        false    false     true
+t1          t1_pkey        false       2             c            N/A        true     false     true
+t1          t1_c_key       false       1             c            ASC        false    false     true
+t1          t1_c_key       false       2             other        ASC        true     true      true
+t1          idx_visible    true        1             other        ASC        false    false     true
+t1          idx_invisible  true        1             other        ASC        false    false     false
+
+# Changing the old primary key index t1_c_key (secondary index now) to invisible
+# is fine.
+statement ok
+ALTER INDEX t1_c_key NOT VISIBLE
+
+# No error if index does not exist and IF EXISTS is specified.
+statement ok
+ALTER INDEX IF EXISTS nonexistent_idx NOT VISIBLE
+
+# No error if table does not exist and IF EXISTS is specified.
+statement ok
+ALTER INDEX IF EXISTS nonexistent_t@idx NOT VISIBLE
+
+# Error if index does not exist and no IF EXISTS.
+statement error pq: index "nonexistent_idx" does not exist
+ALTER INDEX nonexistent_idx NOT VISIBLE
+
+# Error if table exists but index does not exist and no IF EXISTS.
+statement error pq: index "nonexistent_idx" does not exist
+ALTER INDEX t1@nonexistent_idx NOT VISIBLE
+
+# Error if table does not exist and no IF EXISTS.
+statement error pq: relation "nonexistent_table" does not exist
+ALTER INDEX nonexistent_table@nonexistent_idx NOT VISIBLE
+
+statement ok
+CREATE TABLE t2 (c INT PRIMARY KEY, INDEX idx_invisible (c) NOT VISIBLE)
+
+# Check ambiguous index error.
+statement error pq: index name "idx_invisible" is ambiguous
+ALTER INDEX idx_invisible NOT VISIBLE
+
+statement ok
+DROP TABLE t1
+
+statement ok
+DROP TABLE t2
+
+# The following tests check for ALTER INDEX [VISIBLE | NOT VISIBLE] feature.
+statement ok
+CREATE TABLE t (p INT PRIMARY KEY, other INT, INDEX idx (other) VISIBLE)
+
+# idx is visible.
+query TTTB
+SELECT table_name, index_name, column_name, visible FROM [SHOW INDEX FROM t] ORDER BY table_name, index_name, seq_in_index
+----
+t  idx     other  true
+t  idx     p      true
+t  t_pkey  p      true
+t  t_pkey  other  true
+
+# SELECT chooses idx since it is visible now.
+query T
+EXPLAIN SELECT * FROM t WHERE other > 2
 ----
-idx     p  true
-t_pkey  p  true
+distribution: local
+vectorized: true
+·
+• scan
+  missing stats
+  table: t@idx
+  spans: [/3 - ]
 
-statement error pq: unimplemented: altering an index to visible or not visible is not supported yet
+statement ok
 ALTER INDEX idx NOT VISIBLE
 
-statement error pq: unimplemented: altering an index to visible or not visible is not supported yet
+# idx is not visible now.
+query TTTB
+SELECT table_name, index_name, column_name, visible FROM [SHOW INDEX FROM t] ORDER BY table_name, index_name, seq_in_index
+----
+t  idx     other  false
+t  idx     p      false
+t  t_pkey  p      true
+t  t_pkey  other  true
+
+# SELECT ignores idx since idx is not visible.
+query T
+EXPLAIN SELECT * FROM t WHERE other > 2
+----
+distribution: local
+vectorized: true
+·
+• filter
+│ filter: other > 2
+│
+└── • scan
+      missing stats
+      table: t@t_pkey
+      spans: FULL SCAN
+
+statement ok
 ALTER INDEX idx VISIBLE
 
+# idx is back to visible now.
+query TTTB
+SELECT table_name, index_name, column_name, visible FROM [SHOW INDEX FROM t] ORDER BY table_name, index_name, seq_in_index
+----
+t  idx     other  true
+t  idx     p      true
+t  t_pkey  p      true
+t  t_pkey  other  true
+
+# SELECT chooses idx after it becomes visible.
+query T
+EXPLAIN SELECT * FROM t WHERE other > 2
+----
+distribution: local
+vectorized: true
+·
+• scan
+  missing stats
+  table: t@idx
+  spans: [/3 - ]
+
 statement ok
 DROP TABLE t

pkg/util/log/eventpb/ddl_events.proto

@@ -327,6 +327,18 @@ message AlterIndex {
   uint32 mutation_id = 5 [(gogoproto.customname) = "MutationID", (gogoproto.jsontag) = ",omitempty"];
 }
 
+// AlterIndex is recorded when an index visibility is altered.
+message AlterIndexVisible {
+  CommonEventDetails common = 1 [(gogoproto.nullable) = false, (gogoproto.jsontag) = "", (gogoproto.embed) = true];
+  CommonSQLEventDetails sql = 2 [(gogoproto.nullable) = false, (gogoproto.jsontag) = "", (gogoproto.embed) = true];
+  // The name of the table containing the affected index.
+  string table_name = 3 [(gogoproto.jsontag) = ",omitempty"];
+  // The name of the affected index.
+  string index_name = 4 [(gogoproto.jsontag) = ",omitempty"];
+  // Set true if index is not visible.
+  bool not_visible = 5 [(gogoproto.jsontag) = ",omitempty"];
+}
+
 
 // CreateView is recorded when a view is created.
 message CreateView {