On Tue, Sep 10, 2024 at 3:36 AM Michael Banck <mba...@gmx.net> wrote:
> I am a bit confused about the status of this thread. Robert mentioned
> RC1, so I guess it pertains to v17 but I don't see it on the open item
> wiki list?
Yes, v17. Alight, I'll add an item.
> Does the above mean you are going to revert it for v17, Thomas? And if
> so, what exactly? The ANALYZE changes on top of the streaming read API
> or something else about that API that is being discussed on this thread?
I might have been a little pessimistic in that assessment. Another
workaround that seems an awful lot cleaner and less invasive would be
to offer a new ReadStream API function that provides access to block
numbers and the strategy, ie the arguments of v16's
scan_analyze_next_block() function. Mats, what do you think about
this? (I haven't tried to preserve the prefetching behaviour, which
probably didn't actually too work for you in v16 anyway at a guess,
I'm just looking for the absolute simplest thing we can do to resolve
this API mismatch.) TimeScale could then continue to use its v16
coding to handle the two-relations-in-a-trenchcoat problem, and we
could continue discussing how to make v18 better.
I looked briefly at another non-heap-like table AM, the Citus Columnar
TAM. I am not familiar with that code and haven't studied it deeply
this morning, but its _next_block() currently just returns true, so I
think it will somehow need to change to counting calls and returning
false when it thinks its been called enough times (otherwise the loop
in acquire_sample_rows() won't terminate, I think?). I suppose an
easy way to do that without generating extra I/O or having to think
hard about how to preserve the loop cound from v16 would be to use
this function.
I think there are broadly three categories of TAMs with respect to
ANALYZE block sampling: those that are very heap-like (blocks of one
SMgrRelation) and can just use the stream directly, those that are not
at all heap-like (doing something completely different to sample
tuples and ignoring the block aspect but using _next_block() to
control the loop), and then Timescale's case which is sort of
somewhere in between: almost heap-like from the point of view of this
sampling code, ie working with blocks, but fudging the meaning of
block numbers, which we didn't anticipate. (I wonder if it fails to
sample fairly across the underlying relation boundary anyway because
their data densities must surely be quite different, but that's not
what we're here to talk about.)
. o O { We need that wiki page listing TAMs with links to the open
source ones... }
From db05a33e742cb292b1a2d44665582042fcd05d2f Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Tue, 10 Sep 2024 10:15:33 +1200
Subject: [PATCH] Allow raw block numbers to be read from ReadStream.
---
src/backend/storage/aio/read_stream.c | 14 ++++++++++++++
src/include/storage/read_stream.h | 3 +++
2 files changed, 17 insertions(+)
diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c
index 7f0e07d9586..cf914a7712f 100644
--- a/src/backend/storage/aio/read_stream.c
+++ b/src/backend/storage/aio/read_stream.c
@@ -835,3 +835,17 @@ read_stream_end(ReadStream *stream)
read_stream_reset(stream);
pfree(stream);
}
+
+/*
+ * Transitional support for code that would like to perform a read directly,
+ * without using the stream. Returns, and skips, the next block number that
+ * would be read by the stream's look-ahead algorithm, or InvalidBlockNumber
+ * if the end of the stream is reached. Also reports the strategy that would
+ * be used to read it.
+ */
+BlockNumber
+read_stream_next_block(ReadStream *stream, BufferAccessStrategy *strategy)
+{
+ *strategy = stream->ios[0].op.strategy;
+ return read_stream_get_block(stream, NULL);
+}
diff --git a/src/include/storage/read_stream.h b/src/include/storage/read_stream.h
index 2f94ee744b9..37b51934f8d 100644
--- a/src/include/storage/read_stream.h
+++ b/src/include/storage/read_stream.h
@@ -68,6 +68,9 @@ extern ReadStream *read_stream_begin_relation(int flags,
void *callback_private_data,
size_t per_buffer_data_size);
extern Buffer read_stream_next_buffer(ReadStream *stream, void **per_buffer_data);
+extern BlockNumber read_stream_next_block(ReadStream *stream,
+ BufferAccessStrategy *strategy);
+
extern ReadStream *read_stream_begin_smgr_relation(int flags,
BufferAccessStrategy strategy,
SMgrRelation smgr,
--
2.46.0
