On Fri, Mar 30, 2018 at 1:41 AM, Robert Haas <robertmh...@gmail.com> wrote: > On Thu, Mar 29, 2018 at 12:55 AM, Amit Kapila <amit.kapil...@gmail.com> wrote: >> I think that is under acceptable range. I am seeing few regression >> failures with the patch series. The order of targetlist seems to have >> changed for Remote SQL. Kindly find the failure report attached. I >> have requested my colleague Ashutosh Sharma to cross-verify this and >> he is also seeing the same failures. > > Oops. Those just require an expected output change. > >> It seems UPPERREL_TLIST is redundant in the patch now. I think we can >> remove it unless you have something else in mind. > > Yes. > >> I think the handling of partitioned rels looks okay, but we might want >> to once check the overhead of the same unless you are sure that this >> shouldn't be a problem. If you think, we should check it once, then >> is it possible that we can do it as a separate patch as this doesn't >> look to be directly linked to the main patch. It can be treated as an >> optimization for partitionwise aggregates. I think we can treat it >> along with the main patch as well, but it might be somewhat simpler to >> verify it if we do it separately. > > I don't think it should be a problem, although you're welcome to test > it if you're concerned about it. I think it would probably be > penny-wise and pound-foolish to worry about the overhead of > eliminating the Result nodes, which can occur not only with > partition-wise aggregate but also with partition-wise join and, I > think, really any case where the top scan/join plan would be an Append > node. We're talking about a very small amount of additional planning > time to potentially get a better plan. > > I've committed all of these now. >
Cool, I have closed the corresponding CF entry. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
