Hi,
> I'm pretty doubtful that is a good idea. For one it'll make the performance
> effects very hard to understand.
> The outer query might have some buffers of the "old" index pinned etc
Agreed on both, so I've dropped the deferred copy. v5, attached, goes
back to copying the indexes in the ALTER, as v3 did, with the two
optimizations from your other mail, which Tom +1'd:
1) No copy when the ALTER is a top-level statement outside a transaction
block. AlterTableUtilityContext gets an isTopLevel field for that. Two
things can still write to the table in that transaction after the
ALTER, and v5 guards against both. A ddl_command_end event trigger: the
shortcut is off when one exists, which is cheap to check. A later
statement of an extended-protocol pipeline, which shares the transaction
until Sync: v5 sets XACT_FLAGS_NEEDIMMEDIATECOMMIT, as
PreventInTransactionBlock does, so a pipelined ALTER ... SET TABLESPACE
now commits on its own. With the guards removed, each case leaves 50
stale index entries; with v5, none.
2) No copy for an index whose current file was created in the current
subtransaction. It has to be the file's subtransaction, not the
transaction: an index created, reindexed or already copied before a
savepoint survives a rollback to it, and needs the copy. This also
covers your workaround for Tom's case: moving the indexes first and then
the table copies each index once. The regression test checks both.
For the back branches, isTopLevel goes at the end of
AlterTableUtilityContext; core is the only place I found that fills it.
make check passes on master, REL_15 and REL_14 (243, 217 and 216
tests). I also ran randomized transactions (nested savepoints, table
and index moves, REINDEX, DML, TRUNCATE, ROLLBACK TO, top-level moves),
comparing an index scan with a seq scan after each one: 40,000 with v5
and no mismatch, while the same test finds the bug on master. Results
are the same on a streaming standby, after an immediate-mode crash, and
with wal_level=minimal. The nested-query case can't come up now: the
ALTER refuses a table with an open scan ("being used by active
queries").
Regards,
Manu
