[PG19] Sequence sync stops when the subscription owner can't SET ROLE to a sequence owner

Hi,

On Sat, Oct 17, 2026 at 1:19 AM Zsolt Parragi <zsolt.parragi@percona.com> wrote:

Hello

With run_as_owner = false, copy_sequence switches to the sequence
owner with SwitchToUntrustedUser before the UPDATE privilege check. If
the subscription owner can't SET ROLE to that owner, it raises an
ERROR and the whole batch is aborted.

...

The comment in report_sequence_errors also seems to assume that with
run_as_owner = false the switch always succeeds, and only the owner's
UPDATE privilege can be missing.

The attached patch checks member_can_set_role before switching and
returns INSUFFICIENT_PERM if it fails. With it, the example above
gives the WARNING for a2, and a1, a3, a4 are READY.

Thanks for the report and the patch. I personally agree it would be better to
improve this as suggested, though I'll leave it to the original author
to confirm.

I have one question on the patch:

+        * before checking privileges, and the problem can be either that the
+        * subscription owner cannot SET ROLE to the sequence owner or that
+        * the sequence owner lacks UPDATE, so no useful GRANT hint can be
+        * provided.

It's not clear to me why a useful GRANT hint can't be provided here. Without
the fix, the error message at least tells the user that the SET ROLE privilege
is missing, and names both the role and the member, whereas after the fix, the
information is no longer visible in the log. So I'm thinking maybe we could
emit a dedicated HINT for this case. Please see the attached top-up patch
for reference, in case others have the same feeling.

Best Regards,
Zhijie Hou

On Sat, Oct 17, 2026 at 4:19 AM Zsolt Parragi <zsolt.parragi@percona.com> wrote:

Hello

The patch also extends 036_sequences.pl, the new check fails without the fix.

Hi Zsolt,

The fix looks right to me, but the new test in 036_sequences.pl fails
intermittently (about once in 15 runs here). When it fails, it times
out waiting for the "insufficient privileges on subscriber sequence"
WARNING, because the sequencesync worker ends up syncing regress_s4
without any error.

The CONNECTION and password_required changes in the test setup cause
the apply worker to restart, and it can come back up before the OWNER
TO change commits. In that case it still has the old superuser owner,
so the sequencesync worker runs as superuser, member_can_set_role()
succeeds, and no WARNING is emitted.

The test needs to make sure the workers have restarted with the new
owner before running REFRESH SEQUENCES.

Regards,
Ajin Cherian
Fujitsu Australia

On Fri, Oct 16, 2026 at 10:48 PM Zsolt Parragi
<zsolt.parragi@percona.com> wrote:

With run_as_owner = false, copy_sequence switches to the sequence
owner with SwitchToUntrustedUser before the UPDATE privilege check. If
the subscription owner can't SET ROLE to that owner, it raises an
ERROR and the whole batch is aborted.

Setup on the subscriber, with 4 sequences in a FOR ALL SEQUENCES publication:

CREATE ROLE subown LOGIN; CREATE ROLE other;
GRANT pg_create_subscription TO subown;
GRANT CREATE ON DATABASE postgres TO subown;
-- a1, a3, a4 owned by subown, a2 owned by other
CREATE SUBSCRIPTION ss ... WITH (password_required = false,
enabled = false);
ALTER SUBSCRIPTION ss OWNER TO subown;
ALTER SUBSCRIPTION ss ENABLE;

All 4 sequences stay in INIT, and the log only says:

ERROR: role "subown" cannot SET ROLE to "other"

which doesn't say which sequence is the problem.

I think we can improve this one by using error context, if we want. We
have discussed this exact case while fixing other AI reported cases,
see [1]/messages/by-id/CALDaNm30Cr9n9YLHLnVDMbmUT=segVQF2p0J3NMhUjrE1Mm9cw@mail.gmail.com (description of finding-3). I think there could be other
ERRORs as well and we may not be able to add a check for everything
upfront, so, I suggest a general fix for this and other errors where
sequence name is displayed with ERROR for now. I am not sure adding
more and more defensive checks are really worth it because such cases
could be rare in practice and we are giving required information to
users so that it can fix the configuration and let sequencesync
proceed.

[1]: /messages/by-id/CALDaNm30Cr9n9YLHLnVDMbmUT=segVQF2p0J3NMhUjrE1Mm9cw@mail.gmail.com

