On Wed, Jul 8, 2026 at 3:18 AM Alexander Korotkov <[email protected]> wrote:
> On Wed, Jul 8, 2026 at 2:58 AM Noah Misch <[email protected]> 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 <[email protected]> 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 <[email protected]>
> > > Discussion: https://postgr.es/m/[email protected]
> >
> > 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

Attachment: v2-0001-Don-t-create-SPLIT-MERGE-partitions-as-internal-r.patch
Description: Binary data

Reply via email to