Re: BUG #19686: Rolling back SET TABLESPACE

Thank you Manu,

Trying to keep iterations quick :)

On Tue, Sep 29, 2026 at 12:45 PM Manu <manuelreyesbravo(at)gmail(dot)com> wrote:

> Hi Al,
>
> I built v2 on master (82d31451606) with --enable-cassert and checked the
> two cases, plus your question.

v2 fixes both. The double SET TABLESPACE now lands right: a table moved
> to ts and then back to pg_default in one transaction ends in pg_default,
> where v1 left it in ts. The original recipe is still fixed: the
> rollback + INSERT btree case does not trap and bt_index_check reports
> nothing, against master where it fails the _bt_posting_valid assertion.

Yes

> One build problem: v2-0002 does not compile with --enable-cassert.

Fixed, using pointer casts directly (indentation looks weird to me, but that
was pg_indent's choice).

> > I just noticed that if someone check the pg_tablespaces inside the
> > transaction they will get unexpected results. (Is that something we
> > need to fix?)
>
> I can reproduce it. With v2, inside the transaction
> pg_class.reltablespace for an indexed table still reads the old
> tablespace until commit, since the whole move is deferred. On master
> SET TABLESPACE updates the catalog at execution time, so a query in the
> same transaction sees the new tablespace right away. So v2 does change
> that observable behavior.
>
> Whether it is worth fixing is your call. The one thing I'd note is that
> the alternative, showing the pending tablespace in the catalog within
> the transaction, would leave reltablespace pointing at a tablespace the
> file has not reached until commit, so it isn't only a matter of moving
> the catalog update earlier. I'll leave the design to you; I mainly
> wanted to confirm the behavior is real and that it differs from master.

v3 adds a paragraph about deferred copy and clarifies that catalog
updates might not be visible within the transaction, I think it is good
that it reflects the physical location of the relation. Reading the
preceding
paragraph I decided that it was worth adding a test case for partitioned
tables.

The tests for this are already getting long so I moved to a separate file,
tablespace_xact, in parallel with tablespace instead of appending on it.

Regards,
Alexandre