On Thu, Sep 5, 2019 at 7:53 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Sep 2, 2019 at 4:51 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Fri, Aug 9, 2019 at 6:29 PM Robert Haas <robertmhaas@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