On Thu, May 29, 2025 at 8:09 PM vignesh C <vignesh21@gmail.com> wrote:
>
> These comments are handled in the attached v2025029 version patch.
>
1. The current syntax to publish sequences is:
CREATE PUBLICATION pub1 FOR ALL TABLES, ALL SEQUENCES;
The other alternative could be:
CREATE PUBLICATION pub1 FOR ALL TABLES, SEQUENCES;
I think the syntax proposed by the patch is better because of the
following reasons: (a) The use of ALL before both objects makes the
intent explicit and symmetrical. (b) As we are planning to support FOR
TABLE t1, SEQUENCE s1, so ALL SEQUENCES fit naturally as a counterpart
to ALL TABLES.
Please let me know if anyone thinks otherwise.
2. The 0001 patch has the following commit message:
"Introduce pg_sequence_state function for enhanced sequence
management. This patch introduces a new function, 'pg_sequence_state',
which
allows retrieval of sequence values, including the associated LSN."
It doesn't mention the use of introducing this new function.
Following comments on the 0004 patch
3.
List *
-GetSubscriptionRelations(Oid subid, bool not_ready)
+GetSubscriptionRelations(Oid subid, bool get_tables, bool get_sequences,
+ bool all_states)
What is the need to change the not_ready flag? Can't not_ready serve the need?
4.
+void
+SetSequence(Oid relid, int64 next, bool is_called, int64 log_cnt)
The argument order seems odd. Isn't it better to keep 'int64 log_cnt'
next to 'int64 next' and then a bool at the end?
5.
+ /*
+ * XXX: If the subscription is for a sequence-only publication, creating
+ * this origin is unnecessary. It can be created later during the ALTER
+ * SUBSCRIPTION ... REFRESH command, if the publication is updated to
+ * include tables.
+ */
Can you explain in comments why creating an origin is unnecessary? The
same is true for a similar comment on slots. If the patch has
explained it somewhere else, then mention a reference to that.
6. ALTER SUBSCRIPTION ... REFRESH PUBLICATION SEQUENCES
Can we move the implementation of the above command to a separate
patch? This is to make 0004 shorter and easier to review.
--
With Regards,
Amit Kapila.
