public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 1/3] fix CREATE INDEX progress report with nested partitions
20+ messages / 9 participants
[nested] [flat]
* [PATCH 1/3] fix CREATE INDEX progress report with nested partitions
@ 2023-01-31 15:13 Ilya Gladyshev <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Ilya Gladyshev @ 2023-01-31 15:13 UTC (permalink / raw)
The progress reporting was added in v12 (ab0dfc961) but the original
patch didn't seem to consider the possibility of nested partitioning.
When called recursively, DefineIndex() would clobber the number of
completed partitions, and it was possible to end up with the TOTAL
counter greater than the DONE counter.
This clarifies/re-defines that the progress reporting counts both direct
and indirect children, as well as intermediate partitioned tables:
- The TOTAL counter is set once at the start of the command.
- For indexes which are newly-built, the recursively-called
DefineIndex() increments the DONE counter.
- For pre-existing indexes which are ATTACHed rather than built,
DefineIndex() increments the DONE counter, unless the attached index is
partitioned, in which case progress report is not updated.
Author: Ilya Gladyshev, Justin Pryzby
Reviewed-By: Tomas Vondra, Dean Rasheed, Alvaro Herrera, Matthias van de Meent, Tom Lane
Discussion: https://www.postgresql.org/message-id/flat/a15f904a70924ffa4ca25c3c744cff31e0e6e143.camel%40gmail.co...
---
doc/src/sgml/monitoring.sgml | 10 +++-
src/backend/bootstrap/bootparse.y | 2 +
src/backend/commands/indexcmds.c | 60 +++++++++++++++++--
src/backend/commands/tablecmds.c | 4 +-
src/backend/tcop/utility.c | 6 +-
src/backend/utils/activity/backend_progress.c | 28 +++++++++
src/include/commands/defrem.h | 1 +
src/include/utils/backend_progress.h | 1 +
8 files changed, 103 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 7ab4424bf13..cf88dca53f3 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -6929,7 +6929,10 @@ FROM pg_stat_get_backend_idset() AS backendid;
</para>
<para>
When creating an index on a partitioned table, this column is set to
- the total number of partitions on which the index is to be created.
+ the total number of partitions on which the index is to be created or attached.
+ In the case of intermediate partitioned tables, this includes both
+ direct and indirect partitions, and includes the intermediate
+ partitioned tables themselves.
This field is <literal>0</literal> during a <literal>REINDEX</literal>.
</para></entry>
</row>
@@ -6940,7 +6943,10 @@ FROM pg_stat_get_backend_idset() AS backendid;
</para>
<para>
When creating an index on a partitioned table, this column is set to
- the number of partitions on which the index has been created.
+ the number of partitions on which the index has been created or attached.
+ In the case of intermediate partitioned tables, this includes both
+ direct and indirect partitions, and includes the intermediate
+ partitioned tables themselves.
This field is <literal>0</literal> during a <literal>REINDEX</literal>.
</para></entry>
</row>
diff --git a/src/backend/bootstrap/bootparse.y b/src/backend/bootstrap/bootparse.y
index 86804bb598e..81a1b7bfec3 100644
--- a/src/backend/bootstrap/bootparse.y
+++ b/src/backend/bootstrap/bootparse.y
@@ -306,6 +306,7 @@ Boot_DeclareIndexStmt:
$4,
InvalidOid,
InvalidOid,
+ -1,
false,
false,
false,
@@ -358,6 +359,7 @@ Boot_DeclareUniqueIndexStmt:
$5,
InvalidOid,
InvalidOid,
+ -1,
false,
false,
false,
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index ff48f44c66f..edb65d4cacb 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -521,6 +521,7 @@ WaitForOlderSnapshots(TransactionId limitXmin, bool progress)
* case the caller had better have checked it earlier.
* 'skip_build': make the catalog entries but don't create the index files
* 'quiet': suppress the NOTICE chatter ordinarily provided for constraints.
+ * 'total_parts': total number of direct and indirect partitions
*
* Returns the object address of the created index.
*/
@@ -530,6 +531,7 @@ DefineIndex(Oid relationId,
Oid indexRelationId,
Oid parentIndexId,
Oid parentConstraintId,
+ int total_parts,
bool is_alter_table,
bool check_rights,
bool check_not_in_use,
@@ -1225,8 +1227,30 @@ DefineIndex(Oid relationId,
Relation parentIndex;
TupleDesc parentDesc;
- pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
- nparts);
+ /*
+ * Set the total number of partitions at the start of the command;
+ * but don't update it when being called recursively.
+ */
+ if (!OidIsValid(parentIndexId))
+ {
+ /*
+ * When called by ProcessUtilitySlow(), the number of
+ * partitions is passed in as an optimization. This should
+ * count partitions the same way. Subtract one since
+ * find_all_inheritors() includes the rel itself.
+ */
+ if (total_parts < 0)
+ {
+ List *childs = find_all_inheritors(relationId,
+ NoLock, NULL);
+
+ total_parts = list_length(childs) - 1;
+ list_free(childs);
+ }
+
+ pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_TOTAL,
+ total_parts);
+ }
/* Make a local copy of partdesc->oids[], just for safety */
memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts);
@@ -1432,14 +1456,23 @@ DefineIndex(Oid relationId,
InvalidOid, /* no predefined OID */
indexRelationId, /* this is our child */
createdConstraintId,
+ -1,
is_alter_table, check_rights, check_not_in_use,
skip_build, quiet);
SetUserIdAndSecContext(child_save_userid,
child_save_sec_context);
}
+ else
+ {
+ /*
+ * If a pre-existing index was ATTACHed, the progress
+ * report is updated here. A partitioned index is
+ * likewise counted when attached, but not its partitions,
+ * since that's expensive, and ATTACH is fast anyway.
+ */
+ pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
+ }
- pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE,
- i + 1);
free_attrmap(attmap);
}
@@ -1479,6 +1512,16 @@ DefineIndex(Oid relationId,
table_close(rel, NoLock);
if (!OidIsValid(parentIndexId))
pgstat_progress_end_command();
+ else
+ {
+ /*
+ * Update progress for a partitioned index itself; the
+ * recursively-called function will have updated the counter for
+ * its child indexes.
+ */
+ pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
+ }
+
return address;
}
@@ -1490,9 +1533,16 @@ DefineIndex(Oid relationId,
/* Close the heap and we're done, in the non-concurrent case */
table_close(rel, NoLock);
- /* If this is the top-level index, we're done. */
+ /*
+ * If this is the top-level index, the command is done. When called
+ * recursively for child tables, the done partition counter is
+ * incremented now, rather than in the caller, to provide fine-grained
+ * progress reporting in the case of intermediate partitioning.
+ */
if (!OidIsValid(parentIndexId))
pgstat_progress_end_command();
+ else
+ pgstat_progress_incr_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, 1);
return address;
}
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3e2c5f797cd..cc74d703f91 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -1216,6 +1216,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
InvalidOid,
RelationGetRelid(idxRel),
constraintOid,
+ -1,
false, false, false, false, false);
index_close(idxRel, AccessShareLock);
@@ -8640,6 +8641,7 @@ ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
InvalidOid, /* no predefined OID */
InvalidOid, /* no parent index */
InvalidOid, /* no parent constraint */
+ -1, /* total_parts unknown */
true, /* is_alter_table */
check_rights,
false, /* check_not_in_use - we did it already */
@@ -18105,7 +18107,7 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel)
&conOid);
DefineIndex(RelationGetRelid(attachrel), stmt, InvalidOid,
RelationGetRelid(idxRel),
- conOid,
+ conOid, -1,
true, false, false, false, false);
}
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index eada7353639..2dc92c72cea 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1465,6 +1465,7 @@ ProcessUtilitySlow(ParseState *pstate,
Oid relid;
LOCKMODE lockmode;
bool is_alter_table;
+ int nparts = 0;
if (stmt->concurrent)
PreventInTransactionBlock(isTopLevel,
@@ -1503,9 +1504,11 @@ ProcessUtilitySlow(ParseState *pstate,
List *inheritors = NIL;
inheritors = find_all_inheritors(relid, lockmode, NULL);
+ nparts = list_length(inheritors) - 1;
foreach(lc, inheritors)
{
- char relkind = get_rel_relkind(lfirst_oid(lc));
+ Oid partrelid = lfirst_oid(lc);
+ char relkind = get_rel_relkind(partrelid);
if (relkind != RELKIND_RELATION &&
relkind != RELKIND_MATVIEW &&
@@ -1548,6 +1551,7 @@ ProcessUtilitySlow(ParseState *pstate,
InvalidOid, /* no predefined OID */
InvalidOid, /* no parent index */
InvalidOid, /* no parent constraint */
+ nparts,
is_alter_table,
true, /* check_rights */
true, /* check_not_in_use */
diff --git a/src/backend/utils/activity/backend_progress.c b/src/backend/utils/activity/backend_progress.c
index d96af812b19..2a9994b98fd 100644
--- a/src/backend/utils/activity/backend_progress.c
+++ b/src/backend/utils/activity/backend_progress.c
@@ -58,6 +58,34 @@ pgstat_progress_update_param(int index, int64 val)
PGSTAT_END_WRITE_ACTIVITY(beentry);
}
+/*-----------
+ * pgstat_progress_incr_param() -
+ *
+ * Increment index'th member in st_progress_param[] of the current backend.
+ *-----------
+ */
+void
+pgstat_progress_incr_param(int index, int64 incr)
+{
+ volatile PgBackendStatus *beentry = MyBEEntry;
+ int64 val;
+
+ Assert(index >= 0 && index < PGSTAT_NUM_PROGRESS_PARAM);
+
+ if (!beentry || !pgstat_track_activities)
+ return;
+
+ /*
+ * Because no other process should write to this backend's own status, we
+ * can read its value from shared memory without needing to loop to ensure
+ * its consistency.
+ */
+ val = beentry->st_progress_param[index];
+ val += incr;
+
+ pgstat_progress_update_param(index, val);
+}
+
/*-----------
* pgstat_progress_update_multi_param() -
*
diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h
index 4f7f87fc62c..478203ed4c4 100644
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -29,6 +29,7 @@ extern ObjectAddress DefineIndex(Oid relationId,
Oid indexRelationId,
Oid parentIndexId,
Oid parentConstraintId,
+ int total_parts,
bool is_alter_table,
bool check_rights,
bool check_not_in_use,
diff --git a/src/include/utils/backend_progress.h b/src/include/utils/backend_progress.h
index 005e5d75ab6..a84752ade99 100644
--- a/src/include/utils/backend_progress.h
+++ b/src/include/utils/backend_progress.h
@@ -36,6 +36,7 @@ typedef enum ProgressCommandType
extern void pgstat_progress_start_command(ProgressCommandType cmdtype,
Oid relid);
extern void pgstat_progress_update_param(int index, int64 val);
+extern void pgstat_progress_incr_param(int index, int64 incr);
extern void pgstat_progress_update_multi_param(int nparam, const int *index,
const int64 *val);
extern void pgstat_progress_end_command(void);
--
2.34.1
--Jk19qaSm7VaNmsB6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0002-assertions-for-progress-reporting.patch"
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-04-06 22:22 Alexander Korotkov <[email protected]>
0 siblings, 4 replies; 20+ messages in thread
From: Alexander Korotkov @ 2024-04-06 22:22 UTC (permalink / raw)
To: Dmitry Koval <[email protected]>; +Cc: [email protected]
Hi, Dmitry!
On Fri, Apr 5, 2024 at 4:00 PM Dmitry Koval <[email protected]> wrote:
> > I've revised the patchset.
>
> Thanks for the corrections (especially ddl.sgml).
> Could you also look at a small optimization for the MERGE PARTITIONS
> command (in a separate file
> v31-0003-Additional-patch-for-ALTER-TABLE-.-MERGE-PARTITI.patch, I wrote
> about it in an email 2024-03-31 00:56:50)?
>
> Files v31-0001-*.patch, v31-0002-*.patch are the same as
> v30-0001-*.patch, v30-0002-*.patch (after rebasing because patch stopped
> applying due to changes in upstream).
I've pushed 0001 and 0002. I didn't push 0003 for the following reasons.
1) This doesn't keep functionality equivalent to 0001. With 0003, the
merged partition will inherit indexes, constraints, and so on from the
one of merging partitions.
2) This is not necessarily an optimization. Without 0003 indexes on
the merged partition are created after moving the rows in
attachPartitionTable(). With 0003 we merge data into the existing
partition which saves its indexes. That might cause a significant
performance loss because mass inserts into indexes may be much slower
than building indexes from scratch.
I think both aspects need to be carefully considered. Even if we
accept them, this needs to be documented. I think now it's too late
for both of these. So, this should wait for v18.
------
Regards,
Alexander Korotkov
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-04-06 22:38 Dmitry Koval <[email protected]>
parent: Alexander Korotkov <[email protected]>
3 siblings, 0 replies; 20+ messages in thread
From: Dmitry Koval @ 2024-04-06 22:38 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: [email protected]
Hi, Alexander!
> I didn't push 0003 for the following reasons. ....
Thanks for clarifying. You are right, these are serious reasons.
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.com
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-04-07 19:00 Alexander Lakhin <[email protected]>
parent: Alexander Korotkov <[email protected]>
3 siblings, 0 replies; 20+ messages in thread
From: Alexander Lakhin @ 2024-04-07 19:00 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; Dmitry Koval <[email protected]>; +Cc: [email protected]
Hi Alexander and Dmitry,
07.04.2024 01:22, Alexander Korotkov wrote:
> I've pushed 0001 and 0002. I didn't push 0003 for the following reasons.
Please try the following (erroneous) query:
CREATE TABLE t1(i int, t text) PARTITION BY LIST (t);
CREATE TABLE t1pa PARTITION OF t1 FOR VALUES IN ('A');
CREATE TABLE t2 (i int, t text) PARTITION BY RANGE (t);
ALTER TABLE t2 SPLIT PARTITION t1pa INTO
(PARTITION t2a FOR VALUES FROM ('A') TO ('B'),
PARTITION t2b FOR VALUES FROM ('B') TO ('C'));
that triggers an assertion failure:
TRAP: failed Assert("datums != NIL"), File: "partbounds.c", Line: 3434, PID: 1841459
or a segfault (in a non-assert build):
Program terminated with signal SIGSEGV, Segmentation fault.
#0 pg_detoast_datum_packed (datum=0x0) at fmgr.c:1866
1866 if (VARATT_IS_COMPRESSED(datum) || VARATT_IS_EXTERNAL(datum))
(gdb) bt
#0 pg_detoast_datum_packed (datum=0x0) at fmgr.c:1866
#1 0x000055f38c5d5e3f in bttextcmp (...) at varlena.c:1834
#2 0x000055f38c6030dd in FunctionCall2Coll (...) at fmgr.c:1161
#3 0x000055f38c417c83 in partition_rbound_cmp (...) at partbounds.c:3525
#4 check_partition_bounds_for_split_range (...) at partbounds.c:5221
#5 check_partitions_for_split (...) at partbounds.c:5688
#6 0x000055f38c256c49 in transformPartitionCmdForSplit (...) at parse_utilcmd.c:3451
#7 transformAlterTableStmt (...) at parse_utilcmd.c:3810
#8 0x000055f38c2bdf9c in ATParseTransformCmd (...) at tablecmds.c:5650
...
Best regards,
Alexander
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-04-08 10:43 Tender Wang <[email protected]>
parent: Alexander Korotkov <[email protected]>
3 siblings, 1 reply; 20+ messages in thread
From: Tender Wang @ 2024-04-08 10:43 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; Dmitry Koval <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
Hi all,
I went through the MERGE/SPLIT partition codes today, thanks for the
works. I found some grammar errors:
i. in error messages(Users can see this grammar errors, not friendly).
ii. in codes comments
Alexander Korotkov <[email protected]> 于2024年4月7日周日 06:23写道：
> Hi, Dmitry!
>
> On Fri, Apr 5, 2024 at 4:00 PM Dmitry Koval <[email protected]>
> wrote:
> > > I've revised the patchset.
> >
> > Thanks for the corrections (especially ddl.sgml).
> > Could you also look at a small optimization for the MERGE PARTITIONS
> > command (in a separate file
> > v31-0003-Additional-patch-for-ALTER-TABLE-.-MERGE-PARTITI.patch, I wrote
> > about it in an email 2024-03-31 00:56:50)?
> >
> > Files v31-0001-*.patch, v31-0002-*.patch are the same as
> > v30-0001-*.patch, v30-0002-*.patch (after rebasing because patch stopped
> > applying due to changes in upstream).
>
> I've pushed 0001 and 0002. I didn't push 0003 for the following reasons.
> 1) This doesn't keep functionality equivalent to 0001. With 0003, the
> merged partition will inherit indexes, constraints, and so on from the
> one of merging partitions.
> 2) This is not necessarily an optimization. Without 0003 indexes on
> the merged partition are created after moving the rows in
> attachPartitionTable(). With 0003 we merge data into the existing
> partition which saves its indexes. That might cause a significant
> performance loss because mass inserts into indexes may be much slower
> than building indexes from scratch.
> I think both aspects need to be carefully considered. Even if we
> accept them, this needs to be documented. I think now it's too late
> for both of these. So, this should wait for v18.
>
> ------
> Regards,
> Alexander Korotkov
>
>
>
--
Tender Wang
OpenPie: https://en.openpie.com/
Attachments:
[application/octet-stream] 0001-Fix-some-grammer-errors-from-error-messages-and-code.patch (6.3K, ../../CAHewXNkGMPU50QG7V6Q60JGFORfo8LfYO1_GCkCa0VWbmB-fEw@mail.gmail.com/3-0001-Fix-some-grammer-errors-from-error-messages-and-code.patch)
download | inline diff:
From 2b0fff52002df8e4dc35c7b50d2ffa9e302b50d7 Mon Sep 17 00:00:00 2001
From: "tender.wang" <[email protected]>
Date: Mon, 8 Apr 2024 18:35:05 +0800
Subject: [PATCH] Fix some grammer errors from error messages and codes
comments.
---
src/backend/commands/tablecmds.c | 18 +++++++++---------
src/backend/parser/parse_utilcmd.c | 2 +-
src/backend/partitioning/partbounds.c | 6 +++---
src/include/nodes/parsenodes.h | 2 +-
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 582890a302..2b0b594d75 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -19086,7 +19086,7 @@ QueuePartitionConstraintValidation(List **wqueue, Relation scanrel,
}
/*
- * attachPartitionTable: attach new partition to partitioned table
+ * attachPartitionTable: attach a new partition to the partitioned table
*
* wqueue: the ALTER TABLE work queue; can be NULL when not running as part
* of an ALTER TABLE sequence.
@@ -20879,10 +20879,10 @@ GetAttributeStorage(Oid atttypid, const char *storagemode)
*/
typedef struct SplitPartitionContext
{
- ExprState *partqualstate; /* expression for check slot for partition
+ ExprState *partqualstate; /* expression for checking slot for partition
* (NULL for DEFAULT partition) */
BulkInsertState bistate; /* state of bulk inserts for partition */
- TupleTableSlot *dstslot; /* slot for insert row into partition */
+ TupleTableSlot *dstslot; /* slot for inserting row into partition */
Relation partRel; /* relation for partition */
} SplitPartitionContext;
@@ -21129,7 +21129,7 @@ moveSplitTableRows(Relation rel, Relation splitRel, List *partlist, List *newPar
}
/*
- * createPartitionTable: create table for new partition with given name
+ * createPartitionTable: create table for a new partition with given name
* (newPartName) like table (modelRelName)
*
* Emulates command: CREATE TABLE <newPartName> (LIKE <modelRelName>
@@ -21206,7 +21206,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
/*
* We are going to detach and remove this partition: need to use exclusive
- * lock for prevent DML-queries to the partition.
+ * lock for preventing DML-queries to the partition.
*/
splitRel = table_openrv(cmd->name, AccessExclusiveLock);
@@ -21256,13 +21256,13 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
/*
* If new partition has the same name as split partition then we should
- * rename split partition for reuse name.
+ * rename split partition for reusing name.
*/
if (isSameName)
{
/*
* We must bump the command counter to make the split partition tuple
- * visible for rename.
+ * visible for renaming.
*/
CommandCounterIncrement();
/* Rename partition. */
@@ -21271,7 +21271,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
/*
* We must bump the command counter to make the split partition tuple
- * visible after rename.
+ * visible after renaming.
*/
CommandCounterIncrement();
}
@@ -21446,7 +21446,7 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
/*
* We are going to detach and remove this partition: need to use
- * exclusive lock for prevent DML-queries to the partition.
+ * exclusive lock for preventing DML-queries to the partition.
*/
mergingPartition = table_openrv(name, AccessExclusiveLock);
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 88a4a41186..9e3e14087f 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -3804,7 +3804,7 @@ transformAlterTableStmt(Oid relid, AlterTableStmt *stmt,
if (list_length(partcmd->partlist) < 2)
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
- errmsg("list of new partitions should contains at least two items")));
+ errmsg("list of new partitions should contain at least two items")));
if (cmd->subtype == AT_SplitPartition)
transformPartitionCmdForSplit(&cxt, partcmd);
diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index c36e26ba4b..83df89e9b1 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -4986,7 +4986,7 @@ satisfies_hash_partition(PG_FUNCTION_ARGS)
* This is a helper function for check_partitions_for_split() and
* calculate_partition_bound_for_merge().
* This function compares upper bound of first_bound and lower bound of
- * second_bound. These bounds should be equals except case
+ * second_bound. These bounds should be equal except case
* "defaultPart == true" (this means that one of split partitions is DEFAULT).
* In this case upper bound of first_bound can be less than lower bound of
* second_bound because space between of these bounds will be included in
@@ -5262,7 +5262,7 @@ check_partition_bounds_for_split_range(Relation parent,
errmsg("%s bound of partition \"%s\" is %s %s bound of split partition",
first ? "lower" : "upper",
relname,
- defaultPart ? (first ? "less than" : "greater than") : "not equals to",
+ defaultPart ? (first ? "less than" : "greater than") : "not equal to",
first ? "lower" : "upper"),
parser_errposition(pstate, datum->location)));
}
@@ -5483,7 +5483,7 @@ check_parent_values_in_new_partitions(Relation parent,
Const *notFoundVal;
if (!searchNull)
- /* Make Const for get string representation of not found value. */
+ /* Make Const for getting string representation of not found value. */
notFoundVal = makeConst(key->parttypid[0],
key->parttypmod[0],
key->parttypcoll[0],
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 01fa1a6c2e..32df680a9a 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -956,7 +956,7 @@ typedef struct PartitionCmd
NodeTag type;
RangeVar *name; /* name of partition to attach/detach */
PartitionBoundSpec *bound; /* FOR VALUES, if attaching */
- List *partlist; /* list of partitions, for SPLIT PARTITION
+ List *partlist; /* list of partitions, for MERGE/SPLIT PARTITION
* command */
bool concurrent;
} PartitionCmd;
--
2.25.1
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-04-08 12:00 Alexander Lakhin <[email protected]>
parent: Tender Wang <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Alexander Lakhin @ 2024-04-08 12:00 UTC (permalink / raw)
To: Tender Wang <[email protected]>; Alexander Korotkov <[email protected]>; Dmitry Koval <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
Hi Tender Wang,
08.04.2024 13:43, Tender Wang wrote:
> Hi all,
> I went through the MERGE/SPLIT partition codes today, thanks for the works. I found some grammar errors:
> i. in error messages(Users can see this grammar errors, not friendly).
> ii. in codes comments
>
On a quick glance, I saw also:
NULL-value
partitionde
splited
temparary
And a trailing whitespace at:
the quarter partition back to monthly partitions:
warning: 1 line adds whitespace errors.
I'm also confused by "administrators" here:
https://www.postgresql.org/docs/devel/ddl-partitioning.html
(We can find on the same page, for instance:
... whereas table inheritance allows data to be divided in a manner of
the user's choosing.
It seems to me, that "users" should work for merging partitions as well.)
Though the documentation addition requires more than just a quick glance,
of course.
Best regards,
Alexander
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-04-08 20:43 Dmitry Koval <[email protected]>
parent: Alexander Lakhin <[email protected]>
0 siblings, 0 replies; 20+ messages in thread
From: Dmitry Koval @ 2024-04-08 20:43 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
Hi!
Attached fix for the problems found by Alexander Lakhin.
About grammar errors.
Unfortunately, I don't know English well.
Therefore, I plan (in the coming days) to show the text to specialists
who perform technical translation of documentation.
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.com
From 578b3fae50baffa3626570447d55ce4177fc6e7d Mon Sep 17 00:00:00 2001
From: Koval Dmitry <[email protected]>
Date: Mon, 8 Apr 2024 23:10:52 +0300
Subject: [PATCH v1] Fixes for ALTER TABLE ... SPLIT/MERGE PARTITIONS ...
commands
---
doc/src/sgml/ddl.sgml | 2 +-
src/backend/commands/tablecmds.c | 12 ------
src/backend/parser/parse_utilcmd.c | 38 +++++++++++++++++++
src/test/regress/expected/partition_merge.out | 29 +++++++++++---
src/test/regress/expected/partition_split.out | 13 +++++++
src/test/regress/sql/partition_merge.sql | 24 ++++++++++--
src/test/regress/sql/partition_split.sql | 15 ++++++++
7 files changed, 111 insertions(+), 22 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 8ff9a520ca..cd8304ef75 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -4410,7 +4410,7 @@ ALTER TABLE measurement
this operation is not supported for hash-partitioned tables and acquires
an <literal>ACCESS EXCLUSIVE</literal> lock, which could impact high-load
systems due to the lock's restrictive nature. For example, we can split
- the quarter partition back to monthly partitions:
+ the quarter partition back to monthly partitions:
<programlisting>
ALTER TABLE measurement SPLIT PARTITION measurement_y2006q1 INTO
(PARTITION measurement_y2006m01 FOR VALUES FROM ('2006-01-01') TO ('2006-02-01'),
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 865c6331c1..8a98a0af48 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -21223,12 +21223,6 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
*/
splitRel = table_openrv(cmd->name, AccessExclusiveLock);
- if (splitRel->rd_rel->relkind != RELKIND_RELATION)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot split non-table partition \"%s\"",
- RelationGetRelationName(splitRel))));
-
splitRelOid = RelationGetRelid(splitRel);
/* Check descriptions of new partitions. */
@@ -21463,12 +21457,6 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
*/
mergingPartition = table_openrv(name, AccessExclusiveLock);
- if (mergingPartition->rd_rel->relkind != RELKIND_RELATION)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot merge non-table partition \"%s\"",
- RelationGetRelationName(mergingPartition))));
-
/*
* Checking that two partitions have the same name was before, in
* function transformPartitionCmdForMerge().
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 9e3e14087f..0d5ed0079c 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -32,6 +32,7 @@
#include "catalog/heap.h"
#include "catalog/index.h"
#include "catalog/namespace.h"
+#include "catalog/partition.h"
#include "catalog/pg_am.h"
#include "catalog/pg_collation.h"
#include "catalog/pg_constraint.h"
@@ -3415,6 +3416,38 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString,
}
+/*
+ * checkPartition: check that partRelOid is partition of rel
+ */
+static void
+checkPartition(Relation rel, Oid partRelOid)
+{
+ Relation partRel;
+
+ partRel = relation_open(partRelOid, AccessShareLock);
+
+ if (partRel->rd_rel->relkind != RELKIND_RELATION)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is not a table",
+ RelationGetRelationName(partRel))));
+
+ if (!partRel->rd_rel->relispartition)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is not a partition",
+ RelationGetRelationName(partRel))));
+
+ if (get_partition_parent(partRelOid, false) != RelationGetRelid(rel))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_TABLE),
+ errmsg("relation \"%s\" is not a partition of relation \"%s\"",
+ RelationGetRelationName(partRel),
+ RelationGetRelationName(rel))));
+
+ relation_close(partRel, AccessShareLock);
+}
+
/*
* transformPartitionCmdForSplit
* Analyze the ALTER TABLLE ... SPLIT PARTITION command
@@ -3447,6 +3480,8 @@ transformPartitionCmdForSplit(CreateStmtContext *cxt, PartitionCmd *partcmd)
splitPartOid = RangeVarGetRelid(partcmd->name, NoLock, false);
+ checkPartition(parent, splitPartOid);
+
/* Then we should check partitions with transformed bounds. */
check_partitions_for_split(parent, splitPartOid, partcmd->name, partcmd->partlist, cxt->pstate);
}
@@ -3509,6 +3544,9 @@ transformPartitionCmdForMerge(CreateStmtContext *cxt, PartitionCmd *partcmd)
partOid = RangeVarGetRelid(name, NoLock, false);
if (partOid == defaultPartOid)
isDefaultPart = true;
+
+ checkPartition(parent, partOid);
+
partOids = lappend_oid(partOids, partOid);
}
diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out
index 2ba0ec47d9..60eacf6bf3 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -25,9 +25,9 @@ ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022, sales_mar2022, sales_fe
ERROR: partition with name "sales_feb2022" already used
LINE 1: ...e MERGE PARTITIONS (sales_feb2022, sales_mar2022, sales_feb2...
^
--- ERROR: cannot merge non-table partition "sales_apr2022"
+-- ERROR: "sales_apr2022" is not a table
ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022, sales_mar2022, sales_apr2022) INTO sales_feb_mar_apr2022;
-ERROR: cannot merge non-table partition "sales_apr2022"
+ERROR: "sales_apr2022" is not a table
-- ERROR: invalid partitions order, partition "sales_mar2022" can not be merged
-- (space between sections sales_jan2022 and sales_mar2022)
ALTER TABLE sales_range MERGE PARTITIONS (sales_jan2022, sales_mar2022) INTO sales_jan_mar2022;
@@ -590,12 +590,12 @@ CREATE TABLE sales_nord2 PARTITION OF sales_list2 FOR VALUES IN ('Oslo', 'St. Pe
CREATE TABLE sales_others2 PARTITION OF sales_list2 DEFAULT;
CREATE TABLE sales_external (LIKE sales_list);
CREATE TABLE sales_external2 (vch VARCHAR(5));
--- ERROR: partition bound for relation "sales_external" is null
+-- ERROR: "sales_external" is not a partition
ALTER TABLE sales_list MERGE PARTITIONS (sales_west, sales_east, sales_external) INTO sales_all;
-ERROR: partition bound for relation "sales_external" is null
--- ERROR: partition bound for relation "sales_external2" is null
+ERROR: "sales_external" is not a partition
+-- ERROR: "sales_external2" is not a partition
ALTER TABLE sales_list MERGE PARTITIONS (sales_west, sales_east, sales_external2) INTO sales_all;
-ERROR: partition bound for relation "sales_external2" is null
+ERROR: "sales_external2" is not a partition
-- ERROR: relation "sales_nord2" is not a partition of relation "sales_list"
ALTER TABLE sales_list MERGE PARTITIONS (sales_west, sales_nord2, sales_east) INTO sales_all;
ERROR: relation "sales_nord2" is not a partition of relation "sales_list"
@@ -729,4 +729,21 @@ SELECT * FROM sales_list WHERE salesman_name = 'Ivanov';
RESET enable_seqscan;
DROP TABLE sales_list;
--
+-- Try to MERGE partitions of another table.
+--
+CREATE TABLE t1 (i int, a int, b int, c int) PARTITION BY RANGE (a, b);
+CREATE TABLE t1p1 PARTITION OF t1 FOR VALUES FROM (1, 1) TO (1, 2);
+CREATE TABLE t2 (i int, t text) PARTITION BY RANGE (t);
+CREATE TABLE t2pa PARTITION OF t2 FOR VALUES FROM ('A') TO ('C');
+CREATE TABLE t3 (i int, t text);
+-- ERROR: relation "t1p1" is not a partition of relation "t2"
+ALTER TABLE t2 MERGE PARTITIONS (t1p1, t2pa) INTO t2p;
+ERROR: relation "t1p1" is not a partition of relation "t2"
+-- ERROR: "t3" is not a partition
+ALTER TABLE t2 MERGE PARTITIONS (t2pa, t3) INTO t2p;
+ERROR: "t3" is not a partition
+DROP TABLE t3;
+DROP TABLE t2;
+DROP TABLE t1;
+--
DROP SCHEMA partitions_merge_schema;
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index 675a1453c3..26a0d09969 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1414,4 +1414,17 @@ SELECT * FROM sales_others;
DROP TABLE sales_range;
--
+-- Try to SPLIT partition of another table.
+--
+CREATE TABLE t1(i int, t text) PARTITION BY LIST (t);
+CREATE TABLE t1pa PARTITION OF t1 FOR VALUES IN ('A');
+CREATE TABLE t2 (i int, t text) PARTITION BY RANGE (t);
+-- ERROR: relation "t1pa" is not a partition of relation "t2"
+ALTER TABLE t2 SPLIT PARTITION t1pa INTO
+ (PARTITION t2a FOR VALUES FROM ('A') TO ('B'),
+ PARTITION t2b FOR VALUES FROM ('B') TO ('C'));
+ERROR: relation "t1pa" is not a partition of relation "t2"
+DROP TABLE t2;
+DROP TABLE t1;
+--
DROP SCHEMA partition_split_schema;
diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql
index bb461e6623..9afed70365 100644
--- a/src/test/regress/sql/partition_merge.sql
+++ b/src/test/regress/sql/partition_merge.sql
@@ -28,7 +28,7 @@ CREATE TABLE sales_others PARTITION OF sales_range DEFAULT;
-- ERROR: partition with name "sales_feb2022" already used
ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022, sales_mar2022, sales_feb2022) INTO sales_feb_mar_apr2022;
--- ERROR: cannot merge non-table partition "sales_apr2022"
+-- ERROR: "sales_apr2022" is not a table
ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022, sales_mar2022, sales_apr2022) INTO sales_feb_mar_apr2022;
-- ERROR: invalid partitions order, partition "sales_mar2022" can not be merged
-- (space between sections sales_jan2022 and sales_mar2022)
@@ -350,9 +350,9 @@ CREATE TABLE sales_others2 PARTITION OF sales_list2 DEFAULT;
CREATE TABLE sales_external (LIKE sales_list);
CREATE TABLE sales_external2 (vch VARCHAR(5));
--- ERROR: partition bound for relation "sales_external" is null
+-- ERROR: "sales_external" is not a partition
ALTER TABLE sales_list MERGE PARTITIONS (sales_west, sales_east, sales_external) INTO sales_all;
--- ERROR: partition bound for relation "sales_external2" is null
+-- ERROR: "sales_external2" is not a partition
ALTER TABLE sales_list MERGE PARTITIONS (sales_west, sales_east, sales_external2) INTO sales_all;
-- ERROR: relation "sales_nord2" is not a partition of relation "sales_list"
ALTER TABLE sales_list MERGE PARTITIONS (sales_west, sales_nord2, sales_east) INTO sales_all;
@@ -426,5 +426,23 @@ RESET enable_seqscan;
DROP TABLE sales_list;
+--
+-- Try to MERGE partitions of another table.
+--
+CREATE TABLE t1 (i int, a int, b int, c int) PARTITION BY RANGE (a, b);
+CREATE TABLE t1p1 PARTITION OF t1 FOR VALUES FROM (1, 1) TO (1, 2);
+CREATE TABLE t2 (i int, t text) PARTITION BY RANGE (t);
+CREATE TABLE t2pa PARTITION OF t2 FOR VALUES FROM ('A') TO ('C');
+CREATE TABLE t3 (i int, t text);
+
+-- ERROR: relation "t1p1" is not a partition of relation "t2"
+ALTER TABLE t2 MERGE PARTITIONS (t1p1, t2pa) INTO t2p;
+-- ERROR: "t3" is not a partition
+ALTER TABLE t2 MERGE PARTITIONS (t2pa, t3) INTO t2p;
+
+DROP TABLE t3;
+DROP TABLE t2;
+DROP TABLE t1;
+
--
DROP SCHEMA partitions_merge_schema;
diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql
index 8864f6ddaa..625b01ddd1 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -829,5 +829,20 @@ SELECT * FROM sales_others;
DROP TABLE sales_range;
+--
+-- Try to SPLIT partition of another table.
+--
+CREATE TABLE t1(i int, t text) PARTITION BY LIST (t);
+CREATE TABLE t1pa PARTITION OF t1 FOR VALUES IN ('A');
+CREATE TABLE t2 (i int, t text) PARTITION BY RANGE (t);
+
+-- ERROR: relation "t1pa" is not a partition of relation "t2"
+ALTER TABLE t2 SPLIT PARTITION t1pa INTO
+ (PARTITION t2a FOR VALUES FROM ('A') TO ('B'),
+ PARTITION t2b FOR VALUES FROM ('B') TO ('C'));
+
+DROP TABLE t2;
+DROP TABLE t1;
+
--
DROP SCHEMA partition_split_schema;
--
2.40.1.windows.1
Attachments:
[text/plain] v1-0001-Fixes-for-ALTER-TABLE-.-SPLIT-MERGE-PARTITIONS-.-.patch (11.8K, ../../[email protected]/2-v1-0001-Fixes-for-ALTER-TABLE-.-SPLIT-MERGE-PARTITIONS-.-.patch)
download | inline diff:
From 578b3fae50baffa3626570447d55ce4177fc6e7d Mon Sep 17 00:00:00 2001
From: Koval Dmitry <[email protected]>
Date: Mon, 8 Apr 2024 23:10:52 +0300
Subject: [PATCH v1] Fixes for ALTER TABLE ... SPLIT/MERGE PARTITIONS ...
commands
---
doc/src/sgml/ddl.sgml | 2 +-
src/backend/commands/tablecmds.c | 12 ------
src/backend/parser/parse_utilcmd.c | 38 +++++++++++++++++++
src/test/regress/expected/partition_merge.out | 29 +++++++++++---
src/test/regress/expected/partition_split.out | 13 +++++++
src/test/regress/sql/partition_merge.sql | 24 ++++++++++--
src/test/regress/sql/partition_split.sql | 15 ++++++++
7 files changed, 111 insertions(+), 22 deletions(-)
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 8ff9a520ca..cd8304ef75 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -4410,7 +4410,7 @@ ALTER TABLE measurement
this operation is not supported for hash-partitioned tables and acquires
an <literal>ACCESS EXCLUSIVE</literal> lock, which could impact high-load
systems due to the lock's restrictive nature. For example, we can split
- the quarter partition back to monthly partitions:
+ the quarter partition back to monthly partitions:
<programlisting>
ALTER TABLE measurement SPLIT PARTITION measurement_y2006q1 INTO
(PARTITION measurement_y2006m01 FOR VALUES FROM ('2006-01-01') TO ('2006-02-01'),
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 865c6331c1..8a98a0af48 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -21223,12 +21223,6 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
*/
splitRel = table_openrv(cmd->name, AccessExclusiveLock);
- if (splitRel->rd_rel->relkind != RELKIND_RELATION)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot split non-table partition \"%s\"",
- RelationGetRelationName(splitRel))));
-
splitRelOid = RelationGetRelid(splitRel);
/* Check descriptions of new partitions. */
@@ -21463,12 +21457,6 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo *tab, Relation rel,
*/
mergingPartition = table_openrv(name, AccessExclusiveLock);
- if (mergingPartition->rd_rel->relkind != RELKIND_RELATION)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot merge non-table partition \"%s\"",
- RelationGetRelationName(mergingPartition))));
-
/*
* Checking that two partitions have the same name was before, in
* function transformPartitionCmdForMerge().
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 9e3e14087f..0d5ed0079c 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -32,6 +32,7 @@
#include "catalog/heap.h"
#include "catalog/index.h"
#include "catalog/namespace.h"
+#include "catalog/partition.h"
#include "catalog/pg_am.h"
#include "catalog/pg_collation.h"
#include "catalog/pg_constraint.h"
@@ -3415,6 +3416,38 @@ transformRuleStmt(RuleStmt *stmt, const char *queryString,
}
+/*
+ * checkPartition: check that partRelOid is partition of rel
+ */
+static void
+checkPartition(Relation rel, Oid partRelOid)
+{
+ Relation partRel;
+
+ partRel = relation_open(partRelOid, AccessShareLock);
+
+ if (partRel->rd_rel->relkind != RELKIND_RELATION)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is not a table",
+ RelationGetRelationName(partRel))));
+
+ if (!partRel->rd_rel->relispartition)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is not a partition",
+ RelationGetRelationName(partRel))));
+
+ if (get_partition_parent(partRelOid, false) != RelationGetRelid(rel))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_TABLE),
+ errmsg("relation \"%s\" is not a partition of relation \"%s\"",
+ RelationGetRelationName(partRel),
+ RelationGetRelationName(rel))));
+
+ relation_close(partRel, AccessShareLock);
+}
+
/*
* transformPartitionCmdForSplit
* Analyze the ALTER TABLLE ... SPLIT PARTITION command
@@ -3447,6 +3480,8 @@ transformPartitionCmdForSplit(CreateStmtContext *cxt, PartitionCmd *partcmd)
splitPartOid = RangeVarGetRelid(partcmd->name, NoLock, false);
+ checkPartition(parent, splitPartOid);
+
/* Then we should check partitions with transformed bounds. */
check_partitions_for_split(parent, splitPartOid, partcmd->name, partcmd->partlist, cxt->pstate);
}
@@ -3509,6 +3544,9 @@ transformPartitionCmdForMerge(CreateStmtContext *cxt, PartitionCmd *partcmd)
partOid = RangeVarGetRelid(name, NoLock, false);
if (partOid == defaultPartOid)
isDefaultPart = true;
+
+ checkPartition(parent, partOid);
+
partOids = lappend_oid(partOids, partOid);
}
diff --git a/src/test/regress/expected/partition_merge.out b/src/test/regress/expected/partition_merge.out
index 2ba0ec47d9..60eacf6bf3 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -25,9 +25,9 @@ ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022, sales_mar2022, sales_fe
ERROR: partition with name "sales_feb2022" already used
LINE 1: ...e MERGE PARTITIONS (sales_feb2022, sales_mar2022, sales_feb2...
^
--- ERROR: cannot merge non-table partition "sales_apr2022"
+-- ERROR: "sales_apr2022" is not a table
ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022, sales_mar2022, sales_apr2022) INTO sales_feb_mar_apr2022;
-ERROR: cannot merge non-table partition "sales_apr2022"
+ERROR: "sales_apr2022" is not a table
-- ERROR: invalid partitions order, partition "sales_mar2022" can not be merged
-- (space between sections sales_jan2022 and sales_mar2022)
ALTER TABLE sales_range MERGE PARTITIONS (sales_jan2022, sales_mar2022) INTO sales_jan_mar2022;
@@ -590,12 +590,12 @@ CREATE TABLE sales_nord2 PARTITION OF sales_list2 FOR VALUES IN ('Oslo', 'St. Pe
CREATE TABLE sales_others2 PARTITION OF sales_list2 DEFAULT;
CREATE TABLE sales_external (LIKE sales_list);
CREATE TABLE sales_external2 (vch VARCHAR(5));
--- ERROR: partition bound for relation "sales_external" is null
+-- ERROR: "sales_external" is not a partition
ALTER TABLE sales_list MERGE PARTITIONS (sales_west, sales_east, sales_external) INTO sales_all;
-ERROR: partition bound for relation "sales_external" is null
--- ERROR: partition bound for relation "sales_external2" is null
+ERROR: "sales_external" is not a partition
+-- ERROR: "sales_external2" is not a partition
ALTER TABLE sales_list MERGE PARTITIONS (sales_west, sales_east, sales_external2) INTO sales_all;
-ERROR: partition bound for relation "sales_external2" is null
+ERROR: "sales_external2" is not a partition
-- ERROR: relation "sales_nord2" is not a partition of relation "sales_list"
ALTER TABLE sales_list MERGE PARTITIONS (sales_west, sales_nord2, sales_east) INTO sales_all;
ERROR: relation "sales_nord2" is not a partition of relation "sales_list"
@@ -729,4 +729,21 @@ SELECT * FROM sales_list WHERE salesman_name = 'Ivanov';
RESET enable_seqscan;
DROP TABLE sales_list;
--
+-- Try to MERGE partitions of another table.
+--
+CREATE TABLE t1 (i int, a int, b int, c int) PARTITION BY RANGE (a, b);
+CREATE TABLE t1p1 PARTITION OF t1 FOR VALUES FROM (1, 1) TO (1, 2);
+CREATE TABLE t2 (i int, t text) PARTITION BY RANGE (t);
+CREATE TABLE t2pa PARTITION OF t2 FOR VALUES FROM ('A') TO ('C');
+CREATE TABLE t3 (i int, t text);
+-- ERROR: relation "t1p1" is not a partition of relation "t2"
+ALTER TABLE t2 MERGE PARTITIONS (t1p1, t2pa) INTO t2p;
+ERROR: relation "t1p1" is not a partition of relation "t2"
+-- ERROR: "t3" is not a partition
+ALTER TABLE t2 MERGE PARTITIONS (t2pa, t3) INTO t2p;
+ERROR: "t3" is not a partition
+DROP TABLE t3;
+DROP TABLE t2;
+DROP TABLE t1;
+--
DROP SCHEMA partitions_merge_schema;
diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out
index 675a1453c3..26a0d09969 100644
--- a/src/test/regress/expected/partition_split.out
+++ b/src/test/regress/expected/partition_split.out
@@ -1414,4 +1414,17 @@ SELECT * FROM sales_others;
DROP TABLE sales_range;
--
+-- Try to SPLIT partition of another table.
+--
+CREATE TABLE t1(i int, t text) PARTITION BY LIST (t);
+CREATE TABLE t1pa PARTITION OF t1 FOR VALUES IN ('A');
+CREATE TABLE t2 (i int, t text) PARTITION BY RANGE (t);
+-- ERROR: relation "t1pa" is not a partition of relation "t2"
+ALTER TABLE t2 SPLIT PARTITION t1pa INTO
+ (PARTITION t2a FOR VALUES FROM ('A') TO ('B'),
+ PARTITION t2b FOR VALUES FROM ('B') TO ('C'));
+ERROR: relation "t1pa" is not a partition of relation "t2"
+DROP TABLE t2;
+DROP TABLE t1;
+--
DROP SCHEMA partition_split_schema;
diff --git a/src/test/regress/sql/partition_merge.sql b/src/test/regress/sql/partition_merge.sql
index bb461e6623..9afed70365 100644
--- a/src/test/regress/sql/partition_merge.sql
+++ b/src/test/regress/sql/partition_merge.sql
@@ -28,7 +28,7 @@ CREATE TABLE sales_others PARTITION OF sales_range DEFAULT;
-- ERROR: partition with name "sales_feb2022" already used
ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022, sales_mar2022, sales_feb2022) INTO sales_feb_mar_apr2022;
--- ERROR: cannot merge non-table partition "sales_apr2022"
+-- ERROR: "sales_apr2022" is not a table
ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022, sales_mar2022, sales_apr2022) INTO sales_feb_mar_apr2022;
-- ERROR: invalid partitions order, partition "sales_mar2022" can not be merged
-- (space between sections sales_jan2022 and sales_mar2022)
@@ -350,9 +350,9 @@ CREATE TABLE sales_others2 PARTITION OF sales_list2 DEFAULT;
CREATE TABLE sales_external (LIKE sales_list);
CREATE TABLE sales_external2 (vch VARCHAR(5));
--- ERROR: partition bound for relation "sales_external" is null
+-- ERROR: "sales_external" is not a partition
ALTER TABLE sales_list MERGE PARTITIONS (sales_west, sales_east, sales_external) INTO sales_all;
--- ERROR: partition bound for relation "sales_external2" is null
+-- ERROR: "sales_external2" is not a partition
ALTER TABLE sales_list MERGE PARTITIONS (sales_west, sales_east, sales_external2) INTO sales_all;
-- ERROR: relation "sales_nord2" is not a partition of relation "sales_list"
ALTER TABLE sales_list MERGE PARTITIONS (sales_west, sales_nord2, sales_east) INTO sales_all;
@@ -426,5 +426,23 @@ RESET enable_seqscan;
DROP TABLE sales_list;
+--
+-- Try to MERGE partitions of another table.
+--
+CREATE TABLE t1 (i int, a int, b int, c int) PARTITION BY RANGE (a, b);
+CREATE TABLE t1p1 PARTITION OF t1 FOR VALUES FROM (1, 1) TO (1, 2);
+CREATE TABLE t2 (i int, t text) PARTITION BY RANGE (t);
+CREATE TABLE t2pa PARTITION OF t2 FOR VALUES FROM ('A') TO ('C');
+CREATE TABLE t3 (i int, t text);
+
+-- ERROR: relation "t1p1" is not a partition of relation "t2"
+ALTER TABLE t2 MERGE PARTITIONS (t1p1, t2pa) INTO t2p;
+-- ERROR: "t3" is not a partition
+ALTER TABLE t2 MERGE PARTITIONS (t2pa, t3) INTO t2p;
+
+DROP TABLE t3;
+DROP TABLE t2;
+DROP TABLE t1;
+
--
DROP SCHEMA partitions_merge_schema;
diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql
index 8864f6ddaa..625b01ddd1 100644
--- a/src/test/regress/sql/partition_split.sql
+++ b/src/test/regress/sql/partition_split.sql
@@ -829,5 +829,20 @@ SELECT * FROM sales_others;
DROP TABLE sales_range;
+--
+-- Try to SPLIT partition of another table.
+--
+CREATE TABLE t1(i int, t text) PARTITION BY LIST (t);
+CREATE TABLE t1pa PARTITION OF t1 FOR VALUES IN ('A');
+CREATE TABLE t2 (i int, t text) PARTITION BY RANGE (t);
+
+-- ERROR: relation "t1pa" is not a partition of relation "t2"
+ALTER TABLE t2 SPLIT PARTITION t1pa INTO
+ (PARTITION t2a FOR VALUES FROM ('A') TO ('B'),
+ PARTITION t2b FOR VALUES FROM ('B') TO ('C'));
+
+DROP TABLE t2;
+DROP TABLE t1;
+
--
DROP SCHEMA partition_split_schema;
--
2.40.1.windows.1
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-08-08 17:13 Noah Misch <[email protected]>
parent: Alexander Korotkov <[email protected]>
3 siblings, 2 replies; 20+ messages in thread
From: Noah Misch @ 2024-08-08 17:13 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Dmitry Koval <[email protected]>; [email protected]
On Sun, Apr 07, 2024 at 01:22:51AM +0300, Alexander Korotkov wrote:
> I've pushed 0001 and 0002
The partition MERGE (1adf16b8f) and SPLIT (87c21bb94) v17 patches introduced
createPartitionTable() with this code:
createStmt->relation = newPartName;
...
wrapper->utilityStmt = (Node *) createStmt;
...
ProcessUtility(wrapper,
...
newRel = table_openrv(newPartName, NoLock);
This breaks from the CVE-2014-0062 (commit 5f17304) principle of not repeating
name lookups. The attached demo uses this defect to make one partition have
two parents.
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index ae2efdc..654b502 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -3495,7 +3495,11 @@ StorePartitionBound(Relation rel, Relation parent, PartitionBoundSpec *bound)
elog(ERROR, "cache lookup failed for relation %u",
RelationGetRelid(rel));
-#ifdef USE_ASSERT_CHECKING
+ /*
+ * Assertion fails during partition getting multiple parents. Disable the
+ * assertion, to see what non-assert builds experience.
+ */
+#if 0
{
Form_pg_class classForm;
bool isnull;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8fcb188..48207f9 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -95,6 +95,7 @@
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
+#include "utils/injection_point.h"
#include "utils/inval.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
@@ -15825,7 +15826,11 @@ MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel, bool ispart
*/
if (parent_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
{
- Assert(child_att->attinhcount == 1);
+ /*
+ * Assertion fails during partition getting multiple parents.
+ * Disable, to see what non-assert builds experience.
+ */
+ /* Assert(child_att->attinhcount == 1); */
child_att->attislocal = false;
}
@@ -20388,7 +20393,14 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
* Open the new partition with no lock, because we already have
* AccessExclusiveLock placed there after creation.
*/
- newRel = table_openrv(newPartName, NoLock);
+ INJECTION_POINT("merge-after-create");
+ /*
+ * For testing, switch to taking a lock. This solves two problems.
+ * First, it gets an AcceptInvalidationMessages(), so we actually
+ * invalidate the search path. Second, it avoids an assertion failure
+ * from our lack of lock, so we see what non-assert builds experience.
+ */
+ newRel = table_openrv(newPartName, AccessExclusiveLock);
/*
* We intended to create the partition with the same persistence as the
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 2ffd2f7..9af45bf 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -9,7 +9,7 @@ PGFILEDESC = "injection_points - facility for injection points"
REGRESS = injection_points
REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
-ISOLATION = inplace
+ISOLATION = inplace merge
# The injection points are cluster-wide, so disable installcheck
NO_INSTALLCHECK = 1
diff --git a/src/test/modules/injection_points/expected/merge.out b/src/test/modules/injection_points/expected/merge.out
new file mode 100644
index 0000000..04920e7
--- /dev/null
+++ b/src/test/modules/injection_points/expected/merge.out
@@ -0,0 +1,60 @@
+Parsed test spec with 3 sessions
+
+starting permutation: merge1 ct2 detach3
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step merge1:
+ SET search_path = target;
+ ALTER TABLE parted MERGE PARTITIONS (part1, part3) INTO part_all;
+ -- only one of *parted should have rows, but both do
+ SELECT a AS new_target_parted FROM target.parted ORDER BY 1;
+ SELECT a AS old_target_parted FROM old_target.parted ORDER BY 1;
+ SELECT a AS new_target_part_all FROM target.part_all ORDER BY 1;
+ SELECT a AS old_target_part_all FROM old_target.part_all ORDER BY 1;
+ <waiting ...>
+step ct2:
+ ALTER SCHEMA target RENAME TO old_target;
+ CREATE SCHEMA target
+ CREATE TABLE parted (a int) partition by list (a)
+ CREATE TABLE part_all PARTITION OF parted FOR VALUES IN (1, 2, 3, 4)
+
+step detach3:
+ SELECT injection_points_detach('merge-after-create');
+ SELECT injection_points_wakeup('merge-after-create');
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step merge1: <... completed>
+new_target_parted
+-----------------
+ 1
+ 3
+(2 rows)
+
+old_target_parted
+-----------------
+ 1
+ 3
+(2 rows)
+
+new_target_part_all
+-------------------
+ 1
+ 3
+(2 rows)
+
+old_target_part_all
+-------------------
+(0 rows)
+
diff --git a/src/test/modules/injection_points/specs/merge.spec b/src/test/modules/injection_points/specs/merge.spec
new file mode 100644
index 0000000..63bcd2b
--- /dev/null
+++ b/src/test/modules/injection_points/specs/merge.spec
@@ -0,0 +1,51 @@
+setup
+{
+ CREATE EXTENSION injection_points;
+ CREATE SCHEMA target
+ CREATE TABLE parted (a int) partition by list (a)
+ CREATE TABLE part1 PARTITION OF parted FOR VALUES IN (1, 2)
+ CREATE TABLE part3 PARTITION OF parted FOR VALUES IN (3, 4);
+ INSERT INTO target.parted VALUES (1),(3);
+}
+teardown
+{
+ DROP SCHEMA target, old_target CASCADE;
+ DROP EXTENSION injection_points;
+}
+
+# MERGE PARTITIONS
+session s1
+setup {
+ SELECT injection_points_set_local();
+ SELECT injection_points_attach('merge-after-create', 'wait');
+}
+step merge1 {
+ SET search_path = target;
+ ALTER TABLE parted MERGE PARTITIONS (part1, part3) INTO part_all;
+ -- only one of *parted should have rows, but both do
+ SELECT a AS new_target_parted FROM target.parted ORDER BY 1;
+ SELECT a AS old_target_parted FROM old_target.parted ORDER BY 1;
+ SELECT a AS new_target_part_all FROM target.part_all ORDER BY 1;
+ SELECT a AS old_target_part_all FROM old_target.part_all ORDER BY 1;
+}
+
+
+# inject another table via ALTER SCHEMA RENAME
+session s2
+step ct2 {
+ ALTER SCHEMA target RENAME TO old_target;
+ CREATE SCHEMA target
+ CREATE TABLE parted (a int) partition by list (a)
+ CREATE TABLE part_all PARTITION OF parted FOR VALUES IN (1, 2, 3, 4)
+}
+
+
+# injection release
+session s3
+step detach3 {
+ SELECT injection_points_detach('merge-after-create');
+ SELECT injection_points_wakeup('merge-after-create');
+}
+
+
+permutation merge1(detach3) ct2 detach3
Attachments:
[text/plain] repro-merge-partition-race-v0.patch (6.0K, ../../[email protected]/2-repro-merge-partition-race-v0.patch)
download | inline diff:
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index ae2efdc..654b502 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -3495,7 +3495,11 @@ StorePartitionBound(Relation rel, Relation parent, PartitionBoundSpec *bound)
elog(ERROR, "cache lookup failed for relation %u",
RelationGetRelid(rel));
-#ifdef USE_ASSERT_CHECKING
+ /*
+ * Assertion fails during partition getting multiple parents. Disable the
+ * assertion, to see what non-assert builds experience.
+ */
+#if 0
{
Form_pg_class classForm;
bool isnull;
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8fcb188..48207f9 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -95,6 +95,7 @@
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
+#include "utils/injection_point.h"
#include "utils/inval.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
@@ -15825,7 +15826,11 @@ MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel, bool ispart
*/
if (parent_rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
{
- Assert(child_att->attinhcount == 1);
+ /*
+ * Assertion fails during partition getting multiple parents.
+ * Disable, to see what non-assert builds experience.
+ */
+ /* Assert(child_att->attinhcount == 1); */
child_att->attislocal = false;
}
@@ -20388,7 +20393,14 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
* Open the new partition with no lock, because we already have
* AccessExclusiveLock placed there after creation.
*/
- newRel = table_openrv(newPartName, NoLock);
+ INJECTION_POINT("merge-after-create");
+ /*
+ * For testing, switch to taking a lock. This solves two problems.
+ * First, it gets an AcceptInvalidationMessages(), so we actually
+ * invalidate the search path. Second, it avoids an assertion failure
+ * from our lack of lock, so we see what non-assert builds experience.
+ */
+ newRel = table_openrv(newPartName, AccessExclusiveLock);
/*
* We intended to create the partition with the same persistence as the
diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile
index 2ffd2f7..9af45bf 100644
--- a/src/test/modules/injection_points/Makefile
+++ b/src/test/modules/injection_points/Makefile
@@ -9,7 +9,7 @@ PGFILEDESC = "injection_points - facility for injection points"
REGRESS = injection_points
REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress
-ISOLATION = inplace
+ISOLATION = inplace merge
# The injection points are cluster-wide, so disable installcheck
NO_INSTALLCHECK = 1
diff --git a/src/test/modules/injection_points/expected/merge.out b/src/test/modules/injection_points/expected/merge.out
new file mode 100644
index 0000000..04920e7
--- /dev/null
+++ b/src/test/modules/injection_points/expected/merge.out
@@ -0,0 +1,60 @@
+Parsed test spec with 3 sessions
+
+starting permutation: merge1 ct2 detach3
+injection_points_attach
+-----------------------
+
+(1 row)
+
+step merge1:
+ SET search_path = target;
+ ALTER TABLE parted MERGE PARTITIONS (part1, part3) INTO part_all;
+ -- only one of *parted should have rows, but both do
+ SELECT a AS new_target_parted FROM target.parted ORDER BY 1;
+ SELECT a AS old_target_parted FROM old_target.parted ORDER BY 1;
+ SELECT a AS new_target_part_all FROM target.part_all ORDER BY 1;
+ SELECT a AS old_target_part_all FROM old_target.part_all ORDER BY 1;
+ <waiting ...>
+step ct2:
+ ALTER SCHEMA target RENAME TO old_target;
+ CREATE SCHEMA target
+ CREATE TABLE parted (a int) partition by list (a)
+ CREATE TABLE part_all PARTITION OF parted FOR VALUES IN (1, 2, 3, 4)
+
+step detach3:
+ SELECT injection_points_detach('merge-after-create');
+ SELECT injection_points_wakeup('merge-after-create');
+
+injection_points_detach
+-----------------------
+
+(1 row)
+
+injection_points_wakeup
+-----------------------
+
+(1 row)
+
+step merge1: <... completed>
+new_target_parted
+-----------------
+ 1
+ 3
+(2 rows)
+
+old_target_parted
+-----------------
+ 1
+ 3
+(2 rows)
+
+new_target_part_all
+-------------------
+ 1
+ 3
+(2 rows)
+
+old_target_part_all
+-------------------
+(0 rows)
+
diff --git a/src/test/modules/injection_points/specs/merge.spec b/src/test/modules/injection_points/specs/merge.spec
new file mode 100644
index 0000000..63bcd2b
--- /dev/null
+++ b/src/test/modules/injection_points/specs/merge.spec
@@ -0,0 +1,51 @@
+setup
+{
+ CREATE EXTENSION injection_points;
+ CREATE SCHEMA target
+ CREATE TABLE parted (a int) partition by list (a)
+ CREATE TABLE part1 PARTITION OF parted FOR VALUES IN (1, 2)
+ CREATE TABLE part3 PARTITION OF parted FOR VALUES IN (3, 4);
+ INSERT INTO target.parted VALUES (1),(3);
+}
+teardown
+{
+ DROP SCHEMA target, old_target CASCADE;
+ DROP EXTENSION injection_points;
+}
+
+# MERGE PARTITIONS
+session s1
+setup {
+ SELECT injection_points_set_local();
+ SELECT injection_points_attach('merge-after-create', 'wait');
+}
+step merge1 {
+ SET search_path = target;
+ ALTER TABLE parted MERGE PARTITIONS (part1, part3) INTO part_all;
+ -- only one of *parted should have rows, but both do
+ SELECT a AS new_target_parted FROM target.parted ORDER BY 1;
+ SELECT a AS old_target_parted FROM old_target.parted ORDER BY 1;
+ SELECT a AS new_target_part_all FROM target.part_all ORDER BY 1;
+ SELECT a AS old_target_part_all FROM old_target.part_all ORDER BY 1;
+}
+
+
+# inject another table via ALTER SCHEMA RENAME
+session s2
+step ct2 {
+ ALTER SCHEMA target RENAME TO old_target;
+ CREATE SCHEMA target
+ CREATE TABLE parted (a int) partition by list (a)
+ CREATE TABLE part_all PARTITION OF parted FOR VALUES IN (1, 2, 3, 4)
+}
+
+
+# injection release
+session s3
+step detach3 {
+ SELECT injection_points_detach('merge-after-create');
+ SELECT injection_points_wakeup('merge-after-create');
+}
+
+
+permutation merge1(detach3) ct2 detach3
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-08-08 22:43 Alexander Korotkov <[email protected]>
parent: Noah Misch <[email protected]>
1 sibling, 0 replies; 20+ messages in thread
From: Alexander Korotkov @ 2024-08-08 22:43 UTC (permalink / raw)
To: Noah Misch <[email protected]>; +Cc: Dmitry Koval <[email protected]>; [email protected]
On Thu, Aug 8, 2024 at 8:14 PM Noah Misch <[email protected]> wrote:
> On Sun, Apr 07, 2024 at 01:22:51AM +0300, Alexander Korotkov wrote:
> > I've pushed 0001 and 0002
>
> The partition MERGE (1adf16b8f) and SPLIT (87c21bb94) v17 patches introduced
> createPartitionTable() with this code:
>
> createStmt->relation = newPartName;
> ...
> wrapper->utilityStmt = (Node *) createStmt;
> ...
> ProcessUtility(wrapper,
> ...
> newRel = table_openrv(newPartName, NoLock);
>
> This breaks from the CVE-2014-0062 (commit 5f17304) principle of not repeating
> name lookups. The attached demo uses this defect to make one partition have
> two parents.
Thank you for a valuable report. I will dig into and fix that.
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-08-09 07:18 Dmitry Koval <[email protected]>
parent: Noah Misch <[email protected]>
1 sibling, 1 reply; 20+ messages in thread
From: Dmitry Koval @ 2024-08-09 07:18 UTC (permalink / raw)
To: Noah Misch <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: [email protected]
> This breaks from the CVE-2014-0062 (commit 5f17304) principle of not repeating
> name lookups. The attached demo uses this defect to make one partition have
> two parents.
Thank you very much for information (especially for the demo)!
I'm not sure that we can get the identifier of the newly created
partition from the ProcessUtility() function...
Maybe it would be enough to check that the new partition is located in
the namespace in which we created it (see attachment)?
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.com
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0b2a52463f..a1937d078b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20388,6 +20388,14 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
*/
newRel = table_openrv(newPartName, NoLock);
+ /* Check for case namespace was renamed during partition creation. */
+ if (RelationGetNamespace(newRel) != RelationGetNamespace(modelRel))
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot create partition because namespace \"%s\" was changed to \"%s\"",
+ get_namespace_name(RelationGetNamespace(newRel)),
+ get_namespace_name(RelationGetNamespace(modelRel)))));
+
/*
* We intended to create the partition with the same persistence as the
* parent table, but we still need to recheck because that might be
Attachments:
[text/plain] namespace-check.diff (896B, ../../[email protected]/2-namespace-check.diff)
download | inline diff:
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0b2a52463f..a1937d078b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20388,6 +20388,14 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
*/
newRel = table_openrv(newPartName, NoLock);
+ /* Check for case namespace was renamed during partition creation. */
+ if (RelationGetNamespace(newRel) != RelationGetNamespace(modelRel))
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot create partition because namespace \"%s\" was changed to \"%s\"",
+ get_namespace_name(RelationGetNamespace(newRel)),
+ get_namespace_name(RelationGetNamespace(modelRel)))));
+
/*
* We intended to create the partition with the same persistence as the
* parent table, but we still need to recheck because that might be
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-08-10 15:43 Alexander Korotkov <[email protected]>
parent: Dmitry Koval <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Alexander Korotkov @ 2024-08-10 15:43 UTC (permalink / raw)
To: Dmitry Koval <[email protected]>; +Cc: Noah Misch <[email protected]>; [email protected]
On Fri, Aug 9, 2024 at 10:18 AM Dmitry Koval <[email protected]> wrote:
> > This breaks from the CVE-2014-0062 (commit 5f17304) principle of not repeating
> > name lookups. The attached demo uses this defect to make one partition have
> > two parents.
>
> Thank you very much for information (especially for the demo)!
>
> I'm not sure that we can get the identifier of the newly created
> partition from the ProcessUtility() function...
> Maybe it would be enough to check that the new partition is located in
> the namespace in which we created it (see attachment)?
The new partition doesn't necessarily get created in the same
namespace as parent partition. I think it would be better to somehow
open partition by its oid.
It would be quite unfortunate to replicate significant part of
ProcessUtilitySlow(). So, the question is how to get the oid of newly
created relation from ProcessUtility(). I don't like to change the
signature of ProcessUtility() especially as a part of backpatch. So,
I tried to fit this into existing parameters. Probably
QueryCompletion struct fits this purpose best from the existing
parameters. Attached draft patch implements returning oid of newly
created relation as part of QueryCompletion. Thoughts?
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/x-patch] v1-0001-Fix-createPartitionTable-security-issue.patch (2.7K, ../../CAPpHfdtYgQ8PUZMS043c8USeKcKh5isrAW0QQKMAxQacX-+nuA@mail.gmail.com/2-v1-0001-Fix-createPartitionTable-security-issue.patch)
download | inline diff:
From 83b6d8e38d680daa952542f6ae4a41ae48491a62 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Sat, 10 Aug 2024 16:19:28 +0300
Subject: [PATCH v1] Fix createPartitionTable() security issue
---
src/backend/commands/tablecmds.c | 6 ++++--
src/backend/tcop/utility.c | 2 ++
src/include/tcop/cmdtag.h | 3 +++
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1f94f4fdbbc..c64b4953da8 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20329,6 +20329,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
TableLikeClause *tlc;
PlannedStmt *wrapper;
Relation newRel;
+ QueryCompletion qc;
/* If existing rel is temp, it must belong to this session */
if (modelRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP &&
@@ -20373,6 +20374,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
wrapper->stmt_location = context->pstmt->stmt_location;
wrapper->stmt_len = context->pstmt->stmt_len;
+ qc.commandTag = CMDTAG_UNKNOWN;
ProcessUtility(wrapper,
context->queryString,
false,
@@ -20380,13 +20382,13 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
NULL,
NULL,
None_Receiver,
- NULL);
+ &qc);
/*
* Open the new partition with no lock, because we already have
* AccessExclusiveLock placed there after creation.
*/
- newRel = table_openrv(newPartName, NoLock);
+ newRel = table_open(qc.tableAddress.objectId, NoLock);
/*
* We intended to create the partition with the same persistence as the
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index b2ea8125c92..e8a83e0c569 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1165,6 +1165,8 @@ ProcessUtilitySlow(ParseState *pstate,
RELKIND_RELATION,
InvalidOid, NULL,
queryString);
+ if (qc)
+ qc->tableAddress = address;
EventTriggerCollectSimpleCommand(address,
secondaryObject,
stmt);
diff --git a/src/include/tcop/cmdtag.h b/src/include/tcop/cmdtag.h
index 23c99d7eca6..a964a8c0cb7 100644
--- a/src/include/tcop/cmdtag.h
+++ b/src/include/tcop/cmdtag.h
@@ -13,6 +13,8 @@
#ifndef CMDTAG_H
#define CMDTAG_H
+#include "catalog/objectaddress.h"
+
/* buffer size required for command completion tags */
#define COMPLETION_TAG_BUFSIZE 64
@@ -29,6 +31,7 @@ typedef enum CommandTag
typedef struct QueryCompletion
{
CommandTag commandTag;
+ ObjectAddress tableAddress;
uint64 nprocessed;
} QueryCompletion;
--
2.39.3 (Apple Git-146)
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-08-10 15:57 Dmitry Koval <[email protected]>
parent: Alexander Korotkov <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Dmitry Koval @ 2024-08-10 15:57 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Noah Misch <[email protected]>; [email protected]
> Probably
> QueryCompletion struct fits this purpose best from the existing
> parameters. Attached draft patch implements returning oid of newly
> created relation as part of QueryCompletion. Thoughts?
I agree, returning the oid of the newly created relation is the best way
to solve the problem.
(Excuse me, I won't have access to a laptop for the next week - and
won't be able to look at the source code).
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.com
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-08-18 22:24 Alexander Korotkov <[email protected]>
parent: Dmitry Koval <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Alexander Korotkov @ 2024-08-18 22:24 UTC (permalink / raw)
To: Dmitry Koval <[email protected]>; +Cc: Noah Misch <[email protected]>; [email protected]
On Sat, Aug 10, 2024 at 6:57 PM Dmitry Koval <[email protected]> wrote:
> > Probably
> > QueryCompletion struct fits this purpose best from the existing
> > parameters. Attached draft patch implements returning oid of newly
> > created relation as part of QueryCompletion. Thoughts?
>
> I agree, returning the oid of the newly created relation is the best way
> to solve the problem.
> (Excuse me, I won't have access to a laptop for the next week - and
> won't be able to look at the source code).
Thank you for your feedback. Although, I decided QueryCompletion is
not a good place for this new field. It looks more appropriate to
place it to TableLikeClause, which already contains one relation oid
inside. The revised patch is attached.
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v2-0001-Avoid-repeated-table-name-lookups-in-createPartit.patch (2.7K, ../../CAPpHfdu+iX5TP65Cyn6vUe8JKGtLmMA=sKKKgE7tB6G6Qk=18Q@mail.gmail.com/2-v2-0001-Avoid-repeated-table-name-lookups-in-createPartit.patch)
download | inline diff:
From 448776815764803e5c25288b43011b3fcce76c99 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Sat, 10 Aug 2024 16:19:28 +0300
Subject: [PATCH v2] Avoid repeated table name lookups in
createPartitionTable()
Currently, createPartitionTable() opens newly created table using its name.
This approach is prone to privilege escalation attack, because we might end
up opening another table than we just created.
This commit address the issue above by opening newly created table by its
oid. It appears to be tricky to get a relation oid out of ProcessUtility().
We have to extend TableLikeClause with new newRelationOid field, which is
filled within ProcessUtility() to be further accessed by caller.
Security: CVE-2014-0062
Reported-by: Noah Misch
Discussion: https://postgr.es/m/20240808171351.a9.nmisch%40google.com
Reviewed-by: Dmitry Koval
---
src/backend/commands/tablecmds.c | 2 +-
src/backend/tcop/utility.c | 6 ++++++
src/include/nodes/parsenodes.h | 1 +
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7a36db6af6d..8719b4bf291 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20403,7 +20403,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
* Open the new partition with no lock, because we already have
* AccessExclusiveLock placed there after creation.
*/
- newRel = table_openrv(newPartName, NoLock);
+ newRel = table_open(tlc->newRelationOid, NoLock);
/*
* We intended to create the partition with the same persistence as the
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index b2ea8125c92..26a30bfdd56 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1225,6 +1225,12 @@ ProcessUtilitySlow(ParseState *pstate,
morestmts = expandTableLikeClause(table_rv, like);
stmts = list_concat(morestmts, stmts);
+
+ /*
+ * Put the Oid of newly created relation to the
+ * TableLikeClause, so caller might use it.
+ */
+ like->newRelationOid = address.objectId;
}
else
{
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 85a62b538e5..577c4bfef76 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -754,6 +754,7 @@ typedef struct TableLikeClause
RangeVar *relation;
bits32 options; /* OR of TableLikeOption flags */
Oid relationOid; /* If table has been looked up, its OID */
+ Oid newRelationOid; /* OID of newly created table */
} TableLikeClause;
typedef enum TableLikeOption
--
2.39.3 (Apple Git-146)
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-08-21 10:48 Pavel Borisov <[email protected]>
parent: Alexander Korotkov <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Pavel Borisov @ 2024-08-21 10:48 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Dmitry Koval <[email protected]>; Noah Misch <[email protected]>; [email protected]
Hi, Alexander!
On Mon, 19 Aug 2024 at 02:24, Alexander Korotkov <[email protected]>
wrote:
> On Sat, Aug 10, 2024 at 6:57 PM Dmitry Koval <[email protected]>
> wrote:
> > > Probably
> > > QueryCompletion struct fits this purpose best from the existing
> > > parameters. Attached draft patch implements returning oid of newly
> > > created relation as part of QueryCompletion. Thoughts?
> >
> > I agree, returning the oid of the newly created relation is the best way
> > to solve the problem.
> > (Excuse me, I won't have access to a laptop for the next week - and
> > won't be able to look at the source code).
>
> Thank you for your feedback. Although, I decided QueryCompletion is
> not a good place for this new field. It looks more appropriate to
> place it to TableLikeClause, which already contains one relation oid
> inside. The revised patch is attached.
>
I've looked at the patch v2. Remembering the OID of a relation newly
created with LIKE in TableLikeClause seems good to me.
Check-world passes sucessfully.
Shouldn't we also modify the TableLikeClause node in gram.y accordingly?
For the comments:
Put the Oid -> Store the OID
so caller might use it -> for the caller to use it.
(Maybe also caller -> table create function)
Regards,
Pavel Borisov
Supabase
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-08-21 11:55 Alexander Korotkov <[email protected]>
parent: Pavel Borisov <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Alexander Korotkov @ 2024-08-21 11:55 UTC (permalink / raw)
To: Pavel Borisov <[email protected]>; +Cc: Dmitry Koval <[email protected]>; Noah Misch <[email protected]>; [email protected]
Hi, Pavel!
On Wed, Aug 21, 2024 at 1:48 PM Pavel Borisov <[email protected]> wrote:
> On Mon, 19 Aug 2024 at 02:24, Alexander Korotkov <[email protected]> wrote:
>>
>> On Sat, Aug 10, 2024 at 6:57 PM Dmitry Koval <[email protected]> wrote:
>> > > Probably
>> > > QueryCompletion struct fits this purpose best from the existing
>> > > parameters. Attached draft patch implements returning oid of newly
>> > > created relation as part of QueryCompletion. Thoughts?
>> >
>> > I agree, returning the oid of the newly created relation is the best way
>> > to solve the problem.
>> > (Excuse me, I won't have access to a laptop for the next week - and
>> > won't be able to look at the source code).
>>
>> Thank you for your feedback. Although, I decided QueryCompletion is
>> not a good place for this new field. It looks more appropriate to
>> place it to TableLikeClause, which already contains one relation oid
>> inside. The revised patch is attached.
>
>
> I've looked at the patch v2. Remembering the OID of a relation newly created with LIKE in TableLikeClause seems good to me.
> Check-world passes sucessfully.
Thank you.
> Shouldn't we also modify the TableLikeClause node in gram.y accordingly?
On the one hand, makeNode() uses palloc0() and initializes all fields
with zero anyway. On the other hand, there is already assignment of
relationOid. So, yes I'll add assignment of newRelationOid for the
sake of uniformity.
> For the comments:
> Put the Oid -> Store the OID
> so caller might use it -> for the caller to use it.
Accepted.
> (Maybe also caller -> table create function)
I'll prefer to leave it "caller" as more generic term, which could
also fit potential future usages.
The revised patch is attached. I'm going to push it if no objections.
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v3-0001-Avoid-repeated-table-name-lookups-in-createPartit.patch (3.4K, ../../CAPpHfdsEtAWhpcPEaUky+gO20=rz0P03OskAS0c8teuTtHP9hQ@mail.gmail.com/2-v3-0001-Avoid-repeated-table-name-lookups-in-createPartit.patch)
download | inline diff:
From 32d2fb0a65663204e2d8f8efb392063afa174d85 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Sat, 10 Aug 2024 16:19:28 +0300
Subject: [PATCH v3] Avoid repeated table name lookups in
createPartitionTable()
Currently, createPartitionTable() opens newly created table using its name.
This approach is prone to privilege escalation attack, because we might end
up opening another table than we just created.
This commit address the issue above by opening newly created table by its
oid. It appears to be tricky to get a relation oid out of ProcessUtility().
We have to extend TableLikeClause with new newRelationOid field, which is
filled within ProcessUtility() to be further accessed by caller.
Security: CVE-2014-0062
Reported-by: Noah Misch
Discussion: https://postgr.es/m/20240808171351.a9.nmisch%40google.com
Reviewed-by: Dmitry Koval
---
src/backend/commands/tablecmds.c | 3 ++-
src/backend/parser/gram.y | 1 +
src/backend/tcop/utility.c | 6 ++++++
src/include/nodes/parsenodes.h | 1 +
4 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index dfba5f357b8..bcc2d9ae0b3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20381,6 +20381,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
tlc->options = CREATE_TABLE_LIKE_ALL &
~(CREATE_TABLE_LIKE_INDEXES | CREATE_TABLE_LIKE_IDENTITY | CREATE_TABLE_LIKE_STATISTICS);
tlc->relationOid = InvalidOid;
+ tlc->newRelationOid = InvalidOid;
createStmt->tableElts = lappend(createStmt->tableElts, tlc);
/* Need to make a wrapper PlannedStmt. */
@@ -20404,7 +20405,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
* Open the new partition with no lock, because we already have
* AccessExclusiveLock placed there after creation.
*/
- newRel = table_openrv(newPartName, NoLock);
+ newRel = table_open(tlc->newRelationOid, NoLock);
/*
* We intended to create the partition with the same persistence as the
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index c3f25582c38..b7d98eb9f02 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4138,6 +4138,7 @@ TableLikeClause:
n->relation = $2;
n->options = $3;
n->relationOid = InvalidOid;
+ n->newRelationOid = InvalidOid;
$$ = (Node *) n;
}
;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index b2ea8125c92..29868ed04c5 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1225,6 +1225,12 @@ ProcessUtilitySlow(ParseState *pstate,
morestmts = expandTableLikeClause(table_rv, like);
stmts = list_concat(morestmts, stmts);
+
+ /*
+ * Store the Oid of newly created relation to the
+ * TableLikeClause for the caller to use it.
+ */
+ like->newRelationOid = address.objectId;
}
else
{
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 85a62b538e5..577c4bfef76 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -754,6 +754,7 @@ typedef struct TableLikeClause
RangeVar *relation;
bits32 options; /* OR of TableLikeOption flags */
Oid relationOid; /* If table has been looked up, its OID */
+ Oid newRelationOid; /* OID of newly created table */
} TableLikeClause;
typedef enum TableLikeOption
--
2.39.3 (Apple Git-146)
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-08-21 12:06 Pavel Borisov <[email protected]>
parent: Alexander Korotkov <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Pavel Borisov @ 2024-08-21 12:06 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Dmitry Koval <[email protected]>; Noah Misch <[email protected]>; [email protected]
Hi, Alexander!
On Wed, 21 Aug 2024 at 15:55, Alexander Korotkov <[email protected]>
wrote:
> Hi, Pavel!
>
> On Wed, Aug 21, 2024 at 1:48 PM Pavel Borisov <[email protected]>
> wrote:
> > On Mon, 19 Aug 2024 at 02:24, Alexander Korotkov <[email protected]>
> wrote:
> >>
> >> On Sat, Aug 10, 2024 at 6:57 PM Dmitry Koval <[email protected]>
> wrote:
> >> > > Probably
> >> > > QueryCompletion struct fits this purpose best from the existing
> >> > > parameters. Attached draft patch implements returning oid of newly
> >> > > created relation as part of QueryCompletion. Thoughts?
> >> >
> >> > I agree, returning the oid of the newly created relation is the best
> way
> >> > to solve the problem.
> >> > (Excuse me, I won't have access to a laptop for the next week - and
> >> > won't be able to look at the source code).
> >>
> >> Thank you for your feedback. Although, I decided QueryCompletion is
> >> not a good place for this new field. It looks more appropriate to
> >> place it to TableLikeClause, which already contains one relation oid
> >> inside. The revised patch is attached.
> >
> >
> > I've looked at the patch v2. Remembering the OID of a relation newly
> created with LIKE in TableLikeClause seems good to me.
> > Check-world passes sucessfully.
>
> Thank you.
>
> > Shouldn't we also modify the TableLikeClause node in gram.y accordingly?
>
> On the one hand, makeNode() uses palloc0() and initializes all fields
> with zero anyway. On the other hand, there is already assignment of
> relationOid. So, yes I'll add assignment of newRelationOid for the
> sake of uniformity.
>
> > For the comments:
> > Put the Oid -> Store the OID
> so caller might use it -> for the caller to use it.
>
> Accepted.
>
> > (Maybe also caller -> table create function)
>
> I'll prefer to leave it "caller" as more generic term, which could
> also fit potential future usages.
>
> The revised patch is attached. I'm going to push it if no objections.
>
Looked at v3
All good except the patch has "Oid" and "OID" in two comments. I suppose
"OID" is preferred elsewhere in the PG comments.
Regards,
Pavel.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-08-21 12:40 Alexander Korotkov <[email protected]>
parent: Pavel Borisov <[email protected]>
0 siblings, 1 reply; 20+ messages in thread
From: Alexander Korotkov @ 2024-08-21 12:40 UTC (permalink / raw)
To: Pavel Borisov <[email protected]>; +Cc: Dmitry Koval <[email protected]>; Noah Misch <[email protected]>; [email protected]
On Wed, Aug 21, 2024 at 3:06 PM Pavel Borisov <[email protected]> wrote:
> On Wed, 21 Aug 2024 at 15:55, Alexander Korotkov <[email protected]> wrote:
>>
>> Hi, Pavel!
>>
>> On Wed, Aug 21, 2024 at 1:48 PM Pavel Borisov <[email protected]> wrote:
>> > On Mon, 19 Aug 2024 at 02:24, Alexander Korotkov <[email protected]> wrote:
>> >>
>> >> On Sat, Aug 10, 2024 at 6:57 PM Dmitry Koval <[email protected]> wrote:
>> >> > > Probably
>> >> > > QueryCompletion struct fits this purpose best from the existing
>> >> > > parameters. Attached draft patch implements returning oid of newly
>> >> > > created relation as part of QueryCompletion. Thoughts?
>> >> >
>> >> > I agree, returning the oid of the newly created relation is the best way
>> >> > to solve the problem.
>> >> > (Excuse me, I won't have access to a laptop for the next week - and
>> >> > won't be able to look at the source code).
>> >>
>> >> Thank you for your feedback. Although, I decided QueryCompletion is
>> >> not a good place for this new field. It looks more appropriate to
>> >> place it to TableLikeClause, which already contains one relation oid
>> >> inside. The revised patch is attached.
>> >
>> >
>> > I've looked at the patch v2. Remembering the OID of a relation newly created with LIKE in TableLikeClause seems good to me.
>> > Check-world passes sucessfully.
>>
>> Thank you.
>>
>> > Shouldn't we also modify the TableLikeClause node in gram.y accordingly?
>>
>> On the one hand, makeNode() uses palloc0() and initializes all fields
>> with zero anyway. On the other hand, there is already assignment of
>> relationOid. So, yes I'll add assignment of newRelationOid for the
>> sake of uniformity.
>>
>> > For the comments:
>> > Put the Oid -> Store the OID
>>
>> > so caller might use it -> for the caller to use it.
>>
>> Accepted.
>>
>> > (Maybe also caller -> table create function)
>>
>> I'll prefer to leave it "caller" as more generic term, which could
>> also fit potential future usages.
>>
>> The revised patch is attached. I'm going to push it if no objections.
>
> Looked at v3
> All good except the patch has "Oid" and "OID" in two comments. I suppose "OID" is preferred elsewhere in the PG comments.
Correct, the same file contains "OID" multiple times. Revised version
is attached.
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v4-0001-Avoid-repeated-table-name-lookups-in-createPartit.patch (3.4K, ../../CAPpHfdst=eqJ8-n=yhSk=5Xk2QHLF90HWryHWgARuityB3=b7A@mail.gmail.com/2-v4-0001-Avoid-repeated-table-name-lookups-in-createPartit.patch)
download | inline diff:
From cc3992a9262b126bc16a2e9f620c41aedcf99cba Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <[email protected]>
Date: Sat, 10 Aug 2024 16:19:28 +0300
Subject: [PATCH v4] Avoid repeated table name lookups in
createPartitionTable()
Currently, createPartitionTable() opens newly created table using its name.
This approach is prone to privilege escalation attack, because we might end
up opening another table than we just created.
This commit address the issue above by opening newly created table by its
oid. It appears to be tricky to get a relation oid out of ProcessUtility().
We have to extend TableLikeClause with new newRelationOid field, which is
filled within ProcessUtility() to be further accessed by caller.
Security: CVE-2014-0062
Reported-by: Noah Misch
Discussion: https://postgr.es/m/20240808171351.a9.nmisch%40google.com
Reviewed-by: Dmitry Koval
---
src/backend/commands/tablecmds.c | 3 ++-
src/backend/parser/gram.y | 1 +
src/backend/tcop/utility.c | 6 ++++++
src/include/nodes/parsenodes.h | 1 +
4 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index dfba5f357b8..bcc2d9ae0b3 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20381,6 +20381,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
tlc->options = CREATE_TABLE_LIKE_ALL &
~(CREATE_TABLE_LIKE_INDEXES | CREATE_TABLE_LIKE_IDENTITY | CREATE_TABLE_LIKE_STATISTICS);
tlc->relationOid = InvalidOid;
+ tlc->newRelationOid = InvalidOid;
createStmt->tableElts = lappend(createStmt->tableElts, tlc);
/* Need to make a wrapper PlannedStmt. */
@@ -20404,7 +20405,7 @@ createPartitionTable(RangeVar *newPartName, Relation modelRel,
* Open the new partition with no lock, because we already have
* AccessExclusiveLock placed there after creation.
*/
- newRel = table_openrv(newPartName, NoLock);
+ newRel = table_open(tlc->newRelationOid, NoLock);
/*
* We intended to create the partition with the same persistence as the
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index c3f25582c38..b7d98eb9f02 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -4138,6 +4138,7 @@ TableLikeClause:
n->relation = $2;
n->options = $3;
n->relationOid = InvalidOid;
+ n->newRelationOid = InvalidOid;
$$ = (Node *) n;
}
;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index b2ea8125c92..b385175e7a2 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1225,6 +1225,12 @@ ProcessUtilitySlow(ParseState *pstate,
morestmts = expandTableLikeClause(table_rv, like);
stmts = list_concat(morestmts, stmts);
+
+ /*
+ * Store the OID of newly created relation to the
+ * TableLikeClause for the caller to use it.
+ */
+ like->newRelationOid = address.objectId;
}
else
{
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 85a62b538e5..577c4bfef76 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -754,6 +754,7 @@ typedef struct TableLikeClause
RangeVar *relation;
bits32 options; /* OR of TableLikeOption flags */
Oid relationOid; /* If table has been looked up, its OID */
+ Oid newRelationOid; /* OID of newly created table */
} TableLikeClause;
typedef enum TableLikeOption
--
2.39.3 (Apple Git-146)
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-08-22 16:33 Robert Haas <[email protected]>
parent: Alexander Korotkov <[email protected]>
0 siblings, 2 replies; 20+ messages in thread
From: Robert Haas @ 2024-08-22 16:33 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Dmitry Koval <[email protected]>; Noah Misch <[email protected]>; [email protected]
Hi,
In response to some concerns raised about this fix on the
pgsql-release list today, I spent some time investigating this patch.
Unfortunately, I think there are too many problems here to be
reasonably fixed before release, and I think all of SPLIT/MERGE
PARTITION needs to be reverted.
I focused my investigation on createPartitionTable(), which is a
helper for both SPLIT PARTITION and MERGE PARTITION, and it works by
consing up a CREATE TABLE AS statement and then feeding that back
through
ProcessUtility. I think it's bad design to use such a high-level
facility here; it is unlike what we do elsewhere in tablecmds.c and
opens us up to a variety of problems. The first thing that I
discovered is that this patch does not fix all of the repeated name
lookup problems. There is still this:
tlc->relation =
makeRangeVar(get_namespace_name(RelationGetNamespace(modelRel)),
RelationGetRelationName(modelRel), -1);
And also this:
createStmt->tablespacename =
get_tablespace_name(modelRel->rd_rel->reltablespace);
In both cases, we do a reverse lookup on an OID to get a name which
the CREATE TABLE code will later turn back into an OID. If we don't
get the same value, that's at least a bug and probably a security
vulnerability, and there is no way to be certain that we will get the
same value. The only remedy is to not repeat the lookup in the first
place.
Then I got to looking at this:
tlc->options = CREATE_TABLE_LIKE_ALL &
~(CREATE_TABLE_LIKE_INDEXES | CREATE_TABLE_LIKE_IDENTITY |
CREATE_TABLE_LIKE_STATISTICS);
It's not obvious at first glance that there is a critical problem
here, but there are reasons to be nervous. We're deploying a lot of
machinery here to copy a lot of stuff and, while that's efficient from
a coding perspective, it means that stuff you might not expect can
just kind of happen. For instance:
robert.haas=# \d+
List of relations
Schema | Name | Type | Owner | Persistence |
Access method | Size | Description
--------+------+-------------------+-------------+-------------+---------------+------------+-------------
public | foo | partitioned table | robert.haas | permanent |
| 0 bytes |
public | foo1 | table | robert.haas | permanent | heap
| 8192 bytes |
public | foo2 | table | bob | permanent | heap
| 8192 bytes |
(3 rows)
robert.haas=# alter table foo split partition foo2 into (partition
foo3 for values from (10) to (15), partition foo4 for values from (15)
to (20));
ALTER TABLE
robert.haas=# \d+
List of relations
Schema | Name | Type | Owner | Persistence |
Access method | Size | Description
--------+------+-------------------+-------------+-------------+---------------+------------+-------------
public | foo | partitioned table | robert.haas | permanent |
| 0 bytes |
public | foo1 | table | robert.haas | permanent | heap
| 8192 bytes |
public | foo3 | table | robert.haas | permanent | heap
| 8192 bytes |
public | foo4 | table | robert.haas | permanent | heap
| 8192 bytes |
(4 rows)
I've split a partition owned by bob into two partitions owned by
robert.haas. That's rather surprising. It doesn't work to split a
partition that I don't own (and thus gain access to it) but if the
superuser splits a non-superuser's partition, the superuser ends
upowning the new partitions. I don't know if that's a vulnerability or
just unexpected. However, then I found this, which I'm pretty well
certain is a vulnerability:
robert.haas=# set role bob;
SET
robert.haas=> create table foo (a int, b text) partition by range (a);
CREATE TABLE
robert.haas=> create table foo1 partition of foo for values from (0) to (10);
CREATE TABLE
robert.haas=> create table foo2 partition of foo for values from (10) to (20);
CREATE TABLE
robert.haas=> insert into foo values (11, 'carrots'), (16, 'pineapple');
INSERT 0 2
robert.haas=> create or replace function run_me(integer) returns
integer as $$begin raise notice 'you are running me as %',
current_user; return $1; end$$ language plpgsql immutable;
CREATE FUNCTION
robert.haas=> create index on foo (run_me(a));
NOTICE: you are running me as bob
NOTICE: you are running me as bob
CREATE INDEX
robert.haas=> reset role;
RESET
robert.haas=# alter table foo split partition foo2 into (partition
foo3 for values from (10) to (15), partition foo4 for values from (15)
to (20));
NOTICE: you are running me as robert.haas
NOTICE: you are running me as robert.haas
ALTER TABLE
I think it is very unlikely that the problems mentioned above are the
only ones. They're just what I found in an hour or two of testing.
Even if they were, we're probably too close to release to be rushing
out last minute fixes to multiple unanticipated security problems. But
because of the design that was chosen here, I think there is probably
more stuff here that is not right, some of which is security relevant
and some of which is just a question of whether we're really getting
the behavior that we want. And I don't think we can fix all that
without either a very large number of grotty hacks similar to the one
installed by 04158e7fa37c2dda9c3421ca922d02807b86df19, or a complete
redesign of the feature. I believe the latter is probably a wiser
course of action.
...Robert
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-08-22 16:43 Jonathan S. Katz <[email protected]>
parent: Robert Haas <[email protected]>
1 sibling, 0 replies; 20+ messages in thread
From: Jonathan S. Katz @ 2024-08-22 16:43 UTC (permalink / raw)
To: Robert Haas <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Dmitry Koval <[email protected]>; Noah Misch <[email protected]>; [email protected]
On 8/22/24 12:33 PM, Robert Haas wrote:
> I think it is very unlikely that the problems mentioned above are the
> only ones. They're just what I found in an hour or two of testing.
> Even if they were, we're probably too close to release to be rushing
> out last minute fixes to multiple unanticipated security problems. But
> because of the design that was chosen here, I think there is probably
> more stuff here that is not right, some of which is security relevant
> and some of which is just a question of whether we're really getting
> the behavior that we want. And I don't think we can fix all that
> without either a very large number of grotty hacks similar to the one
> installed by 04158e7fa37c2dda9c3421ca922d02807b86df19, or a complete
> redesign of the feature. I believe the latter is probably a wiser
> course of action.
I can't comment on the design as much, but from a release standpoint,
but security concerns this close to the RC/GA period do concern me.
Applying the lessons from PG15 + SQL/JSON where we (and I'll own that I
was the one who pushed hard to include it) let it stay too long when it
should have been reverted, I think we should take more time to work on
this feature, revert it for PG17, and target it for PG18.
I understand it's disappointing to do a late revert of a feature, but I
think it's better to be safer, particularly if we believe there's a an
elevated risk of releasing something with vulnerabilities. As we saw
with SQL/JSON, this we'll give us more time to come up with design we
agree with, further test, and then promote as part of PG18.
Thanks,
Jonathan
Attachments:
[application/pgp-signature] OpenPGP_signature.asc (840B, ../../[email protected]/2-OpenPGP_signature.asc)
download
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
@ 2024-08-22 16:43 Alexander Korotkov <[email protected]>
parent: Robert Haas <[email protected]>
1 sibling, 0 replies; 20+ messages in thread
From: Alexander Korotkov @ 2024-08-22 16:43 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Pavel Borisov <[email protected]>; Dmitry Koval <[email protected]>; Noah Misch <[email protected]>; [email protected]
Hi!
On Thu, Aug 22, 2024 at 7:33 PM Robert Haas <[email protected]> wrote:
> In response to some concerns raised about this fix on the
> pgsql-release list today, I spent some time investigating this patch.
> Unfortunately, I think there are too many problems here to be
> reasonably fixed before release, and I think all of SPLIT/MERGE
> PARTITION needs to be reverted.
>
> I focused my investigation on createPartitionTable(), which is a
> helper for both SPLIT PARTITION and MERGE PARTITION, and it works by
> consing up a CREATE TABLE AS statement and then feeding that back
> through
> ProcessUtility. I think it's bad design to use such a high-level
> facility here; it is unlike what we do elsewhere in tablecmds.c and
> opens us up to a variety of problems. The first thing that I
> discovered is that this patch does not fix all of the repeated name
> lookup problems. There is still this:
>
> tlc->relation =
> makeRangeVar(get_namespace_name(RelationGetNamespace(modelRel)),
> RelationGetRelationName(modelRel), -1);
>
> And also this:
>
> createStmt->tablespacename =
> get_tablespace_name(modelRel->rd_rel->reltablespace);
>
> In both cases, we do a reverse lookup on an OID to get a name which
> the CREATE TABLE code will later turn back into an OID. If we don't
> get the same value, that's at least a bug and probably a security
> vulnerability, and there is no way to be certain that we will get the
> same value. The only remedy is to not repeat the lookup in the first
> place.
>
> Then I got to looking at this:
>
> tlc->options = CREATE_TABLE_LIKE_ALL &
> ~(CREATE_TABLE_LIKE_INDEXES | CREATE_TABLE_LIKE_IDENTITY |
> CREATE_TABLE_LIKE_STATISTICS);
>
> It's not obvious at first glance that there is a critical problem
> here, but there are reasons to be nervous. We're deploying a lot of
> machinery here to copy a lot of stuff and, while that's efficient from
> a coding perspective, it means that stuff you might not expect can
> just kind of happen. For instance:
>
> robert.haas=# \d+
> List of relations
> Schema | Name | Type | Owner | Persistence |
> Access method | Size | Description
> --------+------+-------------------+-------------+-------------+---------------+------------+-------------
> public | foo | partitioned table | robert.haas | permanent |
> | 0 bytes |
> public | foo1 | table | robert.haas | permanent | heap
> | 8192 bytes |
> public | foo2 | table | bob | permanent | heap
> | 8192 bytes |
> (3 rows)
> robert.haas=# alter table foo split partition foo2 into (partition
> foo3 for values from (10) to (15), partition foo4 for values from (15)
> to (20));
> ALTER TABLE
> robert.haas=# \d+
> List of relations
> Schema | Name | Type | Owner | Persistence |
> Access method | Size | Description
> --------+------+-------------------+-------------+-------------+---------------+------------+-------------
> public | foo | partitioned table | robert.haas | permanent |
> | 0 bytes |
> public | foo1 | table | robert.haas | permanent | heap
> | 8192 bytes |
> public | foo3 | table | robert.haas | permanent | heap
> | 8192 bytes |
> public | foo4 | table | robert.haas | permanent | heap
> | 8192 bytes |
> (4 rows)
>
> I've split a partition owned by bob into two partitions owned by
> robert.haas. That's rather surprising. It doesn't work to split a
> partition that I don't own (and thus gain access to it) but if the
> superuser splits a non-superuser's partition, the superuser ends
> upowning the new partitions. I don't know if that's a vulnerability or
> just unexpected. However, then I found this, which I'm pretty well
> certain is a vulnerability:
>
> robert.haas=# set role bob;
> SET
> robert.haas=> create table foo (a int, b text) partition by range (a);
> CREATE TABLE
> robert.haas=> create table foo1 partition of foo for values from (0) to (10);
> CREATE TABLE
> robert.haas=> create table foo2 partition of foo for values from (10) to (20);
> CREATE TABLE
> robert.haas=> insert into foo values (11, 'carrots'), (16, 'pineapple');
> INSERT 0 2
> robert.haas=> create or replace function run_me(integer) returns
> integer as $$begin raise notice 'you are running me as %',
> current_user; return $1; end$$ language plpgsql immutable;
> CREATE FUNCTION
> robert.haas=> create index on foo (run_me(a));
> NOTICE: you are running me as bob
> NOTICE: you are running me as bob
> CREATE INDEX
> robert.haas=> reset role;
> RESET
> robert.haas=# alter table foo split partition foo2 into (partition
> foo3 for values from (10) to (15), partition foo4 for values from (15)
> to (20));
> NOTICE: you are running me as robert.haas
> NOTICE: you are running me as robert.haas
> ALTER TABLE
>
> I think it is very unlikely that the problems mentioned above are the
> only ones. They're just what I found in an hour or two of testing.
> Even if they were, we're probably too close to release to be rushing
> out last minute fixes to multiple unanticipated security problems. But
> because of the design that was chosen here, I think there is probably
> more stuff here that is not right, some of which is security relevant
> and some of which is just a question of whether we're really getting
> the behavior that we want. And I don't think we can fix all that
> without either a very large number of grotty hacks similar to the one
> installed by 04158e7fa37c2dda9c3421ca922d02807b86df19, or a complete
> redesign of the feature. I believe the latter is probably a wiser
> course of action.
Thank you for your feedback. Yes, it seems that there is not enough
time to even carefully analyze all the issues in these features. The
rule of thumb I can get from this experience is "think multiple times
before accessing something already opened by its name". I'm going to
revert these features during next couple days.
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 20+ messages in thread
end of thread, other threads:[~2024-08-22 16:43 UTC | newest]
Thread overview: 20+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-01-31 15:13 [PATCH 1/3] fix CREATE INDEX progress report with nested partitions Ilya Gladyshev <[email protected]>
2024-04-06 22:22 Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
2024-04-06 22:38 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Dmitry Koval <[email protected]>
2024-04-07 19:00 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Lakhin <[email protected]>
2024-04-08 10:43 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Tender Wang <[email protected]>
2024-04-08 12:00 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Lakhin <[email protected]>
2024-04-08 20:43 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Dmitry Koval <[email protected]>
2024-08-08 17:13 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Noah Misch <[email protected]>
2024-08-08 22:43 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
2024-08-09 07:18 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Dmitry Koval <[email protected]>
2024-08-10 15:43 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
2024-08-10 15:57 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Dmitry Koval <[email protected]>
2024-08-18 22:24 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
2024-08-21 10:48 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
2024-08-21 11:55 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
2024-08-21 12:06 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Pavel Borisov <[email protected]>
2024-08-21 12:40 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
2024-08-22 16:33 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Robert Haas <[email protected]>
2024-08-22 16:43 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Jonathan S. Katz <[email protected]>
2024-08-22 16:43 ` Re: Add SPLIT PARTITION/MERGE PARTITIONS commands Alexander Korotkov <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox