Thanks for the patch. On Fri, May 19, 2023 at 8:57 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > I wrote: > > Debian Code Search doesn't know of any outside code touching > > relsubs_done, so I think we are safe in dropping that code in > > ExecScanReScan. It seems quite pointless anyway considering > > that up to now, EvalPlanQualBegin has always zeroed the whole > > array. > > Oh, belay that. What I'd forgotten is that it's possible that > the target relation is on the inside of a nestloop, meaning that > we might need to fetch the EPQ substitute tuple more than once. > So there are three possible states: blocked (never return a > tuple), ready to return a tuple, and done returning a tuple > for this scan. ExecScanReScan needs to reset "done" to "ready", > but not touch the "blocked" state. The attached v2 mechanizes > that using two bool arrays.
Aha, that's clever. So ExecScanReScan() would only reset the relsubs_done[] entry for the currently active ("unblocked") target relation, because that would be the only one "unblocked" during a given EvalPlanQual() invocation. + * Initialize per-relation EPQ tuple states. Result relations, if any, + * get marked as blocked; others as not-fetched. Would it be helpful to clarify that "blocked" means blocked for a given EvalPlanQual() cycle? + /* + * relsubs_blocked[scanrelid - 1] is true if there is no EPQ tuple for + * this target relation. + */ + bool *relsubs_blocked; Similarly, maybe say "no EPQ tuple for this target relation in a given EvalPlanQual() invocation" here? BTW, I didn't quite understand why EPQ involving resultRelations must behave in this new way but not the EPQ during LockRows? > What I'm thinking about doing to back-patch this is to replace > one of the pointer fields in EPQState with a pointer to a > subsidiary palloc'd structure, where we can put the new fields > along with the cannibalized old one. We've done something > similar before, and it seems a lot safer than having basically > different logic in v16 than earlier branches. +1. -- Thanks, Amit Langote EDB: http://www.enterprisedb.com