From c2764515228f401191c94fea3ab3cc49fa388f7c Mon Sep 17 00:00:00 2001
From: Faizan Qazi <faizan@cockroachlabs.com>
Date: Fri, 2 Dec 2022 16:40:58 -0500
Subject: [PATCH] sql/schemachanger: drop index notices shouldn't always be
 generated

Previously, the declarative schema changer notices
were always generated even if no indexes were dropped.
This was inadequate because some of these should have
been reported if any work was done. To address this, this
patch will detect if any indexes were actually dropped.

Release note: None
---
 pkg/sql/pgwire/testdata/pgtest/notice         |  2 +-
 .../internal/scbuildstmt/drop_index.go        | 25 +++++++++++--------
 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/pkg/sql/pgwire/testdata/pgtest/notice b/pkg/sql/pgwire/testdata/pgtest/notice
index 5dc815a2a842..dec1b5f49217 100644
--- a/pkg/sql/pgwire/testdata/pgtest/notice
+++ b/pkg/sql/pgwire/testdata/pgtest/notice
@@ -55,7 +55,7 @@ Query {"String": "DROP INDEX t_x_idx"}
 until crdb_only
 CommandComplete
 ----
-{"Severity":"NOTICE","SeverityUnlocalized":"NOTICE","Code":"00000","Message":"the data for dropped indexes is reclaimed asynchronously","Detail":"","Hint":"The reclamation delay can be customized in the zone configuration for the table.","Position":0,"InternalPosition":0,"InternalQuery":"","Where":"","SchemaName":"","TableName":"","ColumnName":"","DataTypeName":"","ConstraintName":"","File":"drop_index.go","Line":522,"Routine":"dropIndexByName","UnknownFields":null}
+{"Severity":"NOTICE","SeverityUnlocalized":"NOTICE","Code":"00000","Message":"the data for dropped indexes is reclaimed asynchronously","Detail":"","Hint":"The reclamation delay can be customized in the zone configuration for the table.","Position":0,"InternalPosition":0,"InternalQuery":"","Where":"","SchemaName":"","TableName":"","ColumnName":"","DataTypeName":"","ConstraintName":"","File":"drop_index.go","Line":63,"Routine":"DropIndex","UnknownFields":null}
 {"Type":"CommandComplete","CommandTag":"DROP INDEX"}
 
 until noncrdb_only
diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/drop_index.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/drop_index.go
index 1b2855dd3729..5197b4b53d9b 100644
--- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/drop_index.go
+++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/drop_index.go
@@ -59,9 +59,9 @@ func DropIndex(b BuildCtx, n *tree.DropIndex) {
 			pgnotice.Newf("CONCURRENTLY is not required as all indexes are dropped concurrently"))
 	}
 
+	var anyIndexesDropped bool
 	for _, index := range n.IndexList {
-		dropAnIndex(b, index, n.IfExists, n.DropBehavior)
-
+		anyIndexesDropped = dropAnIndex(b, index, n.IfExists, n.DropBehavior) || anyIndexesDropped
 		// Increment subwork ID so we know exactly which portion in
 		// a `DROP INDEX index1, index2, ...` statement is responsible
 		// for the creation of the targets.
@@ -69,20 +69,22 @@ func DropIndex(b BuildCtx, n *tree.DropIndex) {
 		b.IncrementSchemaChangeDropCounter("index")
 
 	}
-	b.EvalCtx().ClientNoticeSender.BufferClientNotice(
-		b,
-		errors.WithHint(
-			pgnotice.Newf("the data for dropped indexes is reclaimed asynchronously"),
-			"The reclamation delay can be customized in the zone configuration for the table.",
-		),
-	)
+	if anyIndexesDropped {
+		b.EvalCtx().ClientNoticeSender.BufferClientNotice(
+			b,
+			errors.WithHint(
+				pgnotice.Newf("the data for dropped indexes is reclaimed asynchronously"),
+				"The reclamation delay can be customized in the zone configuration for the table.",
+			),
+		)
+	}
 }
 
 // dropAnIndex resolves `index` and mark its constituent elements as ToAbsent
 // in the builder state enclosed by `b`.
 func dropAnIndex(
 	b BuildCtx, indexName *tree.TableIndexName, ifExists bool, dropBehavior tree.DropBehavior,
-) {
+) (indexDropped bool) {
 	toBeDroppedIndexElms := b.ResolveIndexByName(indexName, ResolveParams{
 		IsExistenceOptional: ifExists,
 		RequiredPrivilege:   privilege.CREATE,
@@ -90,7 +92,7 @@ func dropAnIndex(
 	if toBeDroppedIndexElms == nil {
 		// Attempt to resolve this index failed but `IF EXISTS` is set.
 		b.MarkNameAsNonExistent(&indexName.Table)
-		return
+		return false
 	}
 	// Panic if dropping primary index.
 	_, _, pie := scpb.FindPrimaryIndex(toBeDroppedIndexElms)
@@ -118,6 +120,7 @@ func dropAnIndex(
 		))
 	}
 	dropSecondaryIndex(b, indexName, dropBehavior, sie)
+	return true
 }
 
 // dropSecondaryIndex is a helper to drop a secondary index which may be used