Re: Calculate total_table_pages after set_base_rel_sizes()
От
Edmund Horner
Тема
Re: Calculate total_table_pages after set_base_rel_sizes()
Дата
Msg-id
153782775738.25845.3844327947842330303.pgcf@coridan.postgresql.org
Ответ на
Calculate total_table_pages after set_base_rel_sizes() (David Rowley)
Список
Дерево обсуждения
Calculate total_table_pages after set_base_rel_sizes() David Rowley <david.rowley@2ndquadrant.com>
Re: Calculate total_table_pages after set_base_rel_sizes() Edmund Horner <ejrh00@gmail.com>
Re: Calculate total_table_pages after set_base_rel_sizes() David Rowley <david.rowley@2ndquadrant.com>
Re: Calculate total_table_pages after set_base_rel_sizes() Tom Lane <tgl@sss.pgh.pa.us>
Re: Calculate total_table_pages after set_base_rel_sizes() David Rowley <david.rowley@2ndquadrant.com>
Re: Calculate total_table_pages after set_base_rel_sizes() Edmund Horner <ejrh00@gmail.com>
Re: Calculate total_table_pages after set_base_rel_sizes() David Rowley <david.rowley@2ndquadrant.com>
Re: Calculate total_table_pages after set_base_rel_sizes() Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Re: Calculate total_table_pages after set_base_rel_sizes() Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Re: Calculate total_table_pages after set_base_rel_sizes() David Rowley <david.rowley@2ndquadrant.com>
Re: Calculate total_table_pages after set_base_rel_sizes() Amit Langote <amitlangote09@gmail.com>
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.
Hi David, I had a quick look at this. (I haven't tested it so this isn't a full review.)
It looks like a fairly straightforward code move. And I think it's correct to exclude the pages from partitions that won't be read.
I have a small tweak. In make_one_rel, we currently have:
/*
* Compute size estimates and consider_parallel flags for each base rel,
* then generate access paths.
*/
set_base_rel_sizes(root);
set_base_rel_pathlists(root);
Your patch inserts code between the two lines. I think the comment should be split too.
/* Compute size estimates and consider_parallel flags for each base rel. */
set_base_rel_sizes(root);
// NEW CODE
/* Generate access paths. */
set_base_rel_pathlists(root);
Cheers,
EdmundВ списке pgsql-hackers по дате отправления