RE: Skipping logical replication transactions on subscriber side

On Thursday, October 21, 2021 12:59 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> 
> I've attached updated patches. In this version, in addition to the
> review comments I go so far, I've changed the view name from
> pg_stat_subscription_errors to pg_stat_subscription_workers as per the
> discussion on including xact info to the view on another thread[1].
> I’ve also changed related codes accordingly.
> 

Thanks for your patch.
I have some minor comments on your 0001 and 0002 patch.

1. For 0001 patch, src/backend/catalog/system_views.sql
+CREATE VIEW pg_stat_subscription_workers AS
+    SELECT
+    e.subid,
+    s.subname,
+    e.subrelid,
+    e.relid,
+    e.command,
+    e.xid,
+    e.count,
+    e.error_message,
+    e.last_error_time,
+    e.stats_reset
+    FROM (SELECT
+              oid as subid,
...

Some places use TABs, I think it's better to use spaces here, to be consistent
with other places in this file.

2. For 0002 patch, I think we can add some changes to tab-complete.c, maybe
something like this:

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index ecae9df8ed..96665f6115 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1654,7 +1654,7 @@ psql_completion(const char *text, int start, int end)
        /* ALTER SUBSCRIPTION <name> */
        else if (Matches("ALTER", "SUBSCRIPTION", MatchAny))
                COMPLETE_WITH("CONNECTION", "ENABLE", "DISABLE", "OWNER TO",
-                                         "RENAME TO", "REFRESH PUBLICATION", "SET",
+                                         "RENAME TO", "REFRESH PUBLICATION", "SET", "RESET",
                                          "ADD PUBLICATION", "DROP PUBLICATION");
        /* ALTER SUBSCRIPTION <name> REFRESH PUBLICATION */
        else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) &&
@@ -1670,6 +1670,12 @@ psql_completion(const char *text, int start, int end)
        /* ALTER SUBSCRIPTION <name> SET ( */
        else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && TailMatches("SET", "("))
                COMPLETE_WITH("binary", "slot_name", "streaming", "synchronous_commit");
+       /* ALTER SUBSCRIPTION <name> RESET */
+       else if (Matches("ALTER", "SUBSCRIPTION", MatchAny, "RESET"))
+               COMPLETE_WITH("(");
+       /* ALTER SUBSCRIPTION <name> RESET ( */
+       else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && TailMatches("RESET", "("))
+               COMPLETE_WITH("binary", "streaming", "synchronous_commit");
        /* ALTER SUBSCRIPTION <name> SET PUBLICATION */
        else if (HeadMatches("ALTER", "SUBSCRIPTION", MatchAny) && TailMatches("SET", "PUBLICATION"))
        {

Regards
Tang