On Tue, 20 Oct 2026 at 19:29, Zhijie Hou <[email protected]> wrote:
>
> Agreed. Here is the new version that removes this.

Thanks for the updated patch, the changes look good to me.
Couple of minor suggestions:
1) The test passes without "ALTER SUBSCRIPTION regress_seq_sub REFRESH
PUBLICATION WITH (copy_data = false);"
+# Remove the dropped sequence (regress_s2) from the subscription, then disable
+# the subscription and wait for its workers to stop before changing its
+# connection and owner.
+$node_subscriber->safe_psql(
+       'postgres', qq(
+       ALTER SUBSCRIPTION regress_seq_sub REFRESH PUBLICATION WITH
(copy_data = false);

Not sure if this is required, even if required can we move this change
along with the previous test as a test completion or cleanup.

2) Similarly the test passes without "GRANT CREATE ON DATABASE
postgres TO regress_seq_sub_owner;", I felt this is not required:
+# Change the subscription owner to regress_seq_sub_owner, and make regress_s4
+# owned by a separate role (regress_seq_other) that regress_seq_sub_owner
+# cannot SET ROLE to. With run_as_owner = false (the default), synchronizing
+# regress_s4 will fail when switching to its owner.
+$node_subscriber->safe_psql(
+       'postgres', qq(
+       ALTER SUBSCRIPTION regress_seq_sub CONNECTION '$publisher_connstr';
+       ALTER SUBSCRIPTION regress_seq_sub SET (password_required = false);
+       CREATE ROLE regress_seq_sub_owner LOGIN;
+       CREATE ROLE regress_seq_other;
+       GRANT CREATE ON DATABASE postgres TO regress_seq_sub_owner;

Regards,
Vignesh


Reply via email to