--
With Regards,
Amit Kapila.

Hi,

On Tue, Oct 20, 2026 at 12:33 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Fri, Oct 16, 2026 at 10:48 PM Zsolt Parragi
<zsolt.parragi@percona.com> wrote:

With run_as_owner = false, copy_sequence switches to the sequence
owner with SwitchToUntrustedUser before the UPDATE privilege check. If
the subscription owner can't SET ROLE to that owner, it raises an
ERROR and the whole batch is aborted.

Setup on the subscriber, with 4 sequences in a FOR ALL SEQUENCES publication:

CREATE ROLE subown LOGIN; CREATE ROLE other;
GRANT pg_create_subscription TO subown;
GRANT CREATE ON DATABASE postgres TO subown;
-- a1, a3, a4 owned by subown, a2 owned by other
CREATE SUBSCRIPTION ss ... WITH (password_required = false,
enabled = false);
ALTER SUBSCRIPTION ss OWNER TO subown;
ALTER SUBSCRIPTION ss ENABLE;

All 4 sequences stay in INIT, and the log only says:

ERROR: role "subown" cannot SET ROLE to "other"

which doesn't say which sequence is the problem.

I think we can improve this one by using error context, if we want. We
have discussed this exact case while fixing other AI reported cases,
see [1] (description of finding-3). I think there could be other
ERRORs as well and we may not be able to add a check for everything
upfront, so, I suggest a general fix for this and other errors where
sequence name is displayed with ERROR for now. I am not sure adding
more and more defensive checks are really worth it because such cases
could be rare in practice and we are giving required information to
users so that it can fix the configuration and let sequencesync
proceed.

I agree that we cannot catch all possible errors. After thinking about this
more, there are other errors—such as deadlock timeout or lock timeout—that
cannot easily be pre-checked, so adding an error context looks like a good
approach to me. Attached is a patch that adds the error context as
suggested.

Best Regards,
Zhijie Hou

Hi Hou,

I agree that we cannot catch all possible errors. After thinking about this
more, there are other errors—such as deadlock timeout or lock timeout—that
cannot easily be pre-checked, so adding an error context looks like a good
approach to me. Attached is a patch that adds the error context as
suggested.

Thanks for the patch. I initially felt that setting/reseting the error context
can be in the copy_sequence(), but yours seem to slightly better.
get_and_validate_seq_info() may report another error.

One comment for the test: below lines may not be needed, because REFRESH SEQUENCES
does not require apply workers, and the launcher eventually starts workers with
the appropriate owner. Doesn't upcoming wait_for_log() enough?

```
# Wait until the apply worker has started under the new subscription owner.
$node_subscriber->poll_query_until('postgres',
"SELECT count(1) = 1 FROM pg_stat_subscription s JOIN pg_stat_activity a USING (pid) WHERE s.subname = 'regress_seq_sub' AND s.worker_type = 'apply' AND a.usename = 'regress_seq_sub_owner'"
) or die "Timed out while waiting for apply worker to start as new owner";
```

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Hi,

On Tue, Oct 20, 2026 at 2:21 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:

I agree that we cannot catch all possible errors. After thinking about this
more, there are other errors—such as deadlock timeout or lock timeout—that
cannot easily be pre-checked, so adding an error context looks like a good
approach to me. Attached is a patch that adds the error context as
suggested.

Thanks for the patch. I initially felt that setting/reseting the error context
can be in the copy_sequence(), but yours seem to slightly better.
get_and_validate_seq_info() may report another error.

One comment for the test: below lines may not be needed, because REFRESH SEQUENCES
does not require apply workers, and the launcher eventually starts workers with
the appropriate owner. Doesn't upcoming wait_for_log() enough?

Agreed. Here is the new version that removes this.

Best Regards,
Zhijie Hou

On Tue, 20 Oct 2026 at 19:29, Zhijie Hou <houzhijie22@gmail.com> 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

On Wed, Oct 21, 2026 at 12:59 AM Zhijie Hou <houzhijie22@gmail.com> wrote:

Hi,

Agreed. Here is the new version that removes this.

I see that the test case has been updated and it passed in 20/20 runs
for me. LGTM

regards,
Ajin Cherian
Fujitsu Australia

Hi,

On Wed, Oct 21, 2026 at 1:19 PM vignesh C <vignesh21@gmail.com> wrote:

