public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alexander Korotkov <[email protected]>
To: Noah Misch <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: SPLIT/MERGE use of is_internal=true
Date: Wed, 8 Jul 2026 21:32:06 +0300
Message-ID: <CAPpHfdu05sNB4M2N8Rq4ov_G3fyaMRJnCwkmByzwP4YDS8pCSg@mail.gmail.com> (raw)
In-Reply-To: <CAPpHfdtjUedmH2cRr2ZWTEg-hofRauJHkQCh9i91_igt9nEWEQ@mail.gmail.com>
References: <[email protected]>
<CAPpHfdsVnD=BNHFo37sUkMCBi5EpsSJA3DYRGn5rOSdpVonpuA@mail.gmail.com>
<[email protected]>
<CAPpHfdtjUedmH2cRr2ZWTEg-hofRauJHkQCh9i91_igt9nEWEQ@mail.gmail.com>
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
Attachments:
[application/octet-stream] v2-0001-Don-t-create-SPLIT-MERGE-partitions-as-internal-r.patch (3.6K, ../CAPpHfdu05sNB4M2N8Rq4ov_G3fyaMRJnCwkmByzwP4YDS8pCSg@mail.gmail.com/2-v2-0001-Don-t-create-SPLIT-MERGE-partitions-as-internal-r.patch)
download | inline diff:
From 2503fc2aa262c3def2fc04c358f28a63bd31dc2a Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Wed, 8 Jul 2026 21:23:15 +0300
Subject: [PATCH v2] Don't create SPLIT/MERGE partitions as internal relations
The new partitions built for ALTER TABLE ... SPLIT PARTITION and
ALTER TABLE ... MERGE PARTITIONS are created at the explicit request of
the user, just like a plain CREATE TABLE. createPartitionTable() passes
is_internal=true to heap_create_with_catalog(), while createTableConstraints()
does the same to StoreAttrDefault() and AddRelationNewConstraints().
Pass is_internal=false in all these places instead, so that object-access
hooks treat them as user-requested objects. The is_internal flag is intended
for objects created as internal implementation details, such as a transient
heap built during CLUSTER.
While at it, pass 0 rather than PERFORM_DELETION_INTERNAL to the
performDeletionCheck() calls that pre-check the drop eligibility of the
old partitions, to match the subsequent performDeletion(). The flag has
no functional effect on performDeletionCheck(), but change this for code
consistency.
Reported-by: Noah Misch <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 19
---
src/backend/commands/tablecmds.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index b116e70a4bf..cb93c3e935a 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -23063,7 +23063,7 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab,
elog(ERROR, "cannot convert whole-row table reference");
/* Add a pre-cooked default expression. */
- StoreAttrDefault(newRel, num, def, true);
+ StoreAttrDefault(newRel, num, def, false);
/*
* Stored generated column expressions in parent_rel might
@@ -23131,7 +23131,7 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab,
/* Install all CHECK constraints. */
cookedConstraints = AddRelationNewConstraints(newRel, NIL, constraints,
- false, true, true, NULL);
+ false, true, false, NULL);
/* Make the additional catalog changes visible. */
CommandCounterIncrement();
@@ -23193,7 +23193,7 @@ createTableConstraints(List **wqueue, AlteredTableInfo *tab,
* We already set pg_attribute.attnotnull in createPartitionTable. No
* need call set_attnotnull again.
*/
- AddRelationNewConstraints(newRel, NIL, nnconstraints, false, true, true, NULL);
+ AddRelationNewConstraints(newRel, NIL, nnconstraints, false, true, false, NULL);
}
}
@@ -23313,7 +23313,7 @@ createPartitionTable(List **wqueue, RangeVar *newPartName,
(Datum) 0,
true,
allowSystemTableMods,
- true,
+ false, /* is_internal */
InvalidOid,
NULL);
@@ -23885,7 +23885,7 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
object.classId = RelationRelationId;
object.objectSubId = 0;
- performDeletionCheck(&object, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);
+ performDeletionCheck(&object, DROP_RESTRICT, 0);
}
/*
@@ -24299,7 +24299,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
object.objectId = splitRelOid;
object.classId = RelationRelationId;
object.objectSubId = 0;
- performDeletionCheck(&object, DROP_RESTRICT, PERFORM_DELETION_INTERNAL);
+ performDeletionCheck(&object, DROP_RESTRICT, 0);
/*
* If a new partition has the same name as the split partition, then we
--
2.50.1 (Apple Git-155)
view thread (6+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: SPLIT/MERGE use of is_internal=true
In-Reply-To: <CAPpHfdu05sNB4M2N8Rq4ov_G3fyaMRJnCwkmByzwP4YDS8pCSg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox