From e656f0de479863adfdaafe033697d85e9b544068 Mon Sep 17 00:00:00 2001
From: Matt Spilchen <matt.spilchen@cockroachlabs.com>
Date: Wed, 23 Oct 2024 14:17:44 +0000
Subject: [PATCH] sqlccl: deflake TestExplainGist when run with concurrent
 ALTER PK
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

TestExplainGist occasionally fails when a query using a secondary index
tries to fetch a column not included in that index (see issue #130282).
This change doesn’t address the root cause, but instead ignores the
error when it occurs. I've also created a more reliable reproducer in
the TestDMLInjectionTest, which we can use to validate the eventual fix
(#133129).

Epic: none
Closes #130282
Release note: none
---
 pkg/ccl/testccl/sqlccl/explain_test.go      |  1 +
 pkg/sql/schemachanger/dml_injection_test.go | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/pkg/ccl/testccl/sqlccl/explain_test.go b/pkg/ccl/testccl/sqlccl/explain_test.go
index 87aaf389216f..e997ecd2723a 100644
--- a/pkg/ccl/testccl/sqlccl/explain_test.go
+++ b/pkg/ccl/testccl/sqlccl/explain_test.go
@@ -191,6 +191,7 @@ func TestExplainGist(t *testing.T) {
 				for _, knownErr := range []string{
 					"invalid datum type given: RECORD, expected RECORD", // #117101
 					"expected equivalence dependants to be its closure", // #119045
+					"not in index", // #133129
 				} {
 					if strings.Contains(err.Error(), knownErr) {
 						// Don't fail the test on a set of known errors.
diff --git a/pkg/sql/schemachanger/dml_injection_test.go b/pkg/sql/schemachanger/dml_injection_test.go
index 2c21ff74d285..88b03d5eb997 100644
--- a/pkg/sql/schemachanger/dml_injection_test.go
+++ b/pkg/sql/schemachanger/dml_injection_test.go
@@ -116,6 +116,9 @@ type testCase struct {
 	schemaChange string
 	expectedErr  string
 	skipIssue    int
+	// Optional: If you want a query to run at each stage, you can include it here.
+	// We don't evaluate the results; we simply assert that the query executes without errors.
+	query string
 }
 
 // Captures testCase before t.Parallel is called.
@@ -299,6 +302,17 @@ func TestAlterTableDMLInjection(t *testing.T) {
 			},
 			schemaChange: "ALTER TABLE tbl ALTER PRIMARY KEY USING COLUMNS (insert_phase_ordinal, operation_phase_ordinal, operation)",
 		},
+		{
+			desc:        "alter primary key and replace rowid in PK",
+			createTable: createTableNoPK,
+			setup: []string{
+				"CREATE INDEX i1 ON tbl (val)",
+			},
+			// Run a query against the secondary index at each stage.
+			query:        "SELECT operation FROM tbl@i1",
+			schemaChange: "ALTER TABLE tbl ALTER PRIMARY KEY USING COLUMNS (insert_phase_ordinal, operation_phase_ordinal, operation)",
+			skipIssue:    133129,
+		},
 		{
 			desc:        "alter primary key using columns using hash",
 			createTable: createTableNoPK,
@@ -523,6 +537,12 @@ func TestAlterTableDMLInjection(t *testing.T) {
 								require.Subset(t, expectedResults, actualResults, errorMessage)
 								require.Subset(t, actualResults, expectedResults, errorMessage)
 
+								// If a query is provided, run it without checking the results—just
+								// ensure it doesn't fail.
+								if tc.query != "" {
+									sqlDB.Exec(t, tc.query)
+								}
+
 								for i := 0; i < poIdx; i++ {
 									insertPO := poSlice[i]
 									// Verify 1 row is correctly deleted.