On Thu, Apr 11, 2024 at 6:04 PM Alexander Korotkov <aekorot...@gmail.com> wrote: > > On Fri, Apr 12, 2024 at 12:04 AM Andres Freund <and...@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.
I had operated under the assumption that we needed to push the streaming read code into heap AM because that is what we did for sequential scan, but now that I think about it, I don't see why we would have to. Bilal's patch pre-27bc1772fc did not do this. But I think the code in acquire_sample_rows() isn't more tied to heap AM after 041b96802ef than it was before it. Are you of the opinion that the code with 041b96802ef ties acquire_sample_rows() more closely to heap format? - Melanie
