Re: Fix bug with accessing to temporary tables of other sessions - Mailing list pgsql-hackers

Hi,

I did some digging:

It seems that b7b0f3f2724 is what actually broke it. By switching from
ReadBufferExtended() to read_stream_next_buffer(), it silently routed
all SELECT/UPDATE/DELETE/COPY away from the check that was sitting in
ReadBufferExtended().

@@ -528,25 +599,23 @@ heap_fetch_next_buffer(HeapScanDesc scan,
ScanDirection dir)
         */
        CHECK_FOR_INTERRUPTS();

-       if (unlikely(!scan->rs_inited))
+       /*
+        * If the scan direction is changing, reset the prefetch block
to the
+        * current block. Otherwise, we will incorrectly prefetch the blocks
+        * between the prefetch block and the current block again before
+        * prefetching blocks in the new, correct scan direction.
+        */
+       if (unlikely(scan->rs_dir != dir))
        {
-               scan->rs_cblock = heapgettup_initial_block(scan, dir);
+               scan->rs_prefetch_block = scan->rs_cblock;
+               read_stream_reset(scan->rs_read_stream);
+       }

-               /* ensure rs_cbuf is invalid when we get
InvalidBlockNumber */
-               Assert(scan->rs_cblock != InvalidBlockNumber ||
-                          !BufferIsValid(scan->rs_cbuf));
+       scan->rs_dir = dir;

-               scan->rs_inited = true;
-       }
-       else
-               scan->rs_cblock = heapgettup_advance_block(scan,
scan->rs_cblock,
-
                          dir);
-
-       /* read block if valid */
-       if (BlockNumberIsValid(scan->rs_cblock))
-               scan->rs_cbuf = ReadBufferExtended(scan->rs_base.rs_rd,
MAIN_FORKNUM,
-
          scan->rs_cblock, RBM_NORMAL,
-
          scan->rs_strategy);
+       scan->rs_cbuf = read_stream_next_buffer(scan->rs_read_stream, NULL);
+       if (BufferIsValid(scan->rs_cbuf))
+               scan->rs_cblock = BufferGetBlockNumber(scan->rs_cbuf);
 }


It simply builds upon 210622c60e1 (introduces StartReadBuffers ->
PinBufferForBlock) and b5a9b18cd0b (builds read_stream_begin_relation on
top of it).

So I thought we can explore the option of adding this check directly in
read_stream_begin_relation():


if (RELATION_IS_OTHER_TEMP(rel))
  ereport(ERROR,
         (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
      errmsg("cannot access temporary relations of other sessions")));


Thoughts?

See v15 attached.

Daniil, feel free to revert it to your last patch if you disagree with
this approach.

Best, Jim