Re: BUG #19099: Conditional DELETE from partitioned table with non-updatable partition raises internal error

On Fri, Jan 16, 2026 at 4:57 PM Amit Langote <amitlangote09(at)gmail(dot)com>
wrote:
> On Wed, Jan 14, 2026 at 10:38 PM Amit Langote <amitlangote09(at)gmail(dot)com>
wrote:
> > On Wed, Jan 14, 2026 at 9:30 PM Amit Langote <amitlangote09(at)gmail(dot)com>
wrote:
> > > Attached is an updated version with improved comments and simplified
test cases.
> > >
> > > Regarding back-patch safety (to v14 where the bug was introduced):
> > >
> > > * EXPLAIN VERBOSE output order changes (ctid now appears before
tableoid)
> > > * AddForeignUpdateTargets is no longer called when the FDW doesn't
> > > support the command
> >
> > Hit send too soon re the 2nd bit I guess. Actually we can just return
> > false when AddForeignUpdateTargets is missing, so the callback is
> > still called when present.
> >
> > Updated patch attached.
>
> To summarize, the two approaches we've thought about:
>
> 1. Executor-side fix
> (v4-0001-Fix-bogus-ctid-requirement-for-dummy-root-partiti.patch
> posted with my Nov 8 email):
>
> Make ExecInitModifyTable() not require ctid when the only result
> relation is a dummy partitioned root. This is minimally invasive but
> leaves EXPLAIN VERBOSE output inconsistent depending on
> enable_partition_pruning -- with pruning off, you see tableoid but no
> ctid, while with pruning on, you see ctid. That's confusing for users
> as mentioned upthread.
>
> 2. Planner-side fix
> (v4-0001-Fix-row-identity-handling-for-dummy-partitioned-r.patch
> posted with my last email):
>
> Don't add tableoid for child relations that don't contribute
> row-identity columns. This keeps root->row_identity_vars empty when
> there exists only one such child relation, so
> distribute_row_identity_vars() can add ctid for the dummy root.
> EXPLAIN output is consistent regardless of pruning setting. (Some may
> notice in the patch that there's still a minor change, but that's due
> to how explain.c decides whether to print the table name before the
> column name, which is unrelated to this.)
>
> I'm inclined to go with the second approach. The only back-patching
> concern is that EXPLAIN VERBOSE output order changes (ctid now appears
> before tableoid). This is cosmetic -- junk columns are looked up by
> name, not position -- but could affect tests or tools that parse
> EXPLAIN output by position.
>
> If there are no objections, I'll commit patch #2 next week.

Tom, do you have any thoughts on the above?

--
Thanks, Amit Langote