MIME-Version: 1.0
References: <20260707185751.f9.noahmisch@microsoft.com>
 <CAPpHfdsVnD=BNHFo37sUkMCBi5EpsSJA3DYRGn5rOSdpVonpuA@mail.gmail.com>
 <20260707235839.5f.noahmisch@microsoft.com>
In-Reply-To: <20260707235839.5f.noahmisch@microsoft.com>
From: Alexander Korotkov <aekorotkov@gmail.com>
Date: Wed, 8 Jul 2026 03:18:18 +0300
Message-ID: 
 <CAPpHfdtjUedmH2cRr2ZWTEg-hofRauJHkQCh9i91_igt9nEWEQ@mail.gmail.com>
Subject: Re: SPLIT/MERGE use of is_internal=true
To: Noah Misch <noah@leadboat.com>
Cc: d.koval@postgrespro.ru, pgsql-hackers@postgresql.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAPpHfdtjUedmH2cRr2ZWTEg-hofRauJHkQCh9i91_igt9nEWEQ%40mail.gmail.com>
Precedence: bulk

On Wed, Jul 8, 2026 at 2:58=E2=80=AFAM 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=E2=80=AFPM Noah Misch <noah@leadboat.com> w=
rote:
> > > This and other places in the SPLIT/MERGE patches pass is_internal=3Dt=
rue, which
> > > ultimately flows to hooks with the following meaning:
> > >
> > >         /*
> > >          * If this flag is set, the user hasn't requested that the ob=
ject be
> > >          * altered, but we're doing it anyway for some internal reaso=
n.
> > >          * 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=3Dfa=
lse, like
> > > when the user runs CREATE/DROP directly.
> >
> > Thank you for catching this.  For sure, we shouldn't pass
> > is_internal=3Dtrue 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 rel=
ations
> >
> > createPartitionTable(), which builds the new partitions for ALTER TABLE
> > ... SPLIT PARTITION and ALTER TABLE ... MERGE PARTITIONS, passed
> > is_internal=3Dtrue 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=3Dfalse instead.  The is_internal flag is me=
ant
> > 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.c=
om
>
> 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/ta=
blecmds.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 *n=
ewPartName,
> >                                                                        =
       (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 =3D=3D true.  I'll check it more precisely tomorrow.

------
Regards,
Alexander Korotkov
Supabase