Re: logical decoding and replication of sequences

On 2/23/22 12:10, Amit Kapila wrote:
> On Wed, Feb 23, 2022 at 4:54 AM Tomas Vondra
> <tomas.vondra@enterprisedb.com> wrote:
>>
>> 7) This updates psql to do roughly the same thing as for tables, so \dRp
>> now list sequences added either directly or through schema, so you might
>> get footer like this:
>>
>>   \dRp+ testpub_mix
>>   ...
>>   Tables:
>>       "public.testpub_tbl1"
>>   Tables from schemas:
>>       "pub_test"
>>   Sequences:
>>       "public.testpub_seq1"
>>   Sequences from schemas:
>>       "pub_test"
>>
>> Maybe it's a bit too verbose, though. It also addes "All sequences" and
>> "Sequences" columns into the publication description, but I don't think
>> that can be done much differently.
>>
>> FWIW I had to switch the describeOneTableDetails() chunk dealing with
>> sequences from printQuery() to printTable() in order to handle dynamic
>> footers.
>>
>> There's also a change in \dn+ because a schema may be included in one
>> publication as "FOR ALL SEQUENCES IN SCHEMA" and in another publication
>> with "FOR ALL TABLES IN SCHEMA". So I modified the output to
>>
>>   \dn+ pub_test1
>>   ...
>>   Publications:
>>       "testpub_schemas" (sequences)
>>       "testpub_schemas" (tables)
>>
>> But maybe it'd be better to aggregate this into a single line like
>>
>>   \dn+ pub_test1
>>   ...
>>   Publications:
>>       "testpub_schemas" (tables, sequences)
>>
>> Opinions?
>>
> 
> I think the second one (aggregated) might be slightly better as that
> will lead to a lesser number of lines when there are multiple such
> publications but it should be okay if you and others prefer first.
> 

Maybe, but I don't think it's very common to have that many schemas
added to the same publication. And it probably does not make much
difference whether you have 1000 or 2000 items in the list - either both
are acceptable or unacceptable, I think.

But I plan to look at this a bit more.

>> 8) There's a shortcoming in the current grammar, because you can specify
>> either
>>
>>   CREATE PUBLICATION p FOR ALL TABLES;
>>
>> or
>>
>>   CREATE PUBLICATION p FOR ALL SEQUENCES;
>>
>> but it's not possible to do
>>
>>   CREATE PUBLICATION p FOR ALL TABLES AND FOR ALL SEQUENCES;
>>
>> which seems like a fairly reasonable thing users might want to do.
>>
> 
> Isn't it better to support this with a syntax as indicated by Tom in
> one of his earlier emails on this topic [1]? IIUC, it would be as
> follows:
> 
> CREATE PUBLICATION p FOR ALL TABLES, ALL SEQUENCES;
> 

Yes. That's mostly what I meant by adding this to PublicationObjSpec.

>> The problem is that "FOR ALL TABLES" (and same for sequences) is
>> hard-coded in the grammar, not defined as PublicationObjSpec. This also
>> means you can't do
>>
>>   ALTER PUBLICATION p ADD ALL TABLES;
>>
>> AFAICS there are two ways to fix this - adding the combinations into the
>> definition of CreatePublicationStmt, or adding FOR ALL TABLES (and
>> sequences) to PublicationObjSpec.
>>
> 
> I can imagine that adding to PublicationObjSpec will look compatible
> with existing code but maybe another way will also be okay.
> 

I think just hard-coding this into CreatePublicationStmt would work, but
it'll be an issue once/if we start adding more options. I'm not sure if
it makes sense to replicate other relkinds, but maybe DDL?

I'll try tweaking PublicationObjSpec, and we'll see.


regards

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