MIME-Version: 1.0
References: <19099-e05dcfa022fe553d@postgresql.org> <CACJufxF9FcuYe8XOuWLgWK77HCUHpOc6+7+NkktFFNmzw15jKg@mail.gmail.com>
 <CAHewXN=vF5d9O4R3+iUwLqEaP7pb8iYAN_e3vEE_p5sJHofn7w@mail.gmail.com>
 <2960545.1761800903@sss.pgh.pa.us> <CALdSSPi7udsgQg3PUG=Z4+-9pRg8wT3HkDvTgYvtg30xNWQ9OA@mail.gmail.com>
 <CALdSSPi9n2KGzKQn2Egqz3H8Nx0cgnZ8UeB5gk-KVdE3uBCj6Q@mail.gmail.com>
 <CA+HiwqFcejrmS_H8YB-AMB7sujB7wdJXFPdAVfDC6-19FXUjgg@mail.gmail.com>
 <CAHewXNmx+UXg46+WUrbPca91bmVipRTpe+SRm19GtxG6mArRhg@mail.gmail.com>
 <CALdSSPi6xR1tG2kLvpwNLnAjG9e0wmaY62r2_MF81ZYg5in+qQ@mail.gmail.com>
 <3017911.1761832112@sss.pgh.pa.us> <CAApHDvpYEqJ6h-3NWi_4S19RY9NARpJ3h8CRmWYbz5MJFqE-sg@mail.gmail.com>
 <CA+HiwqEHHTG5_TKuNw1M0dCrgUd6SauJ5dcdicz7xozMJip0SA@mail.gmail.com> <CA+HiwqGk1X-EiVL5kJjHD7V=a3JVDQodt2pwb9SK7q+cYQnpTg@mail.gmail.com>
In-Reply-To: <CA+HiwqGk1X-EiVL5kJjHD7V=a3JVDQodt2pwb9SK7q+cYQnpTg@mail.gmail.com>
From: Amit Langote <amitlangote09@gmail.com>
Date: Thu, 27 Nov 2025 10:25:36 +0900
Message-ID: <CA+HiwqFs-ycozH3XyTa404R2ZqF87UdeGZiCtrp4kaJP2hRF_g@mail.gmail.com>
Subject: Re: BUG #19099: Conditional DELETE from partitioned table with
 non-updatable partition raises internal error
To: David Rowley <dgrowleyml@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Kirill Reshke <reshkekirill@gmail.com>, 
	Tender Wang <tndrwang@gmail.com>, jian he <jian.universality@gmail.com>, exclusion@gmail.com, 
	pgsql-bugs@lists.postgresql.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CA%2BHiwqFs-ycozH3XyTa404R2ZqF87UdeGZiCtrp4kaJP2hRF_g%40mail.gmail.com>
Precedence: bulk

On Wed, Nov 26, 2025 at 8:27=E2=80=AFPM Amit Langote <amitlangote09@gmail.c=
om> wrote:
> On Thu, Nov 6, 2025 at 7:00=E2=80=AFPM 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=E2=80=99s the only target i=
n 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 add=
ed some
> >      * ROWID_VAR Vars to the query.  However, it's possible that constr=
aint
> >      * exclusion suppressed every leaf relation.  The executor will get=
 upset
> >      * if the plan has no row identity columns at all, even though it w=
ill
> >      * certainly process no rows.  Handle this edge case by re-opening =
the top
> >      * result relation and adding the row identity columns it would hav=
e 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 col=
umns
> >      * 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 =3D=3D NIL)
> >     {
> >         Relation    target_relation;
> >
> >         target_relation =3D 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 =3D=3D NIL) block doesn=E2=80=99t=
 run in
> > this case, so it won=E2=80=99t add any row identity columns such as cti=
d for
> > the partitioned root table.
> >
> > In theory, we could prevent the planner from adding tableoid in the
> > first place when the child table doesn=E2=80=99t support any row identi=
ty
> > column -- or worse, doesn=E2=80=99t support the UPDATE/DELETE/MERGE com=
mand 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=E2=80=99ve implemented this alternative as well -- the version that pre=
vents
> 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.

Forgot to mention that with this approach, unlike the other patch, the
targetlists are identical whether or not enable_partition_pruning is
on, since in both cases the if (root->row_identity_vars =3D=3D NIL) block
of distribute_row_identity_vars() executes:

+SET enable_partition_pruning TO off;
+EXPLAIN (COSTS OFF, VERBOSE) DELETE FROM pt WHERE false;
+           QUERY PLAN
+--------------------------------
+ Delete on public.pt
+   ->  Result
+         Output: pt.ctid
+         Replaces: Scan on pt
+         One-Time Filter: false
+(5 rows)

+SET enable_partition_pruning TO on;
+EXPLAIN (COSTS OFF, VERBOSE) DELETE FROM pt WHERE false;
+           QUERY PLAN
+--------------------------------
+ Delete on public.pt
+   ->  Result
+         Output: ctid
+         Replaces: Scan on pt
+         One-Time Filter: false
+(5 rows)

pt.ctid in the off case vs only ctid in the on case has to do, I
think, with there being more than one entry in the rtable in the
pruning off case.

--=20
Thanks, Amit Langote