sql: properly reset extraTxnState in COPY

Apparently we support some sort of COPY protocol that I know nothing about.
We allow operations in that protocol in the open state and in the noTxn state
in the connExecutor. In the noTxn state we let the `copyMachine` handle its
transaction lifecycle all on its own. In that case, we also hand have the
`connExecutor` in a fresh state when calling `execCopyIn()`. During the
execution of `COPY`, it seems like sometime we can pick up table descriptor
leases. In the noTxn case we'd like to drop those leases and generally leave
the executor in the fresh state in which it was handed to us. To deal with
that, we call `resetExtraTxnState` before returning in the happy noTxn case.

Fixes cockroachdb#52065.

Release note (bug fix): Fixed a bug when using the COPY protocol which could
prevent schema changes for up to 5 minutes.

pkg/sql/conn_executor.go

@@ -1838,6 +1838,14 @@ func (ex *connExecutor) execCopyIn(
 		payload := eventNonRetriableErrPayload{err: err}
 		return ev, payload, nil
 	}
+	// If we let the copy-machine handle its own txn state then we need clean up
+	// anything it might have accumulated. In general this is just going to be
+	// releasing any leases which were acquired.
+	if isNoTxn {
+		if err := ex.resetExtraTxnState(ex.Ctx(), ex.server.dbCache, noEvent); err != nil {
+			return nil, nil, err
+		}
+	}
 	return nil, nil, nil
 }
 

pkg/sql/copy_in_test.go

@@ -15,6 +15,7 @@ import (
 	"fmt"
 	"math"
 	"math/rand"
+	"net/url"
 	"reflect"
 	"strconv"
 	"strings"
@@ -34,6 +35,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/timeofday"
 	"github.com/cockroachdb/cockroach/pkg/util/timetz"
 	"github.com/lib/pq"
+	"github.com/stretchr/testify/require"
 )
 
 func TestCopyNullInfNaN(t *testing.T) {
@@ -498,3 +500,46 @@ func TestCopyFKCheck(t *testing.T) {
 		t.Fatalf("expected FK error, got: %v", err)
 	}
 }
+
+// TestCopyInReleasesLeases if a regression test to ensure that the execution
+// of CopyIn does not retain table descriptor leases after completing by
+// attempting to run a schema change after performing a copy.
+func TestCopyInReleasesLeases(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	params, _ := tests.CreateTestServerParams()
+	s, db, _ := serverutils.StartServer(t, params)
+	tdb := sqlutils.MakeSQLRunner(db)
+	defer s.Stopper().Stop(context.Background())
+	tdb.Exec(t, `CREATE TABLE t (k INT8 PRIMARY KEY)`)
+	tdb.Exec(t, `CREATE USER foo WITH PASSWORD 'testabc'`)
+	tdb.Exec(t, `GRANT admin TO foo`)
+
+	userURL, cleanupFn := sqlutils.PGUrlWithOptionalClientCerts(t,
+		s.ServingSQLAddr(), t.Name(), url.UserPassword("foo", "testabc"),
+		false /* withClientCerts */)
+	defer cleanupFn()
+	conn, err := pgxConn(t, userURL)
+	require.NoError(t, err)
+
+	tag, err := conn.CopyFromReader(strings.NewReader("1\n2\n"),
+		"copy t(k) from stdin")
+	require.NoError(t, err)
+	require.Equal(t, int64(2), tag.RowsAffected())
+
+	// Prior to the bug fix which prompted this test, the below schema change
+	// would hang until the leases expire. Let's make sure it finishes "soon".
+	alterErr := make(chan error, 1)
+	go func() {
+		_, err := db.Exec(`ALTER TABLE t ADD COLUMN v INT NOT NULL DEFAULT 0`)
+		alterErr <- err
+	}()
+	select {
+	case err := <-alterErr:
+		require.NoError(t, err)
+	case <-time.After(testutils.DefaultSucceedsSoonDuration):
+		t.Fatal("alter did not complete")
+	}
+	require.NoError(t, conn.Close())
+}