public inbox for [email protected]  

help / color / mirror / Atom feed

From: Jim Jones <[email protected]>
To: Alexander Korotkov <[email protected]>
To: Daniil Davydov <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Soumya S Murali <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Stepan Neretin <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Mohamed Ali <[email protected]>
Cc: Nazneen Jafri <[email protected]>
Cc: Shawn McCoy <[email protected]>
Subject: Re: Fix bug with accessing to temporary tables of other sessions
Date: Sat, 2 May 2026 19:32:30 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAPpHfds8pwuwF69JFs0SQz58op-U4ddR3KWv0NVEij2mpkzgpw@mail.gmail.com>
References: <[email protected]>
	<CAJDiXghBO_TqvHOSui8MOxiFmwLT20+SAnH5nW1rpWHk7Jwffg@mail.gmail.com>
	<[email protected]>
	<CAJDiXgi0JFk0f2KWWQkzLBC5P7erX9WP18qqnbi-rjZ-K-P=3w@mail.gmail.com>
	<CAMtXxw_hBNuAWQUdSRMpoeRVRYr+5+S7p0bSzuqtHxfpzJPd3w@mail.gmail.com>
	<[email protected]>
	<CAMtXxw_2VnP5U7U+2ObOCC2voEkAzUqAgH=rPTQBXrb_8K=pyA@mail.gmail.com>
	<CAPpHfdtsGSywUExfKMakAuRz8-+61d-4XHkVf=pNQeYTZTVYoA@mail.gmail.com>
	<CAMtXxw8J7L0r8aXt9NBFfqqSnaNyU=-_Uq4tW4aO3PHyVeXLAQ@mail.gmail.com>
	<CAPpHfdvLCgXgdhSvuPCkek+r=P4E-WiNceWfh7UQztiMzfFcvg@mail.gmail.com>
	<[email protected]>
	<CAPpHfdsOkkeobCfO9hJvaE2fRcdGmWJoX0umx--DnApmtmefCg@mail.gmail.com>
	<CAJDiXgjNySH7Od3nZGjWy30sKv+-12c=kajL7Qfkj3y+CDRwtA@mail.gmail.com>
	<CAPpHfds8pwuwF69JFs0SQz58op-U4ddR3KWv0NVEij2mpkzgpw@mail.gmail.com>

Hi

On 02/05/2026 18:34, Alexander Korotkov wrote:
> On Sat, May 2, 2026 at 6:37 PM Daniil Davydov <[email protected]> wrote:
>> On Sat, May 2, 2026 at 9:16 PM Alexander Korotkov <[email protected]> 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.

+1

>>> 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.


Thanks for the comprehensive additional tests!

In addition to the DROP TABLE exception:

It is also possible to LOCK temporary tables from other sessions:

postgres=# BEGIN;
BEGIN
postgres=*# LOCK TABLE pg_temp_91.t IN ACCESS SHARE MODE ;
LOCK TABLE

pg_temp_91.t lives as long the transaction is open -- even after the
origin session closes, which is totally expected. I'd say it falls into
the same category of DROP TABLE, where the table contents are never
read, so I'd argue it's ok.

> 
>> 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.

+1 for a separate patch. I think the scope of the current patch is good
as-is.

Thanks!

Best, Jim

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Fix bug with accessing to temporary tables of other sessions
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox