On Tue, Mar 26, 2024 at 7:16 AM David Rowley <dgrowle...@gmail.com> wrote: > > On Thu, 21 Mar 2024 at 14:10, Masahiko Sawada <msaw...@postgresql.org> wrote: > > > > Add TIDStore, to store sets of TIDs (ItemPointerData) efficiently. > > > src/backend/access/common/tidstore.c | 463 > > +++++++++++++++++++++ > > I was looking at this code and I saw the following: > > /* choose the maxBlockSize to be no larger than 1/16 of max_bytes */ > while (16 * maxBlockSize > max_bytes * 1024L) > maxBlockSize >>= 1; > > I saw the "* 1024L" and was confused by it. "max_bytes * 1024L" > converts the number of bytes into "millibytes" ([1]), which I don't > think is correct. > > Either "max_bytes" is a bad name for this variable or the * 1024L > should be removed. >
Right. We discussed it on the original thread too[1]. Since we're going to change the create API of TidStore, I'm going to fix it along with the API changes[2]. Even if we don't make the API change, I'll fix it individually. Regards, [1] https://www.postgresql.org/message-id/CANWCAZZTE-14ofsucofTuhFsfuDGBNf%3DNZb22TMYT8bxA41oQQ%40mail.gmail.com [2] https://www.postgresql.org/message-id/CAD21AoAxXGf7FbuA9jvx%2BXUFrEJUz9JNOmkxF8SLppOOOvhBtg%40mail.gmail.com Please see v79-0003-Rethink-create-and-attach-APIs-of-shared-TidStor.patch. -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com