Thread

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

Tender Wang <tndrwang@gmail.com> — 2025-11-30T05:59:59Z

Amit Langote <amitlangote09@gmail.com> 于2025年11月26日周三 19:27写道：

> On Thu, Nov 6, 2025 at 7:00 PM Amit Langote <amitlangote09@gmail.com>
> wrote:
> > Among those options, I considered the following block, which adds a
> > ctid for the partitioned root table when it’s the only target in the
> > query after partition pruning removes all child tables due to the
> > WHERE false condition in the problematic case:
> >
> >     /*
> >      * Ordinarily, we expect that leaf result relation(s) will have
> added some
> >      * ROWID_VAR Vars to the query.  However, it's possible that
> constraint
> >      * exclusion suppressed every leaf relation.  The executor will get
> upset
> >      * if the plan has no row identity columns at all, even though it
> will
> >      * certainly process no rows.  Handle this edge case by re-opening
> the top
> >      * result relation and adding the row identity columns it would have
> used,
> >      * as preprocess_targetlist() would have done if it weren't marked
> "inh".
> >      * Then re-run build_base_rel_tlists() to ensure that the added
> columns
> >      * get propagated to the relation's reltarget.  (This is a bit ugly,
> but
> >      * it seems better to confine the ugliness and extra cycles to this
> >      * unusual corner case.)
> >      */
> >     if (root->row_identity_vars == NIL)
> >     {
> >         Relation    target_relation;
> >
> >         target_relation = table_open(target_rte->relid, NoLock);
> >         add_row_identity_columns(root, result_relation,
> >                                  target_rte, target_relation);
> >         table_close(target_relation, NoLock);
> >         build_base_rel_tlists(root, root->processed_tlist);
> >         /* There are no ROWID_VAR Vars in this case, so we're done. */
> >         return;
> >     }
> >
> > If enable_partition_pruning is off, root->row_identity_vars already
> > contains a RowIdentityVarInfo entry for the tableoid Var that was
> > added while processing the foreign-table child partition. Because of
> > that, the if (root->row_identity_vars == NIL) block doesn’t run in
> > this case, so it won’t add any row identity columns such as ctid for
> > the partitioned root table.
> >
> > In theory, we could prevent the planner from adding tableoid in the
> > first place when the child table doesn’t support any row identity
> > column -- or worse, doesn’t support the UPDATE/DELETE/MERGE command at
> > all -- but doing so would require changing the order in which tableoid
> > appears in root->processed_tlist. That would be too invasive for a
> > back-patch.
>
> I’ve implemented this alternative as well -- the version that prevents
> adding tableoid when no other row-identity columns are added for the
> child. That allows to keep root->row_identity_vars empty so the
> dummy-root path can add ctid as intended by the above code block of
> distribute_row_identity_vars().
>
> This provides an alternative approach to compare against the other patch.
>
> --
> Thanks, Amit Langote
>

I apply the patch, and I find it forgets to update the diff for
postgres_fdw.
So I add it in the v2 patch.
With this patch, the targetlists are identical whether or not
enable_partition_pruning is on.

In my first email on this thread, to avoid adding "tableoid", I tried to
add the following codes:
 "(childrte->relkind != RELKIND_PARTITIONED_TABLE && childrte->relkind
!= RELKIND_FOREIGN_TABLE)"
in expand_single_inheritance_child().

But this didn't work for all test cases. It would trigger an assert failure
in fix_scan_expr_walker():
Assert(!(IsA(node, Var) && ((Var *) node)->varno == ROWID_VAR));

Your patch is much better than mine.

 --
Thanks,
Tender Wang