Re: BUG #19686: Rolling back SET TABLESPACE

Hi Shihao,

> ATSetTableSpaceCopyIndexes() also runs for ALTER INDEX, so a plain
> ALTER INDEX SET TABLESPACE in a pipeline now commits on its own.
> Maybe only call it for tables and matviews.

Thanks. It is not only ALTER INDEX: as the first statement of a
pipeline, ALTER TABLE and ALTER MATERIALIZED VIEW also commit on their
own with v6, so a later failure no longer rolls the move back. The
forced commit itself is the problem.

v7 drops it. The shortcut is taken only when the ALTER is the only
statement of a simple Query message, outside a transaction block.
Through the extended protocol it always copies the indexes, since the
backend cannot tell whether more statements will come before Sync.
postgres.c gets a small accessor for that, IsExtendedQueryMessage().
In a pipeline whose next statement fails, the table, index and
matview now stay where they were, as on master.

> A plain top-level ALTER costs the same as master. In a transaction
> block it costs more.

With v7 that holds for the simple protocol only; through the extended
protocol, as most drivers send it, the ALTER copies the indexes. For
1M rows with 29 MB of indexes, master and v7:

WAL, plain ALTER: 35.6 MB, 35.6 MB
WAL, in a block or extended: 35.6 MB, 64.9 MB
AccessExclusiveLocks, in a block: 1, 3

The copy also needs free space where the indexes are. With them on a
nearly full tablespace, moving the table in a block fails on v7 with
"could not extend file"; running the ALTER on its own still works.

In the back branches the patch adds a field at the end of
AlterTableUtilityContext, which TimescaleDB and Apache AGE build to
call AlterTable(). The field is read only for SET TABLESPACE. Built
against the unpatched headers, both run unchanged on the patched
branches, and a garbage value can at worst skip the copy, as today.
If that is unwelcome, the back branches could always copy instead.

Attached are v7 for master (it applies to REL_19 and REL_18 as is),
REL_17, REL_16, REL_15 and REL_14, and the change from v6. Earlier
versions missed REL_17 and REL_16, where the master patch does not
apply. On every branch v7 passes check-world, the random savepoint
test and the standby and crash checks.

Regards,
Manu