On Fri, Aug 3, 2018 at 2:09 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: > On Thu, Aug 2, 2018 at 11:14 PM, Robert Haas <robertmh...@gmail.com> wrote: >> On Thu, Aug 2, 2018 at 5:41 AM, Amit Kapila <amit.kapil...@gmail.com> wrote: >>> I have created three patches (a) move InstrStartParallelQuery from its >>> original location so that we perform it just before ExecutorRun (b) >>> patch to fix the gather stats by calling shutdown at appropriate place >>> and allow stats collection in ExecShutdownNode (c) Probit calling >>> ExecShutdownNode if there is a possibility of backward scans (I have >>> done some basic tests with this patch, if we decide to proceed with >>> it, then some more verification and testing would be required). >>> >>> I think we should commit first two patches as that fixes the problem >>> being discussed in this thread and then do some additional >>> verification for the third patch (mentioned in option c). I can >>> understand if people want to commit the third patch before the second >>> patch, so let me know what you guys think. >> >> I'm happy with the first two patches. >> > > Thanks. I have pushed those two patches. > >> In the third one, I don't think >> "See ExecLimit" is a good thing to put a comment like this, because >> it's too hard to find the comment to which it refers, and because >> future commits are likely to edit or remove that comment without >> noticing the references to it from elsewhere. Instead I would just >> write, in all three places, /* If we know we won't need to back up, we >> can release resources at this point. */ or something like that. >> > > Okay, I have changed the comment as per your suggestion in the > attached patch. I will do some more testing/verification of this > patch and will commit over the weekend or on Monday if everything is > fine. >
I have verified that the patch works whenever we use scrollable cursors. Please find the attached patch with the modified commit message. I think now it is a bit late for this minor-release and this doesn't appear to be a blocker issue, it is better to push it after the release. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
prohibit_shutdown_backward_scans_v3.patch
Description: Binary data
