On Thu, Sep 5, 2019 at 7:53 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Sep 2, 2019 at 4:51 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Fri, Aug 9, 2019 at 6:29 PM Robert Haas <robertmh...@gmail.com> wrote: > > > > > > > > > But beyond that, the issue here is that the Limit node is shutting > > > down the Gather node too early, and the right fix must be to stop > > > doing that, not to change the definition of what it means to shut down > > > a node, as this patch does. So maybe a possible approach here - which > > > I think is more or less what Tom is proposing - is: > > > > > > 1. Remove the code from ExecLimit() that calls ExecShutdownNode(). > > > > > > > Attached patch does that. I have also added one test as a separate > > patch so that later if we introduce shutting down resources in Limit > > node, we don't break anything. As of now, I have kept it separate for > > easy verification, but if we decide to go with this approach and test > > appears fine, we can merge it along with the fix. > > > > I have merged the code change and test case patch as I felt that it is > good to cover this case. I have slightly changed the test case to > make its output predictable (made the inner scan ordered so that the > query always produces the same result). One more thing I am not able > to come up with some predictable test case for 9.6 branches as it > doesn't support Gather Merge which is required for this particular > test to always produce predictable output. There could be some better > way to write this test, so any input in that regards or otherwise is > welcome. So, if we commit this patch the containing test case will be > for branches HEAD~10, but the code will be for HEAD~9.6. >
Robert, Thomas, do you have any more suggestions related to this. I am planning to commit the above-discussed patch (Forbid Limit node to shutdown resources.) coming Monday, so that at least the reported problem got fixed. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
