Re: logical decoding and replication of sequences, take 2

Hi,

I have been hacking on improving the improvements outlined in my
preceding e-mail, but I have some bad news - I ran into an issue that I
don't know how to solve :-(

Consider this transaction:

  BEGIN;
  ALTER SEQUENCE s RESTART 1000;

  SAVEPOINT s1;
  ALTER SEQUENCE s RESTART 2000;
  ROLLBACK TO s1;

  INSERT INTO seq_test SELECT nextval('s') FROM generate_series(1,40);
  COMMIT;

If you try this with the approach relying on rd_newRelfilelocatorSubid
and rd_createSubid, it fails like this on the subscriber:

  ERROR:  could not map filenode "base/5/16394" to relation OID

This happens because ReorderBufferQueueSequence tries to do this in the
non-transactional branch:

  reloid = RelidByRelfilenumber(rlocator.spcOid, rlocator.relNumber);

and the relfilenode is the one created by the first ALTER. But this is
obviously wrong - the changes should have been treated as transactional,
because they are tied to the first ALTER. So how did we get there?

Well, the whole problem is that in case of abort, AtEOSubXact_cleanup
resets the two fields to InvalidSubTransactionId. Which means the
rollback in the above transaction also forgets about the first ALTER.
Now that I look at the RelationData comments, it actually describes
exactly this situation:

  *
  * rd_newRelfilelocatorSubid is the ID of the highest subtransaction
  * the most-recent relfilenumber change has survived into or zero if
  * not changed in the current transaction (or we have forgotten
  * changing it).  This field is accurate when non-zero, but it can be
  * zero when a relation has multiple new relfilenumbers within a
  * single transaction, with one of them occurring in a subsequently
  * aborted subtransaction, e.g.
  *    BEGIN;
  *    TRUNCATE t;
  *    SAVEPOINT save;
  *    TRUNCATE t;
  *    ROLLBACK TO save;
  *    -- rd_newRelfilelocatorSubid is now forgotten
  *

The root of this problem is that we'd need some sort of "history" for
the field, so that when a subxact aborts, we can restore the previous
value. But we obviously don't have that, and I doubt we want to add that
to relcache - for example, it'd either need to impose some limit on the
history (and thus a failure when we reach the limit), or it'd need to
handle histories of arbitrary length.

At this point I don't see a solution for this, which means the best way
forward with the sequence decoding patch seems to be the original
approach, on the decoding side.

I'm attaching the patch with 0005 and 0006, adding two simple tests (no
other changes compared to yesterday's version).


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company