PageGetFreeSpace() isn't quite the right thing for some of its callers

From: Peter Geoghegan <pg(at)bowt(dot)ie>
To: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Subject: PageGetFreeSpace() isn't quite the right thing for some of its callers
Date: 2019-04-08 21:05:02
Message-ID: CAH2-WznAm=PrtUsoadHknqH_2urtx1D7mwEhS96cuknvqHqdjA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I notice that a number of contrib modules call PageGetFreeSpace()
where they should really call PageGetExactFreeSpace() instead.
PageGetFreeSpace() assumes that the overhead for one line pointer
should be pro-actively subtracted, which is handy for plenty of nbtree
code, but doesn't quite make sense for stuff like pageinspect's
bt_page_stats() function.

I was thinking about fixing one or two of these buglets, without
expecting that to be complicated in any way. However, now that I take
a closer look I also notice that there is core code that calls
PageGetFreeSpace() when it probably shouldn't, either. For example,
what business does heap_xlog_visible() have calling
PageGetFreeSpace()? I also doubt that terminate_brin_buildstate()
should call it -- doesn't BRIN avoid using conventional item pointers?
Doesn't GIN's entryIsEnoughSpace() function double-count the item
pointer overhead?

I wonder if we should add an assertion based on the pd_special offset
of the page to PageGetFreeSpace(). That would make it harder to make
mistakes like this in the future. Maybe it would be better to revise
the whole API instead, though.
--
Peter Geoghegan

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2019-04-08 21:06:39 Re: [PATCH] Implement uuid_version()
Previous Message Jose Luis Tallon 2019-04-08 20:56:00 Re: [PATCH] Implement uuid_version()