> On Aug 17, 2018, at 2:49 AM, David Rowley <david.row...@2ndquadrant.com> > wrote: > > On 17 August 2018 at 06:52, Robert Haas <robertmh...@gmail.com> wrote: >> I don't know whether there's actually a defect here any more. I was >> trying to dispel some perceived confusion on the part of David and Tom >> about what this code was trying to accomplish, but the fact that the >> code caused some confusion does not necessarily mean that there's >> anything wrong with it. On the other hand, perhaps it does have >> functional deficiencies, or perhaps the comments need work. Or maybe >> this code is fine taken in isolation but there are still problems in >> how it interacts with partition pruning. Let's start by deciding what >> we think the problems are, and then we can decide who should fix them. > > I think Tom and I both agree that the plan mentioned in [1] and [2] is > not quite as optimal as it could be. The investigation I did in [3] > highlights where this got broken and the reason why it got broken. > > Tom removed the offending Assert() in 59ef49d26d2, but mentioned > there's no test case which tries to ensure having a partitioned table > as an Append child works correctly. I agree that it would be nice to > have a test for this, but I did try a couple of times to come up with > a test case to do this, but I was unable to come up with anything > suitable compact for the regression tests, and even at that, given how > difficult it is to get a plan into this shape, I didn't hold up much > hope of the test's plan remaining stable. Tom seems to agree with this > as he mentioned in [2]. > > I think the best path forward is if you (Robert) could verify the > behaviour that I mentioned in [3] is expected behaviour, if it is then > perhaps the path forward is for me to try harder to write a compact > test for this, but if you discover that the behaviour is unexpected > then I think the best course of action is to fix it to properly > flatten the > Append which will mean there's no need for a test case, and perhaps > the Assert that was removed can be put back again. > > [1] > https://www.postgresql.org/message-id/CAKJS1f9MQd5ntg6xXg7jJe0jB_bWvCDRmaH6yNzZGF%2BTVa5M%3DA%40mail.gmail.com > [2] https://www.postgresql.org/message-id/8600.1533765148%40sss.pgh.pa.us > [3] > https://www.postgresql.org/message-id/CAKJS1f_j9BSJ7svDF7uid6EMm%2Bfu%2BcAhonZ7hcqiYU4ssv3rtg%40mail.gmail.com > > <https://www.postgresql.org/message-id/cakjs1f_j9bsj7svdf7uid6emm+fu+cahonz7hcqiyu4ssv3...@mail.gmail.com>
On behalf of the RMT, I just want to make sure this keeps moving along. It sounds like the next step is for Robert to verify that [3] is the expected behavior and then David can decide what to do from there. Thanks! Jonathan
signature.asc
Description: Message signed with OpenPGP
