| From: | vignesh C <vignesh21(at)gmail(dot)com> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Tristan Partin <tristan(at)partin(dot)io>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Fix publisher-side sequence permission reporting |
| Date: | 2026-07-13 12:54:55 |
| Message-ID: | CALDaNm3UWG57b6h2pHqMwcKL2NtR0hBi1sY08QMDkaCO8AnU2w@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, 13 Jul 2026 at 18:18, vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Wed, 8 Jul 2026 at 14:50, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> >
> > On Thu, Jul 2, 2026 at 6:55 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > > How about something like this instead?
> > > >
> > > > This function returns a row of NULL values if the specified relation
> > > > OID does not exist, if it is not a sequence, if the current user lacks
> > > > <literal>SELECT</literal> privilege on the sequence, if the sequence
> > > > is another session's temporary sequence, or if it is an unlogged
> > > > sequence on a standby server.
> > > >
> > >
> > > Sounds reasonable. But after this we don't need the next para to say:
> > > "It requires <literal>SELECT</literal> privilege on the sequence.".
> > > See attached.
> >
> > Thanks for the patch! I've pushed it.
> >
> > > > I've updated the patch furthermore. Attached.
> > > >
> > >
> > > LGTM.
> >
> > Thanks for the review! I've pushed this as well.
>
> There was an issue reported by Noah at [1], the finding-7 reported
> fails with an assert. The assertion can be triggered if a sequence is
> dropped concurrently while the sequence synchronization worker is
> processing it:
> TRAP: failed Assert("!isnull"), File: "sequencesync.c", Line: 298, PID: 25085
> 0 postgres ExceptionalCondition
> 1 postgres get_and_validate_seq_info
> 2 postgres copy_sequences
> 3 postgres LogicalRepSyncSequences
> 4 postgres start_sequence_sync
> 5 postgres SequenceSyncWorkerMain
>
> It can be reproduced with the following steps:
> Session 1:
> BEGIN;
> DROP SEQUENCE s1;
>
> Session 2:
> ALTER SUBSCRIPTION ... REFRESH SEQUENCES;
>
> Session 1:
> COMMIT;
>
> The assertion occurs because the sequence is dropped after the
> sequence synchronization worker has collected the sequence metadata
> but before it retrieves the sequence data. In this case,
> 'has_sequence_privilege_id()' calls 'get_rel_relkind()', which can no
> longer find the relation in the syscache and therefore returns 'NULL'.
> 'get_and_validate_seq_info()' currently assumes that the privilege
> column is never 'NULL' and triggers the assertion.
>
> I've attached a patch to handle this case by treating a 'NULL'
> privilege value as indicating that the sequence no longer exists and
> reporting it through the existing "missing sequence on publisher"
> path.
> Thoughts?
Sorry, I missed sharing the link in the previous mail. The link for the same is:
[1] - https://www.postgresql.org/message-id/20260710045217.f0.noahmisch%40microsoft.com
Regards,
Vignesh
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Ilia Evdokimov | 2026-07-13 13:20:28 | Re: Wrong argument name in error message from attribute_stats functions |
| Previous Message | vignesh C | 2026-07-13 12:52:42 | Re: sequencesync worker race with REFRESH SEQUENCES |