| From: | Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com> |
|---|---|
| To: | Frédéric Yhuel <frederic(dot)yhuel(at)dalibo(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: pgstattuple: fix free space calculation |
| Date: | 2024-08-22 19:56:10 |
| Message-ID: | CA+FpmFf7Q7Sb_zNGe1jiPZ4AoMEAiZ_6usjZes5tcZuHvDaZyg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, 22 Aug 2024 at 10:11, Frédéric Yhuel <frederic(dot)yhuel(at)dalibo(dot)com>
wrote:
> Hello,
>
> I think that pgstattuple should use PageGetExactFreeSpace() instead of
> PageGetHeapFreeSpace() or PageGetFreeSpace(). The latter two compute the
> free space minus the space of a line pointer. They are used like this in
> the rest of the code (heapam.c):
>
> pagefree = PageGetHeapFreeSpace(page);
>
> if (newtupsize > pagefree) { we need a another page for the tuple }
>
> ... so it makes sense to take the line pointer into account in this
> context.
>
> But it in the pgstattuple context, I think we want the exact free space.
>
> I have attached a patch.
>
> Best regards,
> Frédéric
I agree with the approach here.
A minor comment here is to change the comments in code referring to the
PageGetHeapFreeSpace.
--- a/contrib/pgstattuple/pgstatapprox.c
+++ b/contrib/pgstattuple/pgstatapprox.c
@@ -111,7 +111,7 @@ statapprox_heap(Relation rel, output_type *stat)
* treat them as being free space for our purposes.
*/
if (!PageIsNew(page))
- stat->free_space += PageGetHeapFreeSpace(page);
+ stat->free_space += PageGetExactFreeSpace(page);
--
Regards,
Rafia Sabih
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Bruce Momjian | 2024-08-22 20:07:12 | Re: Partial aggregates pushdown |
| Previous Message | Tomas Vondra | 2024-08-22 19:54:02 | Re: Partial aggregates pushdown |