Re: Table AM Interface Enhancements

On Fri, Apr 12, 2024 at 12:04 AM Andres Freund <andres@anarazel.de> wrote:
> On 2024-04-11 20:46:02 +0300, Alexander Korotkov wrote:
> > I hope this work is targeting pg18.
>
> I think anything of the scope discussed by Melanie would be very clearly
> targeting 18. For 17, I don't know yet whether we should revert the the
> ANALYZE streaming read user (041b96802ef), just do a bit of comment polishing,
> or some other small change.
>
> One oddity is that before 041b96802ef, the opportunities for making the
> interface cleaner were less apparent, because c6fc50cb4028 increased the
> coupling between analyze.c and the way the table storage works.

Thank you for pointing this out about c6fc50cb4028, I've missed this.

> > Otherwise, do I get this right that this post feature-freeze works on
> > designing a new API?  Yes, 27bc1772fc masked the problem.  But it was
> > committed on Mar 30.
>
> Note that there were versions of the patch that were targeting the
> pre-27bc1772fc interface.

Sure, I've checked this before writing.  It looks quite similar to the
result of applying my revert patch [1] to the head.

Let me describe my view over the current situation.

1) If we just apply my revert patch and leave c6fc50cb4028 and
041b96802ef in the tree, then we get our table AM API narrowed.  As
you expressed the current API requires block numbers to be 1:1 with
the actual physical on-disk location [2].  Not a secret I think the
current API is quite restrictive.  And we're getting the ANALYZE
interface narrower than it was since 737a292b5de.  Frankly speaking, I
don't think this is acceptable.

2) Pushing down the read stream and prefetch to heap am is related to
difficulties [3], [4].  That's quite a significant piece of work to be
done post FF.

In token of all of the above, is the in-tree state that bad? (if we
abstract the way 27bc1772fc and dd1f6b0c17 were committed).

The in-tree state provides quite a general API for analyze, supporting
even non-block storages.  There is a way to reuse existing
acquire_sample_rows() for table AMs, which have block numbers 1:1 with
the actual physical on-disk location.  It requires some cleanup for
comments and docs, but does not require us to redesing the API post
FF.

Links.
1. https://www.postgresql.org/message-id/CAPpHfdvuT6DnguzaV-M1UQ2whYGDojaNU%3D-%3DiHc0A7qo9HBEJw%40mail.gmail.com
2. https://www.postgresql.org/message-id/20240410212117.mxsldz2w6htrl36v%40awork3.anarazel.de
3. https://www.postgresql.org/message-id/CAAKRu_ZxU6hucckrT1SOJxKfyN7q-K4KU1y62GhDwLBZWG%2BROg%40mail.gmail.com
4. https://www.postgresql.org/message-id/CAAKRu_YkphAPNbBR2jcLqnxGhDEWTKhYfLFY%3D0R_oG5LHBH7Gw%40mail.gmail.com

------
Regards,
Alexander Korotkov