Thread: Should total_pages be calculated after partition pruning andconstraint exclusion?
Should total_pages be calculated after partition pruning andconstraint exclusion?
From
David Rowley
Date:
It seems quite strange to me that we calculate total_pages before any partition pruning and constraint exclusion is performed during set_base_rel_sizes(). Wouldn't it be better to wait until after that's done so we don't mistakenly count relations we're not going to scan? Delaying the calculation until after set_base_rel_sizes() means it's still done in time before it's needed in set_base_rel_pathlists(). The attached patch implements the change. There are no visible plan changes in the regression tests, but the change can affect the plans for larger partitioned tables. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Attachment
Re: Should total_pages be calculated after partition pruning and constraint exclusion?
From
Tom Lane
Date:
David Rowley <david.rowley@2ndquadrant.com> writes: > It seems quite strange to me that we calculate total_pages before any > partition pruning and constraint exclusion is performed during > set_base_rel_sizes(). Wouldn't it be better to wait until after that's > done so we don't mistakenly count relations we're not going to scan? > Delaying the calculation until after set_base_rel_sizes() means it's > still done in time before it's needed in set_base_rel_pathlists(). I think that when this was first done, there was not a separation between those passes over the rangetable, so that such a simple fix would have been impossible. > The attached patch implements the change. Please add to next CF. It's a bit late to be doing such things in v11, IMO. regards, tom lane
Re: Should total_pages be calculated after partition pruning andconstraint exclusion?
From
David Rowley
Date:
On 16 May 2018 at 08:10, Tom Lane <tgl@sss.pgh.pa.us> wrote: > David Rowley <david.rowley@2ndquadrant.com> writes: >> The attached patch implements the change. > > Please add to next CF. It's a bit late to be doing such things in v11, > IMO. If I do that, it'll go under bug fix. It seems strange to delay fixing this until v12. We've still got 1 week before beta. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: Should total_pages be calculated after partition pruning and constraint exclusion?
From
Tom Lane
Date:
David Rowley <david.rowley@2ndquadrant.com> writes: > On 16 May 2018 at 08:10, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> David Rowley <david.rowley@2ndquadrant.com> writes: >> Please add to next CF. It's a bit late to be doing such things in v11, >> IMO. > If I do that, it'll go under bug fix. No, it should go under "planner improvement". If this were a bug fix, it'd be a candidate for back-patch, which IMO it's not --- if only because of the risk of plan destabilization. But for a more direct analogy, if you'd submitted some other improvement in selectivity or cost estimation, that was not a bug fix in the sense of "corrects outright wrong answers", would you expect it to escape feature freeze at this point? Beta or no beta, there has to be a pretty good argument why changes should go into v11 and not v12 once we're past feature freeze. regards, tom lane
Re: Should total_pages be calculated after partition pruning andconstraint exclusion?
From
David Rowley
Date:
On 16 May 2018 at 11:04, Tom Lane <tgl@sss.pgh.pa.us> wrote: > David Rowley <david.rowley@2ndquadrant.com> writes: >> On 16 May 2018 at 08:10, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> David Rowley <david.rowley@2ndquadrant.com> writes: >>> Please add to next CF. It's a bit late to be doing such things in v11, >>> IMO. > >> If I do that, it'll go under bug fix. > > No, it should go under "planner improvement". If this were a bug fix, > it'd be a candidate for back-patch, which IMO it's not --- if only > because of the risk of plan destabilization. But for a more direct > analogy, if you'd submitted some other improvement in selectivity > or cost estimation, that was not a bug fix in the sense of "corrects > outright wrong answers", would you expect it to escape feature freeze > at this point? I'm not going to make a fuss over it, but I guess we must differ in opinion as I would count tracking relation sizes of relations we're actually not going to scan as a bug. I wouldn't have pushed a backpatch due to possible plan destabilisation. People may have pushed effective_cache_size beyond their machines RAM size to work around it. I'll add to the commitfest before it opens., if it's going in as "planner improvement" I'll probably hold off until I'm ready with the other improvements that I'm working on. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: Should total_pages be calculated after partition pruning and constraint exclusion?
From
Tom Lane
Date:
David Rowley <david.rowley@2ndquadrant.com> writes: > On 16 May 2018 at 11:04, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> No, it should go under "planner improvement". If this were a bug fix, >> it'd be a candidate for back-patch, which IMO it's not --- if only >> because of the risk of plan destabilization. > I'm not going to make a fuss over it, but I guess we must differ in > opinion as I would count tracking relation sizes of relations we're > actually not going to scan as a bug. Dunno, the way in which total_pages is used is so squishy/heuristic that it's really hard to say that changing the way we calculate it is necessarily going to lead to better plans. I agree that this is *probably* an improvement, but I don't think it's absolutely clear. If there were some way to paint this as connected to other stuff already done for v11, I'd be happier about pushing it in now --- but it doesn't seem to be. regards, tom lane