Hi Vignesh.
Some review comments for v20250426-0005.
======
doc/src/sgml/catalogs.sgml
1.
<para>
- State code:
+ State code for tables:
<literal>i</literal> = initialize,
<literal>d</literal> = data is being copied,
<literal>f</literal> = finished table copy,
<literal>s</literal> = synchronized,
<literal>r</literal> = ready (normal replication)
+ </para>
+ <para>
+ State code for sequences:
+ <literal>i</literal> = initialize,
+ <literal>r</literal> = ready
</para></entry>
1a.
There should be an introductory sentence to say what this field is.
e.g. "State code for the table or sequence."
~
1b.
/State code for tables/State codes for tables/
~
1c.
/State code for sequences/State codes for sequences/
======
doc/src/sgml/logical-replication.sgml
2.
+ or <literal>FOR ALL SEQUENCES</literal>. Unlike tables, sequences
allow users
+ to synchronize their current state at any given time. For more information,
+ refer to <xref linkend="logical-replication-sequences"/>.
This is OK, but maybe the "sequences allow users..." is worded
strangely. How about below?
SUGGESTION
Unlike tables, the current state of sequences may be synchronised at any time.
~~~
3.
+ Incremental sequence changes are not replicated. The data in serial or
+ identity columns backed by sequences will of course be replicated as part
+ of the table, the sequences themselves do not replicate ongoing changes.
Seems to be a missing word here
/The data in serial/Although the data in serial/
OR
Just change the punctuation to a semicolon.
/of the table,/of the table;/
======
doc/src/sgml/ref/alter_subscription.sgml
4.
+ <para>
+ Previously subscribed sequences are not re-synchronized. To do that,
+ see <link
linkend="sql-altersubscription-params-refresh-publication-sequences">
+ <command>ALTER SUBSCRIPTION ... REFRESH PUBLICATION
SEQUENCES</command></link>
+ </para>
4a.
Missing period in the last sentence.
~
4b.
AFAIK, when copy_data=false, then not only will *existing* sequences
not be synchronised, but even the *new* sequences will not be
synchronised. Effectively, when copy_data = false, then nothing at all
happens for sequences as far as what the user sees, right?
Experiment:
test_pub=# create publication pub1 for all sequences;
CREATE PUBLICATION
test_sub=# create sequence s1;
CREATE SEQUENCE
NOTICE: created replication slot "sub1" on publisher
CREATE SUBSCRIPTION
test_pub=# create sequence s1;
CREATE SEQUENCE
test_pub=# select * from nextval('s1');
nextval
---------
1
(1 row)
test_pub=# select * from nextval('s1');
nextval
---------
2
(1 row)
test_pub=# select * from nextval('s1');
nextval
---------
3
(1 row)
test_sub=# alter subscription sub1 refresh publication with (copy_data=false);
ALTER SUBSCRIPTION
test_sub=# select * from s1;
last_value | log_cnt | is_called
------------+---------+-----------
1 | 0 | f
(1 row)
So, subscriber side s1 is unaffected.
Maybe it is not worth the effort, but doesn't this mean that you could
optimise the AlterSubscription_refresh() logic to completely skip all
processing for sequences when copy_data=false. e.g. what's the point
of gathering publisher sequence lists and setting INIT states for
them, etc, when it won't synchronise anything because copy_data=false?
Everything will be synchronised later anyway when the user does
REFRESH PUBLICATION SEQUENCES.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
