On 7/20/21 5:30 PM, Peter Eisentraut wrote:
> On 08.06.21 00:28, Tomas Vondra wrote:
>>
>> new "sequence" publication action
>> ---------------------------------
>>
>> The publications now have a new "sequence" publication action, which is
>> enabled by default. This determines whether the publication decodes
>> sequences or what.
>>
>>
>> FOR ALL SEQUENCES
>> -----------------
>>
>> It should be possible to create FOR ALL SEQUENCES publications, just
>> like we have FOR ALL TABLES. But this produces shift/reduce conflicts
>> in the grammar, and I didn't bother dealing with that. So for now it's
>> required to do ALTER PUBLICATION ... [ADD | DROP] SEQUENCE ...
>
> I have been thinking about these DDL-level issues a bit. The most
> common use case will be to have a bunch of tables with implicit
> sequences, and you just want to replicate them from here to there
> without too much effort. So ideally an implicit sequence should be
> replicated by default if the table is part of a publication (unless
> sequences are turned off by the publication option).
>
Agreed, that seems like a reasonable approach.
> We already have support for things like that in
> GetPublicationRelations(), where a partitioned table is expanded to
> include the actual partitions. I think that logic could be reused. So
> in general I would have GetPublicationRelations() include sequences and
> don't have GetPublicationSequenceRelations() at all. Then sequences
> could also be sent by pg_publication_tables(), maybe add a relkind
> column. And then you also don't need so much duplicate DDL code, if you
> just consider everything as a relation. For example, there doesn't seem
> to be an actual need to have fetch_sequence_list() and subsequent
> processing on the subscriber side. It does the same thing as
> fetch_table_list(), so it might as well just all be one thing.
>
Not sure. I agree with replicating implicit sequences by default,
without having to add them to the publication. But I think we should
allow adding other sequences too, and I think some of this code and
differentiation from tables will be needed.
FWIW I'm not claiming there are no duplicate parts - I've mostly
copy-pasted the table-handling code for sequences, and I'll look into
reusing some of it.
> We do, however, probably need some checking that we don't replicate
> tables to sequences or vice versa.
>
True. I haven't tried doing such silly things yet.
> We probably also don't need a separate FOR ALL SEQUENCES option. What
> users really want is a "for everything" option. We could think about
> renaming or alternative syntax, but in principle I think FOR ALL TABLES
> should include sequences by default.
>
> Tests under src/test/subscription/ are needed.
>
Yeah, true. At the moment there are just tests in test_decoding, mostly
because the previous patch versions did not add support for sequences to
built-in replication. Will fix.
> I'm not sure why test_decoding needs a skip-sequences option. The
> source code says it's for backward compatibility, but I don't see why we
> need that.
Hmmm, I'm a bit baffled by skip-sequences true. I think Cary added it to
limit chances in test_decoding tests, while the misleading comment about
backwards compatibility comes from me.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
