diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 88031c0c5fe..c4bf9fab96a 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -17621,8 +17621,9 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode, /* * The heap now has a new relfilenode. Give each of the table's indexes a - * fresh relfilenode too, so that the indexes share the heap's rewrite fate - * across commit and abort. See ATExecSetTableSpaceNewIndexRelfilenumber. + * fresh relfilenode too, so that the indexes share the heap's rewrite + * fate across commit and abort. See + * ATExecSetTableSpaceNewIndexRelfilenumber. */ foreach(lc, reltabidxids) ATExecSetTableSpaceNewIndexRelfilenumber(lfirst_oid(lc), lockmode); @@ -17639,12 +17640,16 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode, * sql_drop and table_rewrite event triggers, and more -- the files are kept * only for the one case where none of them exists: a plain * "ALTER TABLE ... SET TABLESPACE" of a single table, with no other - * subcommand, issued as a top-level statement outside any transaction block. - * What can still run after it is a ddl_command_end event trigger, and a - * further statement of an extended-protocol pipeline, which would share the - * transaction until the next Sync. Rule out the former by looking for such - * triggers, and the latter by forcing the commit right after the ALTER, as - * PreventInTransactionBlock does. + * subcommand, sent by a client as the only statement of a simple Query + * message, outside any transaction block: the transaction then ends right + * after it. What can still run after it is a ddl_command_end event trigger, + * so that case also requires that there is none. + * + * A statement sent by an extended-protocol Execute message always copies: + * later statements of the same pipeline share its transaction until the next + * Sync, and the backend cannot tell whether any will follow. Forcing a commit + * after the ALTER would make it commit on its own when a later statement of + * the pipeline fails, which is not how ALTER TABLE behaves otherwise. */ static bool ATSetTableSpaceCopyIndexes(AlterTableStmt *parsetree, List *wqueue, @@ -17652,6 +17657,8 @@ ATSetTableSpaceCopyIndexes(AlterTableStmt *parsetree, List *wqueue, { if (context == NULL || IsInTransactionBlock(context->isTopLevel)) return true; + if (MyBackendType != B_BACKEND || IsExtendedQueryMessage()) + return true; if (parsetree == NULL || list_length(parsetree->cmds) != 1 || castNode(AlterTableCmd, linitial(parsetree->cmds))->subtype != AT_SetTableSpace || list_length(wqueue) != 1) @@ -17659,7 +17666,6 @@ ATSetTableSpaceCopyIndexes(AlterTableStmt *parsetree, List *wqueue, if (EventCacheLookup(EVT_DDLCommandEnd) != NIL) return true; - MyXactFlags |= XACT_FLAGS_NEEDIMMEDIATECOMMIT; return false; } @@ -17692,9 +17698,10 @@ ATExecSetTableSpaceNewIndexRelfilenumber(Oid indexOid, LOCKMODE lockmode) * Only plain indexes have storage that can hold the stale entries. An * index whose current file was created in this subtransaction (the index * itself, or a new file for it) is discarded together with the heap's new - * file on abort, so it needs no copy. rd_newRelfilelocatorSubid can be zero after - * a rollback to a savepoint even though the file is new; then this falls - * back to rd_createSubid, and copies if that does not match either. + * file on abort, so it needs no copy. rd_newRelfilelocatorSubid can be + * zero after a rollback to a savepoint even though the file is new; then + * this falls back to rd_createSubid, and copies if that does not match + * either. */ if (ind->rd_rel->relkind != RELKIND_INDEX || !RELKIND_HAS_STORAGE(ind->rd_rel->relkind) || diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index b6bdfe213fe..1663cf7aea3 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -2457,6 +2457,23 @@ check_log_statement(List *stmt_list) return false; } +/* + * IsExtendedQueryMessage + * Is the current statement being run by an extended-query-protocol + * message? + * + * Such a statement shares its implicit transaction with whatever else the + * client sends before the next Sync, which the backend cannot know while it + * runs the statement. The first statement of a pipeline cannot be told from + * one followed directly by Sync: XACT_FLAGS_PIPELINING is set only once an + * Execute completes. + */ +bool +IsExtendedQueryMessage(void) +{ + return doing_extended_query_message; +} + /* * check_log_duration * Determine whether current command's duration should be logged diff --git a/src/include/tcop/tcopprot.h b/src/include/tcop/tcopprot.h index 5bc5bcfb20d..99e83189946 100644 --- a/src/include/tcop/tcopprot.h +++ b/src/include/tcop/tcopprot.h @@ -86,6 +86,7 @@ pg_noreturn extern void PostgresMain(const char *dbname, const char *username); extern void ResetUsage(void); extern void ShowUsage(const char *title); +extern bool IsExtendedQueryMessage(void); extern int check_log_duration(char *msec_str, bool was_logged); extern void set_debug_options(int debug_flag, GucContext context, GucSource source); diff --git a/src/test/regress/expected/tablespace.out b/src/test/regress/expected/tablespace.out index aec469aee9f..349a2d3a9b1 100644 --- a/src/test/regress/expected/tablespace.out +++ b/src/test/regress/expected/tablespace.out @@ -965,6 +965,17 @@ SELECT pg_relation_filepath('tbspace_rollback_idx') = :'idx_path' AS idx_file_ke t (1 row) +-- Sent through the extended query protocol, a later statement of the same +-- pipeline could share its transaction, so the indexes get new files. +ALTER TABLE tbspace_rollback SET TABLESPACE pg_default \bind \g +SELECT pg_relation_filepath('tbspace_rollback_idx') <> :'idx_path' AS idx_file_new; + idx_file_new +-------------- + t +(1 row) + +ALTER TABLE tbspace_rollback SET TABLESPACE regress_tblspace; +SELECT pg_relation_filepath('tbspace_rollback_idx') AS idx_path \gset -- In a transaction block the indexes get new files, so that a rollback -- discards the entries added after the move together with the heap's file. BEGIN; diff --git a/src/test/regress/sql/tablespace.sql b/src/test/regress/sql/tablespace.sql index 52cf5d5a09e..e210409fb00 100644 --- a/src/test/regress/sql/tablespace.sql +++ b/src/test/regress/sql/tablespace.sql @@ -429,6 +429,12 @@ SELECT pg_relation_size('tbspace_rollback_idx') AS idx_size_before \gset SELECT pg_relation_filepath('tbspace_rollback_idx') AS idx_path \gset ALTER TABLE tbspace_rollback SET TABLESPACE regress_tblspace; SELECT pg_relation_filepath('tbspace_rollback_idx') = :'idx_path' AS idx_file_kept; +-- Sent through the extended query protocol, a later statement of the same +-- pipeline could share its transaction, so the indexes get new files. +ALTER TABLE tbspace_rollback SET TABLESPACE pg_default \bind \g +SELECT pg_relation_filepath('tbspace_rollback_idx') <> :'idx_path' AS idx_file_new; +ALTER TABLE tbspace_rollback SET TABLESPACE regress_tblspace; +SELECT pg_relation_filepath('tbspace_rollback_idx') AS idx_path \gset -- In a transaction block the indexes get new files, so that a rollback -- discards the entries added after the move together with the heap's file. BEGIN;