Re: The documentation for READ COMMITTED may be incomplete or wrong

Thanks for the patch.

On Fri, May 19, 2023 at 8:57 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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