From 1b6191df09f62a296a51e7c832213cb44bab8d78 Mon Sep 17 00:00:00 2001 From: Matt Spilchen Date: Wed, 26 Feb 2025 12:21:28 -0400 Subject: [PATCH] sql: Auto-commit before DDL in BIND 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 --- pkg/sql/conn_executor_prepare.go | 5 + .../pgtest/schema_changes_implicit_txn | 113 ++++++++++++++++++ .../schema_changes_implicit_txn_with_bind | 37 ------ 3 files changed, 118 insertions(+), 37 deletions(-) create mode 100644 pkg/sql/pgwire/testdata/pgtest/schema_changes_implicit_txn delete mode 100644 pkg/sql/pgwire/testdata/pgtest/schema_changes_implicit_txn_with_bind diff --git a/pkg/sql/conn_executor_prepare.go b/pkg/sql/conn_executor_prepare.go index 9b636862e4cc..1e1ed37bc193 100644 --- a/pkg/sql/conn_executor_prepare.go +++ b/pkg/sql/conn_executor_prepare.go @@ -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 != "" { diff --git a/pkg/sql/pgwire/testdata/pgtest/schema_changes_implicit_txn b/pkg/sql/pgwire/testdata/pgtest/schema_changes_implicit_txn new file mode 100644 index 000000000000..9df7b26880ad --- /dev/null +++ b/pkg/sql/pgwire/testdata/pgtest/schema_changes_implicit_txn @@ -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 diff --git a/pkg/sql/pgwire/testdata/pgtest/schema_changes_implicit_txn_with_bind b/pkg/sql/pgwire/testdata/pgtest/schema_changes_implicit_txn_with_bind deleted file mode 100644 index cf47713cc351..000000000000 --- a/pkg/sql/pgwire/testdata/pgtest/schema_changes_implicit_txn_with_bind +++ /dev/null @@ -1,37 +0,0 @@ -# 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": "CREATE TABLE \"User\"\r\n(\r\n \"UserID\" integer primary key\r\n);"} ----- - -until -ReadyForQuery ----- -{"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 -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"}