Re: TRAP: failed Assert("outerPlan != NULL") in postgres_fdw.c

Sawada-san,

On Wed, Aug 6, 2025 at 3:01 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> Kristian Lejao (colleague, in CC) has found the following assertion
> failure in postgres_fdw.c when rechecking the result tuple via
> EvalPlanQual():
>
> TRAP: failed Assert("outerPlan != NULL"), File: "postgres_fdw.c",
> Line: 2366, PID: 2043518
>
> Here is the reproducible steps that I've simplified from the one
> Kristian originally created:

[...]

> The point is that in the subquery in the target list we pushed the
> inner join to the foreign server. In postgresGetForeignJoinPaths(), we
> prepare the join path for EvalPlanQual() check (and used in
> postgresRecheckForeignScan()) if the query is DELETE, UPDATE, or FOR
> UPDATE/SHARE (as shown below) but we skip it since the subquery itself
> is parsed as a normal SELECT query without rowMarks, leaving
> fdw_outerpath of the ForeignScan node NULL:
>
> /*
> * If there is a possibility that EvalPlanQual will be executed, we need
> * to be able to reconstruct the row using scans of the base relations.
> * GetExistingLocalJoinPath will find a suitable path for this purpose in
> * the path list of the joinrel, if one exists. We must be careful to
> * call it before adding any ForeignPath, since the ForeignPath might
> * dominate the only suitable local path available. We also do it before
> * calling foreign_join_ok(), since that function updates fpinfo and marks
> * it as pushable if the join is found to be pushable.
> */
> if (root->parse->commandType == CMD_DELETE ||
> root->parse->commandType == CMD_UPDATE ||
> root->rowMarks)
> {
> epq_path = GetExistingLocalJoinPath(joinrel);
>
> Therefore, if the tuple is concurrently updated before taking a lock,
> we recheck the traversed tuple via EvalPlanQual() but we end up with
> the assertion failure since we didn't prepare the join plan for that.
>
> The attached patch includes the draft fix and regression tests (using
> injection points).
>
> I don't have enough experience with the planner and FDW code area to
> evaluate whether the patch fixes the issue in the right approach.
> Feedback is very welcome. I've confirmed this assertion could happen
> with the same scenario on all supported branches.

Will review. Thank you for the report and patch!

> In addition to that, I realized that none of the regression tests
> execute postgresRecheckForeignScan()[1]. I think we need to add
> regression tests to cover that function.

Yeah, I think so too.

Best regards,
Etsuro Fujita