Hi Daniil,
Good catch!
On 03.06.26 15:23, Daniil Davydov wrote:
> Recently, we have added two commits ([1], [2]) that are fixing a bug with
> accessing temporary tables of other sessions. I found out that this bug didn't
> go away completely :
>
> == session 1 ==
>
> postgres=# CREATE TABLE empty_table (id INT);
> CREATE TABLE
>
> == session 2 ==
>
> postgres=# INSERT INTO pg_temp_0.empty_table VALUES (1);
> INSERT 0 1
Session 1 here does not create a temporary table (most likely a copy &
paste error), but I could reproduce this error as you suggested:
== session 1 ==
postgres=# CREATE TEMPORARY TABLE t (id int);
== session 2 ==
postgres=# \d pg_temp*.*
Table "pg_temp_2.t"
Column | Type | Collation | Nullable | Default
--------+---------+-----------+----------+---------
id | integer | | |
postgres=# INSERT INTO pg_temp_2.t VALUES (42);
INSERT 0 1
postgres=# INSERT INTO pg_temp_2.t VALUES (42);
ERROR: cannot access temporary tables of other sessions
It only fails after the second INSERT attempt.
With your patch applied it returns an error also in the first query:
psql (19devel)
Type "help" for help.
postgres=# INSERT INTO pg_temp_0.t VALUES (42);
ERROR: cannot access temporary tables of other sessions
>
> As you can see, the INSERT command completes successfully. This happens
> because
> empty_table has no pages, so the insert path looks like this :
> heap_insert -> RelationGetBufferForTuple -> RelationAddBlocks -> ....
>
> The RelationAddBlocks extends relations's smgr and allocates a new temp buffer
> without calling ReadBufferExtended. Thus, we are 1) bypassing the
> "RELATION_IS_OTHER_TEMP" check and 2) creating a buffer in our own temp
> buffers
> pool. The (2) will lead to an error [3] if we attempt to flush such a buffer.
>
> I suggest fixing it by checking whether the relation is other-temp-rel inside
> the ExtendBufferedRelLocal function. As far as I can see, all
> temp-relation-extend paths include this function.
>
> Please, see the attached patch that fixes a problem and adds a new test.
At a first glance the check seems reasonable. One tiny wording nit: the
comment in ExtendBufferedRelLocal says "... covering any attempt to
extend local relation.", but to avoid any confusing with the meaning of
RELATION_IS_LOCAL I'd argue that "covering any attempt to extend a
temporary relation" would be slightly clearer.
Thanks for the fix!
Best, Jim
