On Tue, 27 Jun 2023 at 06:57, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> At high level 0001 looks fine to me, just some suggestions
Thanks for the review.
> 1.
> +Notes about dynamic prefix truncation
> +-------------------------------------
>
> I feel instead of calling it "dynamic prefix truncation" should we can
> call it "dynamic prefix skipping", I mean we are not
> really truncating anything right, we are just skipping those
> attributes in comparison?
The reason I am using "prefix truncation" is that that is a fairly
well-known term in literature (together with "prefix compression"),
and it was introduced on this list with that name by Peter in 2018
[0]. I've seen no good reason to change terminology; especially
considering that normal "static" prefix truncation/compression is also
somewhere on my to-do list.
> 2.
> I think we should add some comments in the _bt_binsrch() function
> where we are having main logic around maintaining highcmpcol and
> lowcmpcol.
> I think the notes section explains that very clearly but adding some
> comments here would be good and then reference to that section in the
> README.
Updated in the attached version 12 of the patchset (which is also
rebased on HEAD @ 9c13b681). No changes apart from rebase fixes and
these added comments.
Kind regards,
Matthias van de Meent
Neon (https://neon.tech)
[0] https://www.postgresql.org/message-id/CAH2-Wzn_NAyK4pR0HRWO0StwHmxjP5qyu+X8vppt030XpqrO6w@mail.gmail.com