Skip to content

Commit

Permalink
sql: emit NOTICEs when implicitly dropping indexes
Browse files Browse the repository at this point in the history
Previously, users would have to either know ahead of time or later
discover if an index would be implicitly dropped by dropping a column
that the index referenced.

At best, this was mildly surprising. At worst, workloads could be
dramatically impacted without much indication as to why.

To ensure implicit drops do not go unnoticed, this commit updates `ALTER
TABLE ... DROP COLUMN ...` to emit NOTICEs for each implicitly dropped
index.

Fixes: #106907
Release note (sql change): NOTICEs are now emitted for each index
dropped by an `ALTER TABLE ... DROP COLUMN ...` statement.
  • Loading branch information
chrisseto committed Aug 2, 2023
1 parent c6c787e commit 5ec47b2
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 4 deletions.
5 changes: 5 additions & 0 deletions pkg/sql/alter_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -1718,6 +1718,11 @@ func dropColumnImpl(
}

for _, idxName := range idxNamesToDelete {
params.EvalContext().ClientNoticeSender.BufferClientNotice(params.ctx, pgnotice.Newf(
"dropping index %q which depends on column %q",
idxName,
colToDrop.ColName(),
))
jobDesc := fmt.Sprintf(
"removing index %q dependent on column %q which is being dropped; full details: %s",
idxName,
Expand Down
14 changes: 10 additions & 4 deletions pkg/sql/logictest/testdata/logic_test/alter_table
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,9 @@ ALTER TABLE t DROP COLUMN IF EXISTS q
statement error cannot drop column "e" because view "v" depends on it
ALTER TABLE t DROP COLUMN IF EXISTS e

statement ok
# Negative test to assert that NOTICEs are NOT emitted if there are no indexes
# being dropped.
statement notice ^$
ALTER TABLE t DROP COLUMN IF EXISTS e CASCADE

statement ok
Expand All @@ -403,12 +405,16 @@ CREATE TABLE o (gf INT REFERENCES t (g), h INT, i INT, INDEX oi (i), INDEX oh (h
statement error "t_g_key" is referenced by foreign key from table "o"
ALTER TABLE t DROP COLUMN g

statement ok
statement notice NOTICE: dropping index "t_g_key" which depends on column "g"
ALTER TABLE t DROP COLUMN g CASCADE

# Dropping columns that are indexed or stored in indexes drops those indexes
# too.
statement ok
# too. Excuse the nasty regex, the legacy schema changer emits additional
# NOTICEs about GCing. We're not testing for that, so we just "eat" them.
# Example: "NOTICE: the data for dropped indexes is reclaimed
# asynchronously\nHINT: The reclamation delay can be customized in the zone
# configuration for the table."
statement notice (NOTICE: dropping index "(oh|oih)" which depends on column "h"(\nNOTICE: .+\nHINT: .+\n)?\n?){2}
ALTER TABLE o DROP COLUMN h

query TTBITTTBBBF colnames,rowsort
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,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/schemachanger/scerrors"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
Expand Down Expand Up @@ -227,6 +228,11 @@ func dropColumn(
Table: *tn,
Index: tree.UnrestrictedName(indexName.Name),
}
b.EvalCtx().ClientNoticeSender.BufferClientNotice(b, pgnotice.Newf(
"dropping index %q which depends on column %q",
indexName.Name,
cn.Name,
))
dropSecondaryIndex(b, &name, behavior, e)
case *scpb.View:
if behavior != tree.DropCascade {
Expand Down

0 comments on commit 5ec47b2

Please sign in to comment.