RE: Skipping logical replication transactions on subscriber side

On Thursday, October 21, 2021 12:59 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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