Re: pgsql: Fix segfault during EvalPlanQual with mix of local and foreign p

On Thu, Aug 12, 2021 at 5:39 PM Heikki Linnakangas
<heikki(dot)linnakangas(at)iki(dot)fi> wrote:
> Fix segfault during EvalPlanQual with mix of local and foreign partitions.
>
> It's not sensible to re-evaluate a direct-modify Foreign Update or Delete
> during EvalPlanQual. However, ExecInitForeignScan() can still get called
> if a table mixes local and foreign partitions. EvalPlanQualStart() left
> the es_result_relations array uninitialized in the child EPQ EState, but
> ExecInitForeignScan() still expected to find it. That caused a segfault.
>
> Fix by skipping the es_result_relations lookup during EvalPlanQual
> processing. To make things a bit more robust, also skip the
> BeginDirectModify calls, and add a runtime check that ExecForeignScan()
> is not called on direct-modify foreign scans during EvalPlanQual
> processing.
>
> This is new in v14, commit 1375422c782. Before that, EvalPlanQualStart()
> copied the whole ResultRelInfo array to the EPQ EState. Backpatch to v14.
>
> Report and diagnosis by Andrey Lepikhov.
>
> Discussion: https://www.postgresql.org/message-id/cb2b808d-cbaa-4772-76ee-c8809bafcf3d%40postgrespro.ru

Thanks Heikki and Andrey for this.

Quite late but to I was looking at this and noticed this comment:

+ /*
+ * Direct modifications cannot be re-evaluated by EvalPlanQual, so
+ * don't bother preparing the FDW. There can ForeignScan nodes in the
+ * EvalPlanQual subtree, but ExecForeignScan should never be called.
+ */

The 2nd sentence seems to be missing a "be" between "can" and
"ForeignScan". Also, does it mean the following?

"There can be ForeignScan nodes in the EvalPlanQual plan subtree, but
ExecForeignScan should never be called on them [during EvalPlanQual]."

I've attached a patch that changes the text like that.

--
Amit Langote
EDB: http://www.enterprisedb.com