Hi, On Thu, Apr 9, 2026 at 9:35 PM Jim Jones <[email protected]> wrote: > > 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.
Hm. I see that read_stream_begin_relation immediately calls read_stream_begin_impl, where we have a "rel != NULL" check (read_stream.c:787). Anyway, I think that we shouldn't rely on the fact that a given Relation will always be valid. Please, correct me if I am wrong. I see that you don't really like the idea of moving this check. But since a vectored variant of ReadBuffer() may be used by anyone, don't we need to take it into account? > Also, wouldn't it potentially make this check multiple times in a table > scan? Yep, it will. It is exactly the same logic as for ReadBuffer_common, PrefetchBuffer and ReadBufferExtended (i.e. checking this constraint before each buffer read). I don't see anything wrong with this approach. More precisely, it would be good to avoid multiple checks, but I don't see a way to do that. -- Best regards, Daniil Davydov