On Tue, 20 Oct 2026 at 19:29, Zhijie Hou <houzhijie22@gmail.com> 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.

Thanks for the comments.

This cleanup is to prevent the apply worker from repeatedly starting the
sequencesync worker for the missing sequence, which would pollute the logs
and delay the next worker launch. I moved it as suggested and added a brief
comment.

2) Similarly the test passes without "GRANT CREATE ON DATABASE
postgres TO regress_seq_sub_owner;", I felt this is not required:

Agreed, this was a copy-paste oversight. Removed.

Attached is the v4 patch.

Best Regards,
Zhijie Hou

Sorry for not being active in the thread, I am at pgconf.eu this week

+# cannot SET ROLE to. With run_as_owner = false (the default), synchronizing
+# regress_s4 will fail when switching to its owner.
...
+	ALTER SEQUENCE regress_s1 OWNER TO regress_seq_sub_owner;
+	ALTER SEQUENCE regress_s3 OWNER TO regress_seq_sub_owner;
+	ALTER SEQUENCE "regress'quote" OWNER TO regress_seq_sub_owner;
+	ALTER SEQUENCE regress_s4 OWNER TO regress_seq_other;
+	ALTER SUBSCRIPTION regress_seq_sub OWNER TO regress_seq_sub_owner;
+	ALTER SUBSCRIPTION regress_seq_sub ENABLE;

I see that the current consensus seems to be that we do not fix the stall, only improve the error reporting.

Same thing in the commit message:

the error message does not
identify which sequence or subscription triggered the failure.

We should at least mention that the stall is also there, and that it is intentionally not fixed.

Even in this case, I think it would be worth to keep the test similar to the original and:
* give second or third sequence to someobody else, not the last
* Assert that not even the first one gets ready if we have one stall, with a comment explaining the decision

Otherwise this will likely generate more bugreports later, as the current comment understates the issue.

On Wed, Oct 21, 2026 at 11:59 AM Zsolt Parragi
<zsolt.parragi@percona.com> wrote:

Sorry for not being active in the thread, I am at pgconf.eu this week

+# cannot SET ROLE to. With run_as_owner = false (the default), synchronizing
+# regress_s4 will fail when switching to its owner.
...
+       ALTER SEQUENCE regress_s1 OWNER TO regress_seq_sub_owner;
+       ALTER SEQUENCE regress_s3 OWNER TO regress_seq_sub_owner;
+       ALTER SEQUENCE "regress'quote" OWNER TO regress_seq_sub_owner;
+       ALTER SEQUENCE regress_s4 OWNER TO regress_seq_other;
+       ALTER SUBSCRIPTION regress_seq_sub OWNER TO regress_seq_sub_owner;
+       ALTER SUBSCRIPTION regress_seq_sub ENABLE;

I see that the current consensus seems to be that we do not fix the
stall, only improve the error reporting.

Same thing in the commit message:

the error message does not
identify which sequence or subscription triggered the failure.

We should at least mention that the stall is also there, and that it
is intentionally not fixed.

This is the current behaviour of logical replication with or without
sequencesync. Any ERROR will lead to apply worker restarting till it
is manually resolved by the user. So, not sure a specific mention is
required but will try to adjust the commit message to reflect the same
before commit.

Even in this case, I think it would be worth to keep the test similar
to the original and:
* give second or third sequence to someobody else, not the last
* Assert that not even the first one gets ready if we have one stall,
with a comment explaining the decision

I am not completely sure whether to have this test at the first place
but anyway it is just testing whether the new CONTEXT is added with
the ERROR generated during sequencesync, it is not specific to a
particular ERROR. So, I can't see how these additions in the test are
helpful.

--
With Regards,
Amit Kapila.

On Wed, 21 Oct 2026 at 11:50, Zhijie Hou <houzhijie22@gmail.com> wrote:

Attached is the v4 patch.

Thanks for the updated patch, the patch looks good to me. I have no
more comments.

Regards,
Vignesh

On Wed, Oct 21, 2026 at 1:56 PM vignesh C <vignesh21@gmail.com> wrote:

On Wed, 21 Oct 2026 at 11:50, Zhijie Hou <houzhijie22@gmail.com> wrote:

Attached is the v4 patch.

Thanks for the updated patch, the patch looks good to me. I have no
more comments.

Thanks for the patch and review, pushed now.

--
With Regards,
Amit Kapila.