Horton

Re: Fix bug with accessing to temporary tables of other sessions

Alexander Korotkov <aekorotkov@gmail.com>

View thread
From: Alexander Korotkov <aekorotkov@gmail.com>
To: Daniil Davydov <3danissimo@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Soumya S Murali <soumyamurali.work@gmail.com>, Jim Jones <jim.jones@uni-muenster.de>, Tom Lane <tgl@sss.pgh.pa.us>, Stepan Neretin <slpmcf@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, Mohamed Ali <moali.pg@gmail.com>, Nazneen Jafri <jafrinazneen@gmail.com>, Shawn McCoy <shawn.the.mccoy@gmail.com>
Date: 2026-05-02T16:34:00Z
Lists: pgsql-hackers

Attachments

On Sat, May 2, 2026 at 6:37 PM Daniil Davydov <3danissimo@gmail.com> wrote:
> On Sat, May 2, 2026 at 9:16 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> >
> > Thank you for your feedback.  I think the scope of this patch is well
> > described in [1].  We don't want to restrict the superuser from
> > something, but our buffer manager just technically can't access the
> > local buffers of other sessions.  Read streams introduced a new code
> > path for reading relations, which was lacking of the proper check for
> > local buffers of other sessions.  And this patch attempts to fix that.
> > DROP TABLE is an exclusion.  It actually don't need to read contents
> > of buffers, just drop them.  And DropRelationBuffers() have a special
> > exclusion for this case.  So, DROP TABLE appears to be the only
> > operation that makes sense, it's a conscious exclusion, and there is
> > no intention to forbid it.
>
> Yep, exactly.
>
> > I've revised the patch.  0001 contains tests and states the current
> > behavior.  0002 contains fix and the corresponding changes in the
> > tests.  I made a change in 0001: removed the check in
> > ReadBufferExtended().  We added the same check to ReadBuffer_common(),
> > and I don't think it makes sense to do this check twice in the row.
>
> Thank you! But I'm afraid that you forgot to attach the patches..

Here they are.

> BTW, what do you think about this proposal? :
> > On the other hand, we have an error message that says "cannot access...", which
> > may look like every kind of "access" is forbidden. I bet that this is the place
> > that has confused you. More accurate error message would be "cannot access
> > pages..." or "cannot access content...". I think we can change our error
> > message in this way. What do you think?
>
> We can easily include it in the first patch.

This is possible, but I would keep that in a separate patch.  We now
have clear scope for both patches: 0001 includes additional tests,
0002 fixes the bug and restores old behavior.

------
Regards,
Alexander Korotkov
Supabase