Re: [PATCH] Fix stale relation close in sequence synchronization - Mailing list pgsql-hackers

Few comments:
1) Since we are setting the variable to NULL for every sequence now,
this is not required:
@@ -246,6 +246,8 @@ get_and_validate_seq_info(TupleTableSlot *slot,
Relation *sequence_rel,
        Form_pg_sequence local_seq;
        LogicalRepSequenceInfo *seqinfo_local;

+       *sequence_rel = NULL;
+
        *seqidx = DatumGetInt32(slot_getattr(slot, ++col, &isnull));
        Assert(!isnull);

2) Creating a subscription is costly as it has more work to do as it
has to sync all relations and requires more processes to be started,
instead shall we use "ALTER SUBSCRIPTION ... SET CONNECTION" followed
by "ALTER SUBSCRIPTION ... REFRESH SEQUENCES"
+$node_subscriber->safe_psql(
+       'postgres',
+       "CREATE SUBSCRIPTION regress_seq_sub_no_select CONNECTION
'$publisher_limited_connstr' PUBLICATION regress_seq_pub WITH
(disable_on_error = true)"
+);

3) You can use sequence name as regress_s5 to be consistent with the
other sequence names nearby or alternatively you can change the privs
of an already existing sequence:
+       CREATE ROLE regress_seq_repl LOGIN REPLICATION;
+       CREATE SEQUENCE regress_no_select;
+       GRANT USAGE ON SCHEMA public TO regress_seq_repl;

4) I feel this is not required:
+$result = $node_subscriber->safe_psql('postgres', 'SELECT 1');
+is($result, '1',
+       'subscriber remains running after publisher returns NULL
sequence data');