Hi Daniil
On 09/04/2026 13:46, Daniil Davydov wrote:
> 1) Right now, read stream seems like an appropriate place for this restriction.
> But actually the StartReadBuffers is not "binded" to the read stream logic. I
> mean that if someone calls it bypassing the "read_stream_begin_relation"
> function (it is OK to do so), then our restriction will be violated again.
> I think that it will be more reliable to add the restriction directly to the
> StartReadBuffers. Also, we can add an assertion ("relation is not other temp
> table") to the PinBufferForBlock. What do you think?> 2) If we decide to leave restriction in the
"read_stream_begin_relation"
> function, I would suggest adding a "rel != NULL" check here (read_stream.c):
> + if (RELATION_IS_OTHER_TEMP(rel))
> + ereport(ERROR,
> + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> + errmsg("cannot access temporary relations of other
> sessions")));
> 3) The "rel != NULL" checks may use the "RelationIsValid" macro, which seems
> more pretty to me.
Mm, not so sure...
AFAICT moving the check to StartReadBuffersImpl would require an extra
NULL guard that isn't needed in read_stream_begin_relation, as the
callers already pass valid Relations. So, rel != NULL is not needed.
Also, wouldn't it potentially make this check multiple times in a table
scan?
Am I missing something?
Best, Jim
