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

On Wed, Aug 6, 2025 at 8:30 PM Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> wrote:
> On Wed, Aug 6, 2025 at 3:01 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > 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!

First, my apologies for the delay.

I reviewed the postgres_fdw.c part of the fix:

@@ -6313,9 +6314,18 @@ postgresGetForeignJoinPaths(PlannerInfo *root,
* 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)
+ for (PlannerInfo *proot = root; proot != NULL; proot = proot->parent_root)
+ {
+ if (proot->parse->commandType == CMD_DELETE ||
+ proot->parse->commandType == CMD_UPDATE ||
+ proot->rowMarks)
+ {
+ need_epq = true;
+ break;
+ }
+ }
+
+ if (need_epq)
{
epq_path = GetExistingLocalJoinPath(joinrel);
if (!epq_path)

I think this successfully avoids the assertion failure and produces
the correct result, but sorry, I don't think it's the right way to go.
I think the root cause of this issue is in the EPQ handling of
foreign/custom joins in ExecScanFetch() in execScan.h: it runs the
recheck method function for a given foreign/custom join whenever it is
called for EPQ rechecking, but that is not 100% correct. I think the
correct handling is: run the recheck method function for the join if
it is called for EPQ rechecking and the join is a *descendant* join in
the EPQ plan tree; otherwise run the access method function for the
join even if it is called for EPQ rechecking, like the attached (where
I used the epqParam of the given EPQState to determine whether the
join is a descendant join or not, which localizes the fix pretty
well). For the SELECT FOR UPDATE query shown upthread, when doing an
EPQ recheck, the fix evaluates the sub-select expression in the target
list by doing a remote join, not a local join, so it would work more
efficiently than the fix you proposed.

Sorry, I have not yet reviewed the isolation test part. Will do next.

Best regards,
Etsuro Fujita