Re: [PATCH] Fix stale relation close in sequence synchronization

Hi,

Thanks for reviewing and confirming the issue.

On Tue, 28 Apr 2026 at 18:48, vignesh C <vignesh21(at)gmail(dot)com> wrote:

>
> Thanks for finding and reporting the issue. I was able to reproduce
> the issue with the steps you provided.
> Few comments:
> 1) Here "SELECT nextval('regress_no_select');" and "REVOKE ALL ON
> SEQUENCE regress_no_select FROM PUBLIC;" is not required for this test
> case:
> +$node_publisher->safe_psql(
> + 'postgres', qq(
> + CREATE ROLE regress_seq_repl LOGIN REPLICATION;
> + CREATE SEQUENCE regress_no_select;
> + SELECT nextval('regress_no_select');
> + GRANT USAGE ON SCHEMA public TO regress_seq_repl;
> + GRANT SELECT ON ALL SEQUENCES IN SCHEMA public TO regress_seq_repl;
> + REVOKE ALL ON SEQUENCE regress_no_select FROM PUBLIC;
> + REVOKE ALL ON SEQUENCE regress_no_select FROM regress_seq_repl;
> +));
>

Agreed. Removed both statements from the test.

> 2) Since the comment about “dropped concurrently” has been removed,
> could you merge that context into the new wording:
>

Done. The comment now mentions both insufficient privileges and
concurrent drops.

>
> 3) Can we change this:
> ##########
> # A NULL sequence data row from the publisher must not make the subscriber
> # close the previously synchronized sequence relation again.
> ##########
>
> To something like:
> ##########
> # Ensure that insufficient privileges on the publisher for a sequence
> # do not disrupt the subscriber. The subscriber should log a warning
> # and continue retrying.
> ##########
>

Done. I used that wording for the TAP test block comment.

Attached is v3 with these changes.

Regards,
Ayush