From 565217711f16b8b93b2bc145d8ad20b78ea9825b Mon Sep 17 00:00:00 2001 From: Eliran Ben-Zikri Date: Fri, 28 Aug 2026 19:03:10 +0300 Subject: [PATCH 1/2] Deallocate a failed prepare by the name sent in Parse When Prepare is called with name == sql (the stdlib/database/sql path), the statement is prepared on the server under stmt_ while the client keys it by the SQL text. The cleanup for a prepare that failed during Describe stored that client key and later deallocated by it, sending Close for a statement name that does not exist. The real stmt_ leaked on the server, and any retry of the same SQL on that connection failed with 42P05 duplicate_prepared_statement for the rest of the connection's life. The cleanup's own round trip could also surface its errors on whatever query next prepared on the connection, quoting the old query's full SQL. Store the name actually sent in Parse so the deallocation closes the real statement. The named-prepare path is unchanged: there the key and the wire name are the same. https://github.com/jackc/pgx/issues/2640 --- conn.go | 5 +++- conn_test.go | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/conn.go b/conn.go index 229c380c5..5e0b9cf51 100644 --- a/conn.go +++ b/conn.go @@ -360,7 +360,10 @@ func (c *Conn) Prepare(ctx context.Context, name, sql string) (sd *pgconn.Statem if err != nil { var pErr *pgconn.PrepareError if errors.As(err, &pErr) { - c.failedDescribeStatement = psKey + // The server-side statement was created under psName — the name sent in + // Parse. In the name == sql case psKey is the SQL text, and deallocating + // by it would close a nonexistent statement while leaking the real one. + c.failedDescribeStatement = psName } return nil, err } diff --git a/conn_test.go b/conn_test.go index db3578b4b..c4d423218 100644 --- a/conn_test.go +++ b/conn_test.go @@ -3,7 +3,9 @@ package pgx_test import ( "bytes" "context" + "crypto/sha256" "database/sql" + "encoding/hex" "io" "net" "os" @@ -532,6 +534,80 @@ func TestPrepareHandlesTimeoutBetweenParseAndDescribe(t *testing.T) { require.NotNil(t, psd) } +// https://github.com/jackc/pgx/issues/2640 +func TestPrepareWithDigestedNameHandlesTimeoutBetweenParseAndDescribe(t *testing.T) { + // Not parallel because it is a timing sensitive test. + // + // stdlib (and therefore database/sql) calls Prepare(ctx, sql, sql). In that case the statement is prepared on the + // server under stmt_ while the client keys it by the SQL text. Cleanup after a Describe phase failure must + // deallocate the digest name — deallocating by the SQL text closes a nonexistent statement, so the leaked + // statement stays on the server and re-preparing the same SQL fails with 42P05 for the rest of the connection's + // life. + + config, err := pgx.ParseConfig(os.Getenv("PGX_TEST_DATABASE")) + require.NoError(t, err) + + var faultyConn *faultyconn.Conn + config.AfterNetConnect = func(ctx context.Context, config *pgconn.Config, conn net.Conn) (net.Conn, error) { + faultyConn = faultyconn.New(conn) + return faultyConn, nil + } + + ctx := context.Background() + conn, err := pgx.ConnectConfig(ctx, config) + require.NoError(t, err) + defer closeConn(t, conn) + require.NotNil(t, faultyConn) + + pgxtest.SkipCockroachDB(t, conn, "Induced error does not occur on CockroachDB") + + _, err = conn.Exec(ctx, "set statement_timeout = '100ms'") + require.NoError(t, err) + + faultyConn.HandleFrontendMessage = func(backendWriter io.Writer, msg pgproto3.FrontendMessage) error { + if _, ok := msg.(*pgproto3.Describe); ok { + time.Sleep(200 * time.Millisecond) + } + buf, err := msg.Encode(nil) + if err != nil { + return err + } + _, err = backendWriter.Write(buf) + return err + } + + sql := "select $1::varchar" + digest := sha256.Sum256([]byte(sql)) + psName := "stmt_" + hex.EncodeToString(digest[0:24]) + + psd, err := conn.Prepare(ctx, sql, sql) + var pgErr *pgconn.PgError + require.ErrorAs(t, err, &pgErr) + require.Equal(t, "57014", pgErr.Code) + require.Nil(t, psd) + + faultyConn.HandleFrontendMessage = nil + + _, err = conn.Exec(ctx, "set statement_timeout = default") + require.NoError(t, err) + + var existsOnServer bool + err = conn.QueryRow( + ctx, + "select exists(select 1 from pg_prepared_statements where name = $1)", + // Avoid using the prepared statement cache or it will clear the broken statement before we can check for its + // existence. + pgx.QueryExecModeExec, + psName, + ).Scan(&existsOnServer) + require.NoError(t, err) + require.True(t, existsOnServer) + + psd, err = conn.Prepare(ctx, sql, sql) + require.NoError(t, err) + require.NotNil(t, psd) +} + func TestPrepareBadSQLFailure(t *testing.T) { t.Parallel() From 8f4925c0e747789e7939147e1ec046e97738d5f4 Mon Sep 17 00:00:00 2001 From: Eliran Ben-Zikri Date: Fri, 28 Aug 2026 19:04:04 +0300 Subject: [PATCH 2/2] Skip failed-prepare cleanup when Parse never completed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A PrepareError with ParseComplete == false means the server rejected the Parse itself — no statement was created, so there is nothing to deallocate. Scheduling the cleanup anyway cost a wasted round trip at the start of the next Prepare on the connection after every failed prepare (e.g. any syntax error), and gave that round trip a chance to fail and surface its error on an unrelated query. https://github.com/jackc/pgx/issues/2640 --- conn.go | 4 +++- conn_test.go | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/conn.go b/conn.go index 5e0b9cf51..1e60af2d8 100644 --- a/conn.go +++ b/conn.go @@ -359,10 +359,12 @@ func (c *Conn) Prepare(ctx context.Context, name, sql string) (sd *pgconn.Statem sd, err = c.pgConn.Prepare(ctx, psName, sql, nil) if err != nil { var pErr *pgconn.PrepareError - if errors.As(err, &pErr) { + if errors.As(err, &pErr) && pErr.ParseComplete { // The server-side statement was created under psName — the name sent in // Parse. In the name == sql case psKey is the SQL text, and deallocating // by it would close a nonexistent statement while leaking the real one. + // When Parse never completed no statement was created at all, so there + // is nothing to clean up. c.failedDescribeStatement = psName } return nil, err diff --git a/conn_test.go b/conn_test.go index c4d423218..60fd63e88 100644 --- a/conn_test.go +++ b/conn_test.go @@ -608,6 +608,55 @@ func TestPrepareWithDigestedNameHandlesTimeoutBetweenParseAndDescribe(t *testing require.NotNil(t, psd) } +// https://github.com/jackc/pgx/issues/2640 +func TestPrepareFailedParseSchedulesNoCleanup(t *testing.T) { + t.Parallel() + + // A Prepare that fails before Parse completes leaves no statement on the server, so the next Prepare must not + // spend a round trip deallocating anything. + + config, err := pgx.ParseConfig(os.Getenv("PGX_TEST_DATABASE")) + require.NoError(t, err) + + var faultyConn *faultyconn.Conn + config.AfterNetConnect = func(ctx context.Context, config *pgconn.Config, conn net.Conn) (net.Conn, error) { + faultyConn = faultyconn.New(conn) + return faultyConn, nil + } + + ctx := context.Background() + conn, err := pgx.ConnectConfig(ctx, config) + require.NoError(t, err) + defer closeConn(t, conn) + require.NotNil(t, faultyConn) + + sql := "select foo" + psd, err := conn.Prepare(ctx, sql, sql) + require.Error(t, err) + require.Nil(t, psd) + var pErr *pgconn.PrepareError + require.ErrorAs(t, err, &pErr) + require.False(t, pErr.ParseComplete) + + var sentClose bool + faultyConn.HandleFrontendMessage = func(backendWriter io.Writer, msg pgproto3.FrontendMessage) error { + if _, ok := msg.(*pgproto3.Close); ok { + sentClose = true + } + buf, err := msg.Encode(nil) + if err != nil { + return err + } + _, err = backendWriter.Write(buf) + return err + } + + psd, err = conn.Prepare(ctx, "select 1", "select 1") + require.NoError(t, err) + require.NotNil(t, psd) + require.False(t, sentClose) +} + func TestPrepareBadSQLFailure(t *testing.T) { t.Parallel()