diff --git a/docs/generated/eventlog.md b/docs/generated/eventlog.md index 768064079aa7..6fb3d4309b13 100644 --- a/docs/generated/eventlog.md +++ b/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 | diff --git a/pkg/sql/alter_index_visible.go b/pkg/sql/alter_index_visible.go index 223412c0cef5..8a4f1733bfe9 100644 --- a/pkg/sql/alter_index_visible.go +++ b/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{} } diff --git a/pkg/sql/opt/exec/execbuilder/testdata/not_visible_index b/pkg/sql/opt/exec/execbuilder/testdata/not_visible_index index b505a0c71370..2a8699f7d8a9 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/not_visible_index +++ b/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 diff --git a/pkg/util/log/eventpb/ddl_events.proto b/pkg/util/log/eventpb/ddl_events.proto index 184595051eef..1ae8439745ad 100644 --- a/pkg/util/log/eventpb/ddl_events.proto +++ b/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 { diff --git a/pkg/util/log/eventpb/eventlog_channels_generated.go b/pkg/util/log/eventpb/eventlog_channels_generated.go index edc173026467..a953049f5c5f 100644 --- a/pkg/util/log/eventpb/eventlog_channels_generated.go +++ b/pkg/util/log/eventpb/eventlog_channels_generated.go @@ -75,6 +75,9 @@ func (m *AlterDatabaseSurvivalGoal) LoggingChannel() logpb.Channel { return logp // LoggingChannel implements the EventPayload interface. func (m *AlterIndex) LoggingChannel() logpb.Channel { return logpb.Channel_SQL_SCHEMA } +// LoggingChannel implements the EventPayload interface. +func (m *AlterIndexVisible) LoggingChannel() logpb.Channel { return logpb.Channel_SQL_SCHEMA } + // LoggingChannel implements the EventPayload interface. func (m *AlterSequence) LoggingChannel() logpb.Channel { return logpb.Channel_SQL_SCHEMA } diff --git a/pkg/util/log/eventpb/json_encode_generated.go b/pkg/util/log/eventpb/json_encode_generated.go index 27b8f5799d70..213eee3bab2c 100644 --- a/pkg/util/log/eventpb/json_encode_generated.go +++ b/pkg/util/log/eventpb/json_encode_generated.go @@ -339,6 +339,48 @@ func (m *AlterIndex) AppendJSONFields(printComma bool, b redact.RedactableBytes) return printComma, b } +// AppendJSONFields implements the EventPayload interface. +func (m *AlterIndexVisible) AppendJSONFields(printComma bool, b redact.RedactableBytes) (bool, redact.RedactableBytes) { + + printComma, b = m.CommonEventDetails.AppendJSONFields(printComma, b) + + printComma, b = m.CommonSQLEventDetails.AppendJSONFields(printComma, b) + + if m.TableName != "" { + if printComma { + b = append(b, ',') + } + printComma = true + b = append(b, "\"TableName\":\""...) + b = append(b, redact.StartMarker()...) + b = redact.RedactableBytes(jsonbytes.EncodeString([]byte(b), string(redact.EscapeMarkers([]byte(m.TableName))))) + b = append(b, redact.EndMarker()...) + b = append(b, '"') + } + + if m.IndexName != "" { + if printComma { + b = append(b, ',') + } + printComma = true + b = append(b, "\"IndexName\":\""...) + b = append(b, redact.StartMarker()...) + b = redact.RedactableBytes(jsonbytes.EncodeString([]byte(b), string(redact.EscapeMarkers([]byte(m.IndexName))))) + b = append(b, redact.EndMarker()...) + b = append(b, '"') + } + + if m.NotVisible { + if printComma { + b = append(b, ',') + } + printComma = true + b = append(b, "\"NotVisible\":true"...) + } + + return printComma, b +} + // AppendJSONFields implements the EventPayload interface. func (m *AlterRole) AppendJSONFields(printComma bool, b redact.RedactableBytes) (bool, redact.RedactableBytes) {