Re: [PATCH] Use streaming read I/O in sample scans

Hi,

On Fri, 28 Aug 2026 at 13:05, Yuhang Qiu <iamqyh(at)gmail(dot)com> wrote:

>
> The rescan issue is now completely separate from the SampleScan changes.
> The independent bug fix is 0002 rather than 0001 because it depends on the
> `read_stream_set_strategy()` API added by 0001. Patch 0001 only adds the
> API, 0002 fixes the heap rescan issue, and 0003 and 0004 contain the
> SampleScan and EXPLAIN changes respectively.

Thanks, 0001 and 0002 LGTM.

> > I believe we need to change the documentation at least, since it
> > mentions: 'maximum number of milliseconds to spend reading the table'.
>
> Agreed. I changed the `SYSTEM_TIME` documentation to say:
>
> > specifies approximately how many milliseconds to spend reading the table.
> > This gives you approximate control over how long the query takes

I don't think changing "maximum" to "approximately" fully explains the
behavioral change. Providing users with the exact reason would work
better, perhaps something along these lines:

```
The time limit is checked while selecting blocks. As blocks can be
selected and read ahead, any blocks already queued when the limit
expires are still processed. The query can therefore exceed the
requested duration by an amount that depends on the look-ahead
distance and how long those blocks take to read and process.
```

I am also not yet convinced that changing the documentation is
sufficient for `SYSTEM_TIME`. It would be useful to get more opinions
on whether that change is acceptable or whether `SYSTEM_TIME` should
avoid to use read streams.

Other than this point, 0003 looks good to me.

> > We must use AIO and DIO together; otherwise, DIO without AIO will
> > perform badly. And to use AIO for reads, read-streams are the general
> > solution instead of using AIO's own functions. Could you please
> > explain your idea in more detail?
>
> I agree that DIO needs to be used with AIO, and that ReadStream should be
> the general interface for AIO reads. Paths with an explicit block sequence
> should still be converted to ReadStream first.
>
> The heuristic read-ahead I have in mind would also use AIO. It is not an
> alternative to AIO. The motivation is that once DIO bypasses the page
> cache, we no longer have an equivalent mechanism for some cases that were
> previously covered by Linux readahead.
>
> For example, many in-core paths and external extensions have not yet been
> adapted to ReadStream. Some access patterns only emerge from temporal and
> spatial locality across individual reads, sometimes even across multiple SQL
> statements, rather than from an explicit block sequence known in advance.
> The idea would be to detect such locality heuristically from observed
> relation/block accesses and use AIO to read likely subsequent blocks ahead.

It is clearer now, thanks. I can see this being useful as a fallback
for unmodified code with simple and stable access patterns. For paths
where the upcoming block sequence is known, using a read stream makes
sense.

For access patterns that are difficult to predict, mispredictions
could waste I/O and CPU and cause cache pollution. I would therefore
be cautious about introducing this as a general mechanism.

I think creating a separate thread makes sense so that the relevant
details can be discussed. As a small suggestion, I would start by
sharing the idea first rather than beginning implementation; to see
what others think first.

--
Regards,
Nazir Bilal Yavuz
Microsoft