Hi Vignesh, I reviewed the latest v20240808-0003 patch.

Attached are my minor change suggestions.

======
Kind Regards,
Peter Smith.
Fujitsu Australia
diff --git a/src/backend/catalog/pg_subscription.c 
b/src/backend/catalog/pg_subscription.c
index 4e2f960..a77e810 100644
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -517,9 +517,9 @@ HasSubscriptionTables(Oid subid)
  *
  * all_states:
  * If getting tables, if all_states is true get all tables, otherwise
- * only get tables that have not reached 'READY' state.
+ * only get tables that have not reached READY state.
  * If getting sequences, if all_states is true get all sequences,
- * otherwise only get sequences that are in 'init' state.
+ * otherwise only get sequences that are in INIT state.
  *
  * The returned list is palloc'ed in the current memory context.
  */
diff --git a/src/backend/commands/subscriptioncmds.c 
b/src/backend/commands/subscriptioncmds.c
index bb6aa8e..2833379 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -868,10 +868,12 @@ CreateSubscription(ParseState *pstate, 
CreateSubscriptionStmt *stmt,
  * Update the subscription to refresh both the publication and the publication
  * objects associated with the subscription.
  *
- * If 'copy_data' parameter is true, the function will set the state
- * to "init"; otherwise, it will set the state to "ready".
+ * Parameters:
  *
- * When 'validate_publications' is provided with a publication list, the
+ * If 'copy_data' is true, the function will set the state to INIT; otherwise,
+ * it will set the state to READY.
+ *
+ * If 'validate_publications' is provided with a publication list, the
  * function checks that the specified publications exist on the publisher.
  *
  * If 'refresh_tables' is true, update the subscription by adding or removing
@@ -882,9 +884,22 @@ CreateSubscription(ParseState *pstate, 
CreateSubscriptionStmt *stmt,
  * sequences that have been added or removed since the last subscription
  * creation or publication refresh.
  *
- * If 'resync_all_sequences' is true, mark all objects with "init" state
- * for re-synchronization; otherwise, only update the newly added tables and
- * sequences based on the copy_data parameter.
+ * Note, this is a common function for handling different REFRESH commands
+ * according to the parameter 'resync_all_sequences'
+ *
+ * 1. ALTER SUBSCRIPTION ... REFRESH PUBLICATION SEQUENCES
+ *    (when parameter resync_all_sequences is true)
+ *
+ *    The function will mark all sequences with INIT state.
+ *    Assert copy_data is true.
+ *    Assert refresh_tables is false.
+ *    Assert refresh_sequences is true.
+ *
+ * 2. ALTER SUBSCRIPTION ... REFRESH PUBLICATION [WITH (copy_data=true|false)]
+ *    (when parameter resync_all_sequences is false)
+ *
+ *    The function will update only the newly added tables and/or sequences
+ *    based on the copy_data parameter.
  */
 static void
 AlterSubscription_refresh(Subscription *sub, bool copy_data,
@@ -910,11 +925,10 @@ AlterSubscription_refresh(Subscription *sub, bool 
copy_data,
        WalReceiverConn *wrconn;
        bool            must_use_password;
 
-       /* resync_all_sequences cannot be specified with refresh_tables */
-       Assert(!(resync_all_sequences && refresh_tables));
-
-       /* resync_all_sequences cannot be specified with copy_data as false */
-       Assert(!(resync_all_sequences && !copy_data));
+#ifdef USE_ASSERT_CHECKING
+       if (resync_all_sequences)
+               Assert(copy_data && !refresh_tables && refresh_sequences);
+#endif
 
        /* Load the library providing us libpq calls. */
        load_file("libpqwalreceiver", false);

Reply via email to