sql: migrate prepared statements during session migration

[rafiss]

fixes #76398

See individual commits. The last one contains the business logic changes,
and the rest are just refactors.

Release note (sql change): The crdb_internal.serialize_session and
crdb_internal.deserialize_session now can handle prepared statements.
When deserializing, any prepared statements that existed when the
session was serialized are re-prepared. It is an error to re-prepare a
statement if the current session already has a statement with that name.

[cockroach-teamcity]

This change is 

[otan]

changes lgtm.

one thing i am curious about - what happens to prepared statements if a schema change happened and the types change? in turn, does AddPreparedStatement during a session migration potentially change the result of the prepared statement producing an entirely different statement? is it worth testing?

[andy-kimball]

CC @RaduBerinde to make sure to take a look at this.

[rafiss]

what happens to prepared statements if a schema change happened and the types change?

This is the behavior without any session migration involved:

exec
CREATE TABLE t(a INT4, b TEXT, c DATE)
----

wire_prepare s2
INSERT INTO t VALUES($1, $2, $3)
----

exec
ALTER TABLE t DROP COLUMN c
----

wire_exec s2 1 cat 2022-02-10
----
ERROR: INSERT has more expressions than target columns, 3 expressions for 2 targets (SQLSTATE 42601)

in turn, does AddPreparedStatement during a session migration potentially change the result of the prepared statement producing an entirely different statement? is it worth testing?

yeah we definitely should test. given the above, i guess the two options are to not serialize a prepared statement if it's not valid at the time of serialization, or to ignore certain types of prepare errors at the time of deserialization.

[rafiss]

We could also keep the behavior as-is and just error out. (The user-impact is that their client would get a hard disconnect rather than a graceful session migration.)

[otan]

happy with erroring out. also happy for you to add this test in a separate PR.

[rafiss]

confirmed the behavior for prepared statements after a schema change at the last multi-tenant team meeting with signoff from @jeffswenson so going to merge this.

bors r=otan

[craig]

Canceled.

[rafiss]

bors r=otan

[jaylim-crl]

I believe this will fail:

cockroach/pkg/ccl/testccl/sqlccl/show_transfer_state_test.go

Lines 199 to 225 in 696cdd5

	t.Run("prepared_statements", func(t *testing.T) {
	pgURL, cleanup := sqlutils.PGUrl(
	t,
	tenant.SQLAddr(),
	"TestShowTransferState-errors-prepared_statements",
	url.UserPassword(security.TestUser, "hunter2"),
	)
	defer cleanup()
	conn, err := gosql.Open("postgres", pgURL.String())
	require.NoError(t, err)
	defer conn.Close()
	// Use a dummy prepared statement.
	stmt, err := conn.Prepare("SELECT 1 WHERE 1 = 1")
	require.NoError(t, err)
	defer stmt.Close()
	var errVal, sessionState, sessionRevivalToken gosql.NullString
	err = conn.QueryRow("SHOW TRANSFER STATE").Scan(&errVal, &sessionState, &sessionRevivalToken)
	require.NoError(t, err)
	require.True(t, errVal.Valid)
	require.Equal(t, "cannot serialize a session which has portals or prepared statements", errVal.String)
	require.False(t, sessionState.Valid)
	require.False(t, sessionRevivalToken.Valid)
	})

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jaylim-crl and @rafiss)

---

pkg/sql/types/types.go, line 1746 at r5 (raw file):

// messages
//and also to produce the output of SHOW CREATE.

nit: typo here?

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[rafiss]

ah, whoops. fixing

bors r-

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jaylim-crl)

---

pkg/sql/types/types.go, line 1746 at r5 (raw file):

Previously, jaylim-crl (Jay Lim) wrote…

// messages
//and also to produce the output of SHOW CREATE.

nit: typo here?

fixed

[craig]

Canceled.

[rafiss]

@jaylim-crl i edited that test, in case you want to take a look

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jaylim-crl)

[otan]

do you think this is causing the TestShowTransferState/errors/prepared_statements flake?

edit: nvm i see it is done above

[jaylim-crl]

@jaylim-crl i edited that test, in case you want to take a look

Thanks. The test LGTM.

Reviewed 2 of 2 files at r1, 2 of 2 files at r2, 4 of 4 files at r3, 13 of 16 files at r4, 4 of 4 files at r6, 3 of 3 files at r7, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)

[rafiss]

tftrs!

bors r=otan

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build failed:

• GitHub CI (Cockroach)

[rafiss]

the failures all seem to be flakes as far as i can tell..

bors r=otan

[craig]

Build failed:

• GitHub CI (Cockroach)

[rafiss]

yet another flake..

bors r=otan

[craig]

Build succeeded:

• GitHub CI (Cockroach)

[otan]

congrats on winning the CI lottery!