Re: Logical Replication of sequences - Mailing list pgsql-hackers

>
> Thanks for the comment, the attached v20250622 version patch has the
> changes for the same.
>

Thanks for the patches, I am not done with review yet, but please find
the feedback so far:


1)
+ if (!OidIsValid(seq_relid))
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("sequence \"%s.%s\" does not exist",
+    schema_name, sequence_name));

ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE might not be a correct error
code here. Shall we have ERRCODE_UNDEFINED_OBJECT? Thoughts?


2)
tab-complete here shows correctly:

postgres=# CREATE PUBLICATION pub6 FOR ALL
SEQUENCES  TABLES

But tab-complete for these 2 commands does not show anything:

postgres=# CREATE PUBLICATION pub6 FOR ALL TABLES,
postgres=# CREATE PUBLICATION pub6 FOR ALL SEQUENCES,

We shall specify SEQUENCES/TABLES in above commands. IIUC, we do not
support any other combination like (table <name>, tables in schema
<name>) once we get ALL clause in command. So it is safe to display
tab-complete as either TABLES or SEQUENECS in above.

3)
postgres=#  CREATE publication pub1 for sequences;
ERROR:  invalid publication object list
LINE 1: CREATE publication pub1 for sequences;
                                 ^
DETAIL:  One of TABLE, TABLES IN SCHEMA or ALL SEQUENCES must be
specified before a standalone table or schema name.


This message is not correct as we can not have ALL SEQUENCES *before*
a standalone table or schema name. The problem is that gram.y is
taking *sequences* as a table or schema name. I noticed that it does
same with *tables* as well:

postgres=#  CREATE publication pub1 for tables;
ERROR:  invalid publication object list
LINE 1: CREATE publication pub1 for tables;
                                  ^
DETAIL:  One of TABLE, TABLES IN SCHEMA or ALL SEQUENCES must be
specified before a standalone table or schema name.


But since gram.y here can not identify an in-between missing keyword
*all*, thus it is considering it (tables/sequenecs) as a literal/name
instead of keyword. We can revert back to old message in such a case.
I am unable to think of a good solution here.


4)
I think the error here is wrong as we are not trying to specify
multiple all-tables entries.

postgres=# CREATE PUBLICATION pub6 for all tables, tables in schema public;
ERROR:  invalid publication object list
LINE 1: CREATE PUBLICATION pub6 for all tables, tables in schema pub...

                                                ^
DETAIL:  ALL TABLES can be specified only once.


5)
The log messages still has some scope of improvement:

2025-06-24 08:52:17.988 IST [110359] LOG:  logical replication
sequence synchronization worker for subscription "sub1" has started
2025-06-24 08:52:18.029 IST [110359] LOG:  logical replication
sequence synchronization for subscription "sub1" - total
unsynchronized: 3
2025-06-24 08:52:18.090 IST [110359] LOG:  logical replication
sequence synchronization for subscription "sub1" - batch #1 = 3
attempted, 0 succeeded, 2 mismatched, 1 missing
2025-06-24 08:52:18.091 IST [110359] ERROR:  logical replication
sequence synchronization failed for subscription "sub1"
2025-06-24 08:52:18.091 IST [110359] DETAIL:  Sequences
("public.myseq100") are missing on the publisher. Additionally,
parameters differ for the remote and local sequences
("public.myseq101", "public.myseq102").
2025-06-24 08:52:18.091 IST [110359] HINT:  Use ALTER SUBSCRIPTION ...
REFRESH PUBLICATION or use ALTER SUBSCRIPTION ... REFRESH PUBLICATION
SEQUENCES. Alter or re-create local sequences to have the same
parameters as the remote sequences


a)
Sequences ("public.myseq100") are missing on the publisher.
Additionally, parameters differ for the remote and local sequences
("public.myseq101", "public.myseq102").

Shall we change this to:
missing sequence(s) on publisher: ("public.myseq100"); mismatched
sequences(s) on subscriber: ("public.myseq101", "public.myseq102")

It will then be similar to the previous log pattern ( 3 attempted, 0
succeeded etc) instead of being more verbal. Thoughts?


b)
Hints are a little odd. First line of hint is just saying to use
'ALTER SUBSCRIPTION' without giving any purpose. While the second line
of hint is giving the purpose of alter-sequences.

Shall we have?
For missing sequences, use ALTER SUBSCRIPTION with either REFRESH
PUBLICATION or REFRESH PUBLICATION SEQUENCES
For mismatched sequences, alter or re-create local sequences to have
matching parameters as publishers.

Thoughts?

6)
postgres=# create publication pub1 for table tab1, all sequences;
ERROR:  syntax error at or near "all"
LINE 1: create publication pub1 for table tab1, all sequences;

We can mention in commit msg that this combination is also not
supported or what all combinations are supported. Currently it is not
clear f

thanks
Shveta