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


---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

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

---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

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



---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend

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

---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly