Re: BUG #19686: Rolling back SET TABLESPACE

Hi Shihao,

Thanks for catching this.

> A sql_drop event trigger also runs after the ALTER, and v5 only
> checks ddl_command_end.

Right, and it is not the only one. An ALTER that also adds a CHECK
constraint validates it on the table's inheritance children after the
table itself was moved, so a volatile check function can write to the
table and then fail. That needs no event trigger, so no superuser: an
ordinary role that owns the table leaves 50 stale index entries with
v5, and your diff does not cover it.

Rather than list every such place, v6 keeps the shortcut only for a
plain ALTER TABLE ... SET TABLESPACE of a single table, with no other
subcommand. Then nothing else is in the work queue, nothing is dropped
and nothing is validated, so a ddl_command_end trigger and a pipeline
are all that remain, as in v5. Any combined ALTER copies the indexes.

Stale entries after the abort, for v5, v5 with your diff, and v6:

sql_drop trigger: 50, 0, 0
CHECK on an inheritance child: 50, 50, 0
ddl_command_end trigger, pipeline: 0, 0, 0

A plain SET TABLESPACE still leaves the index files alone. The
regression test gains the child CHECK case. It passes on master,
REL_15 and REL_14 (243, 217 and 216 tests), and the standby, crash
and wal_level=minimal checks stay clean.

> Also, ALTER TABLE ALL IN TABLESPACE always copies the indexes. Is
> that on purpose?

Yes. It moves each table through AlterTableInternal(), which has no
utility context, and moves them all in one transaction, which can
also be a transaction block, so the shortcut's condition cannot hold
there.

Attached are v6 for master, REL_15 and REL_14, and the change from v5.

Regards,
Manu