Re: SPLIT/MERGE use of is_internal=true

On Wed, Jul 8, 2026 at 2:58 AM Noah Misch <noah(at)leadboat(dot)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(at)leadboat(dot)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(at)leadboat(dot)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