Re: MinIndexTupleSize seems slightly wrong

Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> writes:
> At Thu, 12 Apr 2018 17:26:33 -0700, Peter Geoghegan <pg(at)bowt(dot)ie> wrote in <CAH2-WzkQmb54Kbx-YHXstRKXcNc+_87jwV3DRb54xcybLR7Oig(at)mail(dot)gmail(dot)com>
>> However, that at least seems questionable to me. See _bt_pgaddtup()
>> for a simple example of this -- "minus infinity" items on internal
>> pages are sized sizeof(IndexTupleData).

> MaxIndexTuplesPerPage is 408. If we have 407 normal and one
> 0-attr index tuple, they leave 16 byte spare space, in which no
> normal tuple fits. In the case where we have BLCKSZ of 3 * 8192 =
> 24576 bytes, MaxIndexTuplesPerPage is 1227. 1226 normal and one
> 0-attr tuples uses 24556 bytes and it leaves spare 20 bytes, in
> which one extra normal tuple can reside. This can cause
> off-by-one error.

We don't support BLCKSZ that isn't a power of 2, and are unlikely
to start. So that particular counterexample doesn't excite me.

As long as btree only has one no-data tuple per page, I think we are good,
because this calculation does not account for page special space. We might
be underestimating how many tuples can fit by one MAXALIGN quantum, but
the special space takes up at least one MAXALIGN quantum, so it's safe.

Twouldn't be a bad idea to document this reasoning, though.

Also, my first reaction on looking at this code was "who added
MinIndexTupleSize and then didn't replace the equivalent subexpression
of MaxIndexTuplesPerPage with MinIndexTupleSize?". But if
MinIndexTupleSize isn't used anywhere, and I don't see it either,
I think we should just remove the unused macro. It's not buying
anything, and it's confusing because the preceding comment is
about MaxIndexTuplesPerPage and doesn't describe MinIndexTupleSize.

regards, tom lane