From: | Edmund Horner <ejrh00(at)gmail(dot)com> |
---|---|
To: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> |
Cc: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Calculate total_table_pages after set_base_rel_sizes() |
Date: | 2018-10-06 05:20:37 |
Message-ID: | CAMyN-kDw19SopObb59B3xf9ywXfZ6MBkg9mkya=90_azowBe8A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
David Rowley said:
> I believe that we should be delaying the PlannerInfo's
> total_table_pages calculation until after constraint exclusion and
> partition pruning have taken place. Doing this calculation before we
> determine which relations we don't need to scan can lead to
> incorrectly applying random_page_cost to too many pages processed
> during an Index Scan.
>
> We already don't count relations removed by join removals from this
> calculation, so counting pruned partitions seems like an omission.
>
> The attached patch moves the calculation to after set_base_rel_sizes()
> is called and before set_base_rel_pathlists() is called, where the
> information is actually used.
>
> I am considering this a bug fix, but I'm proposing this for PG12 only
> as I don't think destabilising plans in the back branches is a good
> idea. I'll add this to the September commitfest.
I played with the new patched today with a set of large partitions.
It seems to work, though the effect is subtle. The expected behaviour
of index_pages_fetched is a bit hard to fathom when the cache share
pushes it between cases A,B and C of the formula, but that's not
really down to this patch.
Basically I think it's ready for a committer to look at. Should I
update the CF entry?
Edmund
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2018-10-06 06:00:38 | Re: Assertion failure with ALTER TABLE ATTACH PARTITION with log_min_messages >= DEBUG1 |
Previous Message | Andrew Gierth | 2018-10-06 04:32:58 | Re: Performance improvements for src/port/snprintf.c |