Two different defs of MAX_TUPLES_PER_PAGE

Started by ITAGAKI Takahiroover 20 years ago4 messageshackers
Jump to latest
#1ITAGAKI Takahiro
itagaki.takahiro@oss.ntt.co.jp

Hi,

I found two different definitions of MAX_TUPLES_PER_PAGE.
Which is reasonable? Or do they have another meaning?

backend/commands/vacuumlazy.c
#define MAX_TUPLES_PER_PAGE ((int) (BLCKSZ / sizeof(HeapTupleHeaderData)))

backend/nodes/tidbitmap.c
#define MAX_TUPLES_PER_PAGE ((BLCKSZ - 1) / MAXALIGN(offsetof(HeapTupleHeaderData, t_bits) + sizeof(ItemIdData)) + 1)

---
ITAGAKI Takahiro
NTT Cyber Space Laboratories

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: ITAGAKI Takahiro (#1)
Re: Two different defs of MAX_TUPLES_PER_PAGE

ITAGAKI Takahiro <itagaki.takahiro@lab.ntt.co.jp> writes:

I found two different definitions of MAX_TUPLES_PER_PAGE.
Which is reasonable? Or do they have another meaning?

Hmm, I think those were both my fault at different times :-(.
Right now I am thinking that they are both not quite right,
in particular it ought to be

#define MAX_TUPLES_PER_PAGE ((BLCKSZ - 1) / (MAXALIGN(offsetof(HeapTupleHeaderData, t_bits)) + sizeof(ItemIdData)) + 1)

That is, the heaptuple space is padded to a MAXALIGN boundary, but the
itemid that points to it isn't. Comments?

(I believe that both modules want a ceiling definition not a floor
definition, ie round up any fraction. The -1 / +1 trick is of course
just one way to get that.)

Also, is this something that should be in a common header file? If so
which one? BLCKSZ, HeapTupleHeaderData, and ItemIdData are all defined
in different places ...

regards, tom lane

#3ITAGAKI Takahiro
itagaki.takahiro@oss.ntt.co.jp
In reply to: Tom Lane (#2)
Re: Two different defs of MAX_TUPLES_PER_PAGE

Tom Lane <tgl@sss.pgh.pa.us> wrote:

#define MAX_TUPLES_PER_PAGE ((BLCKSZ - 1) / (MAXALIGN(offsetof(HeapTupleHeaderData, t_bits)) + sizeof(ItemIdData)) + 1)
(I believe that both modules want a ceiling definition not a floor
definition, ie round up any fraction. The -1 / +1 trick is of course
just one way to get that.)

Don't you think about PageHeaderData? Also I guess a floor definition is ok
because 'number of tuples' is an integer. How about the following?

((BLCKSZ - offsetof(PageHeaderData, pd_linp)) /
(MAXALIGN(offsetof(HeapTupleHeaderData, t_bits)) + sizeof(ItemIdData)))

Also, is this something that should be in a common header file? If so
which one? BLCKSZ, HeapTupleHeaderData, and ItemIdData are all defined
in different places ...

Considering include-hierarchy, I think bufpage.h is a good place.

---
ITAGAKI Takahiro
NTT Cyber Space Laboratories

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: ITAGAKI Takahiro (#3)
Re: Two different defs of MAX_TUPLES_PER_PAGE

ITAGAKI Takahiro <itagaki.takahiro@lab.ntt.co.jp> writes:

Don't you think about PageHeaderData?

I doubt it's really worth taking into account ... we can though.

Also I guess a floor definition is ok
because 'number of tuples' is an integer.

Right, now that I'm more awake I agree with that ;-)

Also, is this something that should be in a common header file? If so
which one? BLCKSZ, HeapTupleHeaderData, and ItemIdData are all defined
in different places ...

Considering include-hierarchy, I think bufpage.h is a good place.

No, that's a pretty bad place because it violates the module hierarchy:
access is on top of storage. None of the include/storage files know
what a HeapTupleHeader looks like.

I think we can just put it in htup.h, since that includes bufpage.h
already. Will make it happen.

regards, tom lane