Re: SPLIT/MERGE use of is_internal=true

On Wed, Jul 8, 2026 at 2:58 AM Noah Misch <noah@leadboat.com> wrote:
>
> On Wed, Jul 08, 2026 at 02:49:45AM +0300, Alexander Korotkov wrote:
> > On Tue, Jul 7, 2026 at 9:57 PM Noah Misch <noah@leadboat.com> wrote:
> > > This and other places in the SPLIT/MERGE patches pass is_internal=true, which
> > > ultimately flows to hooks with the following meaning:
> > >
> > >         /*
> > >          * If this flag is set, the user hasn't requested that the object be
> > >          * altered, but we're doing it anyway for some internal reason.
> > >          * Permissions-checking hooks may want to skip checks if, say, we're alter
> > >          * the constraints of a temporary heap during CLUSTER.
> > >          */
> > >         bool            is_internal;
> > >
> > > SPLIT/MERGE constitute user requests to create and/or drop tables, so they're
> > > not like a CLUSTER temporary heap.  They should pass is_internal=false, like
> > > when the user runs CREATE/DROP directly.
> >
> > Thank you for catching this.  For sure, we shouldn't pass
> > is_internal=true here.  Attached patch fixes this.  I'm going to push
> > it if no objections.
>
> > Subject: [PATCH v1] Don't create SPLIT/MERGE partitions as internal relations
> >
> > createPartitionTable(), which builds the new partitions for ALTER TABLE
> > ... SPLIT PARTITION and ALTER TABLE ... MERGE PARTITIONS, passed
> > is_internal=true to heap_create_with_catalog().  These relations are
> > created at the explicit request of the user, just like a plain CREATE
> > TABLE, so pass is_internal=false instead.  The is_internal flag is meant
> > for objects created as an internal implementation detail (for example, a
> > transient heap built during CLUSTER), and it is observed by
> > object-access hooks.
> >
> > Reported-by: Noah Misch <noah@leadboat.com>
> > Discussion: https://postgr.es/m/20260707185751.f9.noahmisch@microsoft.com
>
> Log should have a "Backpatch-through: 19" line.

Thank you for catching.

> > ---
> >  src/backend/commands/tablecmds.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
> > index 95abaf4890c..0b6a84b32b7 100644
> > --- a/src/backend/commands/tablecmds.c
> > +++ b/src/backend/commands/tablecmds.c
> > @@ -23137,7 +23137,7 @@ createPartitionTable(List **wqueue, RangeVar *newPartName,
> >                                                                               (Datum) 0,
> >                                                                               true,
> >                                                                               allowSystemTableMods,
> > -                                                                             true,
> > +                                                                             false,  /* is_internal */
> >                                                                               InvalidOid,
> >                                                                               NULL);
>
> The edit looks fine for this code site.  As I mentioned above, other
> SPLIT/MERGE code sites need the same change.

This is the only call heap_create_with_catalog() using in SPLIT/MERGE
code.  But now I see there are calls of other catalog functions with
is_internal == true.  I'll check it more precisely tomorrow.

------
Regards,
Alexander Korotkov
Supabase