Skip to content

Commit

Permalink
sql: Auto-commit before DDL in BIND
Browse files Browse the repository at this point in the history
Several parts of conn_executor_*go already auto-commit transactions
before handling DDL commands. However, we were missing the auto-commit
for the BIND command. Without it, binding a prepared statement could
trigger the auto-commit after the bind, which closes the portal and
leads to unknown portal errors.

Epic: none
Release note (bug fix): Fixed a bug when running with
autocommit_before_ddl that could cause a runtime error when binding a
previously prepared DDL statement.

Fixes #141576
Fixes #141881
  • Loading branch information
spilchen committed Feb 26, 2025
1 parent 8cd9b03 commit 1b6191d
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 37 deletions.
5 changes: 5 additions & 0 deletions pkg/sql/conn_executor_prepare.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,11 @@ func (ex *connExecutor) execBind(
}
}

// Check if we need to auto-commit the transaction due to DDL.
if ev, payload := ex.maybeAutoCommitBeforeDDL(ctx, ps.AST); ev != nil {
return ev, payload
}

portalName := bindCmd.PortalName
// The unnamed portal can be freely overwritten.
if portalName != "" {
Expand Down
113 changes: 113 additions & 0 deletions pkg/sql/pgwire/testdata/pgtest/schema_changes_implicit_txn
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
# This test exercises running schema changes in an implicit transaction using
# the extended protocol with Bind operations. This is a regression test for
# issue #82921.

send
Query {"String": "DROP TABLE IF EXISTS \"User\""}
Query {"String": "CREATE TABLE \"User\"\r\n(\r\n \"UserID\" integer primary key\r\n);"}
----

until
ReadyForQuery
ReadyForQuery
----
{"Type":"CommandComplete","CommandTag":"DROP TABLE"}
{"Type":"ReadyForQuery","TxStatus":"I"}
{"Type":"CommandComplete","CommandTag":"CREATE TABLE"}
{"Type":"ReadyForQuery","TxStatus":"I"}

send
Parse {"Query": "ALTER TABLE \"User\" ADD \"SponsorID\" INT NULL;"}
Bind
Execute
Parse {"Query": "CREATE INDEX User_SponsorID_IDX ON \"User\" (\"SponsorID\");"}
Bind
Describe {"ObjectType": "P", "Name": ""}
Execute
Sync
----

until crdb_only
ReadyForQuery
----
{"Type":"ParseComplete"}
{"Type":"BindComplete"}
{"Type":"CommandComplete","CommandTag":"ALTER TABLE"}
{"Severity":"NOTICE","SeverityUnlocalized":"NOTICE","Code":"00000","Message":"auto-committing transaction before processing DDL due to autocommit_before_ddl setting","Detail":"","Hint":"","Position":0,"InternalPosition":0,"InternalQuery":"","Where":"","SchemaName":"","TableName":"","ColumnName":"","DataTypeName":"","ConstraintName":"","File":"conn_executor_ddl.go","Line":0,"Routine":"maybeAutoCommitBeforeDDL","UnknownFields":null}
{"Type":"ParseComplete"}
{"Type":"BindComplete"}
{"Type":"NoData"}
{"Type":"CommandComplete","CommandTag":"CREATE INDEX"}
{"Type":"ReadyForQuery","TxStatus":"I"}

until noncrdb_only
ReadyForQuery
----
{"Type":"ParseComplete"}
{"Type":"BindComplete"}
{"Type":"CommandComplete","CommandTag":"ALTER TABLE"}
{"Type":"ParseComplete"}
{"Type":"BindComplete"}
{"Type":"NoData"}
{"Type":"CommandComplete","CommandTag":"CREATE INDEX"}
{"Type":"ReadyForQuery","TxStatus":"I"}

# This is a reproduction of the issue reported in GH #141576. It ensures that
# auto-commit occurs during the bind phase, which is necessary when using
# prepared statements.
subtest autocommit_for_bind

send
Query {"String": "DROP TABLE IF EXISTS t"}
Query {"String": "CREATE TABLE t (a INT PRIMARY KEY)"}
----

until ignore=NoticeResponse
ReadyForQuery
ReadyForQuery
----
{"Type":"CommandComplete","CommandTag":"DROP TABLE"}
{"Type":"ReadyForQuery","TxStatus":"I"}
{"Type":"CommandComplete","CommandTag":"CREATE TABLE"}
{"Type":"ReadyForQuery","TxStatus":"I"}

send
Parse {"Name": "ddl_stmt_t", "Query": "DROP TABLE t"}
Sync
----

until
ReadyForQuery
----
{"Type":"ParseComplete"}
{"Type":"ReadyForQuery","TxStatus":"I"}

send
Query {"String": "BEGIN"}
Bind {"PreparedStatement": "ddl_stmt_t"}
Execute
Sync
----

until crdb_only
ReadyForQuery
ReadyForQuery
----
{"Type":"CommandComplete","CommandTag":"BEGIN"}
{"Type":"ReadyForQuery","TxStatus":"T"}
{"Severity":"NOTICE","SeverityUnlocalized":"NOTICE","Code":"00000","Message":"auto-committing transaction before processing DDL due to autocommit_before_ddl setting","Detail":"","Hint":"","Position":0,"InternalPosition":0,"InternalQuery":"","Where":"","SchemaName":"","TableName":"","ColumnName":"","DataTypeName":"","ConstraintName":"","File":"conn_executor_ddl.go","Line":0,"Routine":"maybeAutoCommitBeforeDDL","UnknownFields":null}
{"Type":"BindComplete"}
{"Type":"CommandComplete","CommandTag":"DROP TABLE"}
{"Type":"ReadyForQuery","TxStatus":"I"}

until noncrdb_only
ReadyForQuery
ReadyForQuery
----
{"Type":"CommandComplete","CommandTag":"BEGIN"}
{"Type":"ReadyForQuery","TxStatus":"T"}
{"Type":"BindComplete"}
{"Type":"CommandComplete","CommandTag":"DROP TABLE"}
{"Type":"ReadyForQuery","TxStatus":"T"}

subtest end

This file was deleted.

0 comments on commit 1b6191d

Please sign in to comment.