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

Hi all,

On Wed, Apr 8, 2026 at 4:11 PM Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> wrote:
>
> Hi
>
> On 08/04/2026 11:17, Soumya S Murali wrote:
> > I worked on the issue of accessing temporary tables belonging to other
> > sessions and tried implementing the fix at the buffer manager level,
> > as suggested. I added checks in ReadBuffer_common() and
> > PrefetchBuffer() to reject access when a relation is temporary
> > (relpersistence = TEMP) but does not use local buffers
> > (!RelationUsesLocalBuffers) so that it ensures only heap page access
> > is blocked, while catalog lookups and other metadata operations
> > continue to work as before. While testing, I observed that in many
> > cases the query does not reach the buffer manager because name
> > resolution fails earlier with “relation does not exist”. However, the
> > added checks ensure that even if execution reaches the buffer layer,
> > access to other sessions’ temporary tables is safely rejected. The
> > change is minimal, and did not modify parser/ACL behavior and all
> > regression tests got passed successfully too.
> > Kindly review the attached patch herewith. Please let me know if this
> > approach aligns with expectations or if further adjustments are
> > needed.
>
> A few comments:
>
> == PrefetchBuffer ==
>
> The condition nested inside the if (RelationUsesLocalBuffers(reln))
> tests the opposite of the main if !RelationUsesLocalBuffers(reln):
>
> if (RelationUsesLocalBuffers(reln))
> {
> /* ACCESS DENIED CHECK */
> if (reln != NULL &&
> reln->rd_rel != NULL &&
> reln->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
> !RelationUsesLocalBuffers(reln))
> {
> ereport(ERROR,
> (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> errmsg("cannot access temporary tables of other sessions")));
> }
> ...
> }
>
> So it'll be always false, making the ereport unreachable.
>
> == ReadBufferExtended ==
>
> These conditions cancel each other out:
>
> if (reln->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
> !RelationUsesLocalBuffers(reln))
>
> RelationUsesLocalBuffers(reln) expands to
> ((relation)->rd_rel->relpersistence == RELPERSISTENCE_TEMP), making the
> error message unreachable. Perhaps you meant RELATION_IS_OTHER_TEMP?
>
> == ReadBuffer_common ==
>
> Same as in ReadBufferExtended and PrefetchBuffer.
>
> == tests ==
>
> You excluded the tests from the patch.
>
> == patch version ==
>
> You forgot to add the patch version.

Thank you for the detailed review and for pointing out these issues. I
understood that the condition I used with RelationUsesLocalBuffers()
was incorrect and made the checks unreachable. Also the
RELATION_IS_OTHER_TEMP() was not the appropriate macro to use here.
Regarding PrefetchBuffer, placing the check inside the
RelationUsesLocalBuffers() branch was also logically inconsistent.
Apologies for missing the tests and patch versioning in my submission.
I will ensure these are included properly in future iterations. Based
on the subsequent discussion and the analysis of the regression
introduced by routing through read_stream_next_buffer(), I agree that
the fix is better in the read stream / buffer read entry points rather
than only in bufmgr.
Thank you for your guidance. It really helped clarify both the root
cause and the correct placement of the fix.

Regards,
Soumya