Skip to content

Commit 5befaa7

Browse files
committed
sql: do not evaluate AOST timestamp in session migrations
Release note (bug fix): Fixed a bug where a session migration performed by SHOW TRANSFER STATE would not handle prepared statements that used the AS OF SYSTEM TIME clause. Users who encountered this bug would see errors such as `expected 1 or 0 for number of format codes, got N`. This bug was present since v22.2.0.
1 parent a598a75 commit 5befaa7

File tree

3 files changed

+57
-8
lines changed

3 files changed

+57
-8
lines changed

pkg/ccl/testccl/sqlccl/show_transfer_state_test.go

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,12 @@ func TestShowTransferState(t *testing.T) {
4343
require.NoError(t, err)
4444
_, err = tenantDB.Exec("CREATE TYPE typ AS ENUM ('foo', 'bar')")
4545
require.NoError(t, err)
46+
_, err = tenantDB.Exec("CREATE TABLE tab (a INT4, b typ)")
47+
require.NoError(t, err)
48+
_, err = tenantDB.Exec("INSERT INTO tab VALUES (1, 'foo')")
49+
require.NoError(t, err)
50+
_, err = tenantDB.Exec("GRANT SELECT ON tab TO testuser")
51+
require.NoError(t, err)
4652

4753
testUserConn := tenant.SQLConnForUser(t, username.TestUser, "")
4854

@@ -90,12 +96,18 @@ func TestShowTransferState(t *testing.T) {
9096
defer func() { _ = conn.Close(ctx) }()
9197

9298
// Add a prepared statement to make sure SHOW TRANSFER STATE handles it.
93-
_, err = conn.Prepare(ctx, "prepared_stmt", "SELECT $1::INT4, 'foo'::typ WHERE 1 = 1")
99+
_, err = conn.Prepare(ctx, "prepared_stmt_const", "SELECT $1::INT4, 'foo'::typ WHERE 1 = 1")
100+
require.NoError(t, err)
101+
_, err = conn.Prepare(ctx, "prepared_stmt_aost", "SELECT a, b FROM tab AS OF SYSTEM TIME '-1us'")
94102
require.NoError(t, err)
95103

96104
var intResult int
97105
var enumResult string
98-
err = conn.QueryRow(ctx, "prepared_stmt", 1).Scan(&intResult, &enumResult)
106+
err = conn.QueryRow(ctx, "prepared_stmt_const", 1).Scan(&intResult, &enumResult)
107+
require.NoError(t, err)
108+
require.Equal(t, 1, intResult)
109+
require.Equal(t, "foo", enumResult)
110+
err = conn.QueryRow(ctx, "prepared_stmt_aost").Scan(&intResult, &enumResult)
99111
require.NoError(t, err)
100112
require.Equal(t, 1, intResult)
101113
require.Equal(t, "foo", enumResult)
@@ -171,14 +183,26 @@ func TestShowTransferState(t *testing.T) {
171183
// session.
172184
result := conn.PgConn().ExecPrepared(
173185
ctx,
174-
"prepared_stmt",
186+
"prepared_stmt_const",
175187
[][]byte{{0, 0, 0, 2}}, // binary representation of 2
176188
[]int16{1}, // paramFormats - 1 means binary
177-
[]int16{1}, // resultFormats - 1 means binary
189+
[]int16{1, 1}, // resultFormats - 1 means binary
178190
).Read()
191+
require.NoError(t, result.Err)
179192
require.Equal(t, [][][]byte{{
180193
{0, 0, 0, 2}, {0x66, 0x6f, 0x6f}, // binary representation of 2, 'foo'
181194
}}, result.Rows)
195+
result = conn.PgConn().ExecPrepared(
196+
ctx,
197+
"prepared_stmt_aost",
198+
[][]byte{}, // paramValues
199+
[]int16{}, // paramFormats
200+
[]int16{1, 1}, // resultFormats - 1 means binary
201+
).Read()
202+
require.NoError(t, result.Err)
203+
require.Equal(t, [][][]byte{{
204+
{0, 0, 0, 1}, {0x66, 0x6f, 0x6f}, // binary representation of 1, 'foo'
205+
}}, result.Rows)
182206
})
183207

184208
// Errors should be displayed as a SQL value.

pkg/sql/conn_executor_prepare.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -288,8 +288,12 @@ func (ex *connExecutor) prepare(
288288
}
289289

290290
// Use the existing transaction.
291-
if err := prepare(ctx, ex.state.mu.txn); err != nil && origin != PreparedStatementOriginSessionMigration {
292-
return nil, err
291+
if err := prepare(ctx, ex.state.mu.txn); err != nil {
292+
if origin != PreparedStatementOriginSessionMigration {
293+
return nil, err
294+
} else {
295+
log.Warningf(ctx, "could not prepare statement during session migration: %v", err)
296+
}
293297
}
294298

295299
// Account for the memory used by this prepared statement.
@@ -320,8 +324,15 @@ func (ex *connExecutor) populatePrepared(
320324
return 0, err
321325
}
322326
p.extendedEvalCtx.PrepareOnly = true
323-
if err := ex.handleAOST(ctx, p.stmt.AST); err != nil {
324-
return 0, err
327+
// If the statement is being prepared by a session migration, then we should
328+
// not evaluate the AS OF SYSTEM TIME timestamp. During session migration,
329+
// there is no way for the statement being prepared to be executed in this
330+
// transaction, so there's no need to fix the timestamp, unlike how we must
331+
// for pgwire- or SQL-level prepared statements.
332+
if origin != PreparedStatementOriginSessionMigration {
333+
if err := ex.handleAOST(ctx, p.stmt.AST); err != nil {
334+
return 0, err
335+
}
325336
}
326337

327338
// PREPARE has a limited subset of statements it can be run with. Postgres

pkg/sql/testdata/session_migration/prepared_statements

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ wire_prepare s4
3333
INSERT INTO t2 VALUES($1, $2)
3434
----
3535

36+
# Regression test for transferring statements with AOST.
37+
wire_prepare s5
38+
SELECT a, b FROM t2 AS OF SYSTEM TIME '-2us'
39+
----
40+
3641
wire_prepare s_empty
3742
;
3843
----
@@ -102,6 +107,15 @@ SELECT * FROM t2
102107
----
103108
1 cat
104109

110+
query
111+
SELECT pg_sleep(0.1)
112+
----
113+
true
114+
115+
wire_query s5
116+
----
117+
1 cat
118+
105119
reset
106120
----
107121

0 commit comments

Comments
 (0)