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