Thread: Forbid to DROP temp tables of other sessions

Daniil Davydov <3danissimo@gmail.com> writes:
> I noticed that TRUNCATE and ALTER commands on temporary tables of other
> sessions produce an error "cannot truncate/alter temporary tables of other
> sessions". But are there any reasons to allow us to DROP such tables?
> It seems to me that the only case when we may need it is the removal of
> orphan tables. But the autovacuum is responsible for this and it uses a
> different functionality. I'm wondering if there are any other cases. If
> not, can we just handle it for example in ExecDropStmt and produce an error
> like "cannot drop temporary tables of other sessions"?

If autovacuum can do it, I don't see a reason to prevent superusers
from doing it manually.

            regards, tom lane

> On 14 Nov 2024, at 11:55, Daniil Davydov <3danissimo@gmail.com> wrote:
>
> On Wed, Oct 30, 2024 at 7:32 PM Rafia Sabih <rafia.pghackers@gmail.com> wrote:
>
>> Good catch. I agree with this being an unwarranted behaviour.
>> A minor comment from my end is the wording of the error message.
>> Based on the Postgresql error message style huide, something like this could be better,
>> "could not access temporary relations of other sessions".
>> --
>> Regards,
>> Rafia Sabih
>> CYBERTEC PostgreSQL International GmbH
>>
> Thanks for your comment. I attach a patch with a fixed error message.
> Also you can find it in commit fest
> (https://commitfest.postgresql.org/51/5379/)

I suspect that protection of temp tables was broken by 00d1e02be249. And I'd suggest fixing it in a line with how it
workedbefore. Changes to locking mechanism is kind of a super subtle matters, it is really hard to bring this checks
herewithout breaking something else. Maybe not immidiately. but still. I'd suggest fixing somewhere around
RelationAddBlocks().But be sure to check all code pathes that lead to this check. 

Also, having an isolation test is nice. But do we actually do isolation tests with PL\pgSQL?

Thanks!


Best regards, Andrey Borodin.

Hi,

On 2024-11-21 23:52:52 +0300, Andrey M. Borodin wrote:
> I suspect that protection of temp tables was broken by 00d1e02be249.

I think we might have broken this in multiple ways in recent releases. In 17
one can even read the data from the other relation when using a sequential
scan, because that'll go through a streaming read and from there directly to
StartReadBuffers(), bypassing the check in ReadBufferExtended().


> And I'd suggest fixing it in a line with how it worked before. Changes to
> locking mechanism is kind of a super subtle matters, it is really hard to
> bring this checks here without breaking something else. Maybe not
> immidiately. but still. I'd suggest fixing somewhere around
> RelationAddBlocks(). But be sure to check all code pathes that lead to this
> check.

Yea, I don't think the lock approach would work that well.  However, I don't
love having to put RELATION_IS_OTHER_TEMP() checks everywhere either. After
all we seem to have introduced two independent oversights related to this...

I wonder if we could handle this by having a few locations explicitly opt-in
to accessing another database's temp table and erroring out everywhere else -
there's not that many places we need to do so.


> Also, having an isolation test is nice. But do we actually do isolation
> tests with PL\pgSQL?

There are several other tests creating functions. But I think this one goes a
bit too far...


Greetings,

Andres Freund

> On 22 Nov 2024, at 03:02, Andres Freund <andres@anarazel.de> wrote:
>
> I don't
> love having to put RELATION_IS_OTHER_TEMP() checks everywhere either.

+1. I do not understand why this restriction (protecting temp tables from access) is a responsibility of the buffer
manager.

Actually, I like the idea that superuser knows better what to do.
What if we say it's not a bug, but a feature. Will it break some contracts with user or some functionality?

It seems that protection of temp tables should belong to ACL stuff. And in a logic of this subsystem would be natural
tojust allow superuser do whatever they want with. 

Is it some lunatic idea? Or does it make some sense?


Best regards, Andrey Borodin.