On Thu, Aug 2, 2018 at 8:38 AM, Andres Freund <and...@anarazel.de> wrote: > Hi, > > On 2018-08-02 08:21:58 +0530, Amit Kapila wrote: >> I think something on the lines what Tom and you are suggesting can be >> done with the help of EXEC_FLAG_BACKWARD, but I don't see the need to >> do anything for this patch. The change in nodeLimit.c is any way for >> forward scans, so there shouldn't be any need for any other check. > > I think this is almost a guarantee to introduce bugs in the future. And > besides that, as Robert points out, it's essentially an exiting bug for > custom scans. Given that EXEC_FLAG_BACKWARD already exists, why not do > the right thing here? >
Sure, if we want we can do it in this patch, but this is not the problem of this patch. It is also related to existing usage of shutdown callbacks. I think we can use es_top_eflags in Estate to detect it and then call shutdown only if EXEC_FLAG_BACKWARD is not set. I think we should do that as a separate patch either before or after this patch rather than conflating that change into this patch. IIUC, then Robert also said that we should fix that separately. I will do as per whatever consensus we reach here. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
