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

On Thu, Oct 30, 2025 at 8:08 PM Tender Wang <tndrwang@gmail.com> wrote:
> Amit Langote <amitlangote09@gmail.com> 于2025年10月30日周四 16:29写道：
>> I am not really sure if we should play with the planner code.
>>
>> I suspect the real issue is that we’re assuming partitioned tables
>> always need a ctid, which wasn’t true before MERGE started using the
>> root ResultRelInfo. In fact, the old code already looked wrong -- it’s
>> been requiring a ctid even for partitioned tables where that was never
>> necessary. We can fix this by only requiring the junk ctid when we
>> actually operate through the root partitioned table, that is, for
>> MERGE.  Like the attached.
>
>
> With your patch, this issue didn't happen again.
> But I still get a different output when I enable verbose in EXPLAIN,
>
> Output: ctid (enable_partition_pruning = on)
> vs
> Output: NULL::oid(enable_partition_pruning = off)
>
> From the user's perspective, it's a bit confusing.

Hmm, that's perhaps not ideal.  That's the row identity var "tableoid"
added by expand_single_inheritance_child():

            /*
             * If we have any child target relations, assume they all need to
             * generate a junk "tableoid" column.  (If only one child survives
             * pruning, we wouldn't really need this, but it's not worth
             * thrashing about to avoid it.)
             */
            rrvar = makeVar(childRTindex,
                            TableOidAttributeNumber,
                            OIDOID,
                            -1,
                            InvalidOid,
                            0);
            add_row_identity_var(root, rrvar, childRTindex, "tableoid");

The WHERE false excludes the child that adds the above var, so you end
up with a NULL in the targetlist because of this part of
set_plan_refs():

                    /*
                     * The tlist of a childless Result could contain
                     * unresolved ROWID_VAR Vars, in case it's representing a
                     * target relation which is completely empty because of
                     * constraint exclusion.  Replace any such Vars by null
                     * constants, as though they'd been resolved for a leaf
                     * scan node that doesn't support them.  We could have
                     * fix_scan_expr do this, but since the case is only
                     * expected to occur here, it seems safer to special-case
                     * it here and keep the assertions that ROWID_VARs
                     * shouldn't be seen by fix_scan_expr.
                     *
                     * We also must handle the case where set operations have
                     * been short-circuited resulting in a dummy Result node.
                     * prepunion.c uses varno==0 for the set op targetlist.
                     * See generate_setop_tlist() and generate_setop_tlist().
                     * Here we rewrite these to use varno==1, which is the
                     * varno of the first set-op child.  Without this, EXPLAIN
                     * will have trouble displaying targetlists of dummy set
                     * operations.
                     */
                    foreach(l, splan->plan.targetlist)
                    {
                        TargetEntry *tle = (TargetEntry *) lfirst(l);
                        Var        *var = (Var *) tle->expr;

                        if (var && IsA(var, Var))
                        {
                            if (var->varno == ROWID_VAR)
                                tle->expr = (Expr *) makeNullConst(var->vartype,

var->vartypmod,

var->varcollid);

I’m not sure how hard we should try to avoid that kind of confusion in
the EXPLAIN VERBOSE output.

> I agree more with Tom’s opinion — we should throw an error like "cannot delete from foreign table p1"
> But the plan only had a dummy root relation; CheckValidResultRel() doesn't work.
> Some other code place may need to do something.

Yeah, I’m also not sure there’s an obvious place where we could detect
that earlier. Once pruning removes all real children, the planner ends
up with just a dummy root, and by that point there’s no surviving
foreign table ResultRelInfo to check or even any child relation data
structure in the planner.

--
Thanks, Amit Langote