Re: SPLIT/MERGE use of is_internal=true

On Wed, Jul 8, 2026 at 3:18 AM Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
> 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.

The attached patch removes is_internal = true from 3 other function
calls, and removes PERFORM_DELETION_INTERNAL flag from
performDeletionCheck() (the flag in ignored, no functional effect but
code consistency). This is all I managed to find. What do you think?

------
Regards,
Alexander Korotkov
Supabase