Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Hi! 13.05.2024 11:45, Daniel Gustafsson пишет: Commit 3ca43dbbb67f which adds the permission checks seems to cause conflicts in the pg_upgrade tests Thanks! It will probably be enough to rename the roles: regress_partition_merge_alice -> regress_partition_split_alice regress_partition_merge_bob -> regress_partition_split_bob (changes in attachment) -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.comFrom f307add79397bcfe20f7c779e77630fb0df73020 Mon Sep 17 00:00:00 2001 From: Koval Dmitry Date: Mon, 13 May 2024 12:32:43 +0300 Subject: [PATCH v1] Rename roles to avoid conflicts in concurrent work --- src/test/regress/expected/partition_split.out | 20 +-- src/test/regress/sql/partition_split.sql | 20 +-- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out index b1108c92a2..999e923ef1 100644 --- a/src/test/regress/expected/partition_split.out +++ b/src/test/regress/expected/partition_split.out @@ -1516,33 +1516,33 @@ DROP TABLE t; DROP ACCESS METHOD partition_split_heap; -- Test permission checks. The user needs to own the parent table and the -- the partition to split to do the split. -CREATE ROLE regress_partition_merge_alice; -CREATE ROLE regress_partition_merge_bob; -SET SESSION AUTHORIZATION regress_partition_merge_alice; +CREATE ROLE regress_partition_split_alice; +CREATE ROLE regress_partition_split_bob; +SET SESSION AUTHORIZATION regress_partition_split_alice; CREATE TABLE t (i int) PARTITION BY RANGE (i); CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); -SET SESSION AUTHORIZATION regress_partition_merge_bob; +SET SESSION AUTHORIZATION regress_partition_split_bob; ALTER TABLE t SPLIT PARTITION tp_0_2 INTO (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); ERROR: must be owner of table t RESET SESSION AUTHORIZATION; -ALTER TABLE t OWNER TO regress_partition_merge_bob; -SET SESSION AUTHORIZATION regress_partition_merge_bob; +ALTER TABLE t OWNER TO regress_partition_split_bob; +SET SESSION AUTHORIZATION regress_partition_split_bob; ALTER TABLE t SPLIT PARTITION tp_0_2 INTO (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); ERROR: must be owner of table tp_0_2 RESET SESSION AUTHORIZATION; -ALTER TABLE tp_0_2 OWNER TO regress_partition_merge_bob; -SET SESSION AUTHORIZATION regress_partition_merge_bob; +ALTER TABLE tp_0_2 OWNER TO regress_partition_split_bob; +SET SESSION AUTHORIZATION regress_partition_split_bob; ALTER TABLE t SPLIT PARTITION tp_0_2 INTO (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); RESET SESSION AUTHORIZATION; DROP TABLE t; -DROP ROLE regress_partition_merge_alice; -DROP ROLE regress_partition_merge_bob; +DROP ROLE regress_partition_split_alice; +DROP ROLE regress_partition_split_bob; -- Check extended statistics aren't copied from the parent table to new -- partitions. CREATE TABLE t (i int, j int) PARTITION BY RANGE (i); diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql index 7f231b0d39..be4a785b75 100644 --- a/src/test/regress/sql/partition_split.sql +++ b/src/test/regress/sql/partition_split.sql @@ -896,36 +896,36 @@ DROP ACCESS METHOD partition_split_heap; -- Test permission checks. The user needs to own the parent table and the -- the partition to split to do the split. -CREATE ROLE regress_partition_merge_alice; -CREATE ROLE regress_partition_merge_bob; +CREATE ROLE regress_partition_split_alice; +CREATE ROLE regress_partition_split_bob; -SET SESSION AUTHORIZATION regress_partition_merge_alice; +SET SESSION AUTHORIZATION regress_partition_split_alice; CREATE TABLE t (i int) PARTITION BY RANGE (i); CREATE TABLE tp_0_2 PARTITION OF t FOR VALUES FROM (0) TO (2); -SET SESSION AUTHORIZATION regress_partition_merge_bob; +SET SESSION AUTHORIZATION regress_partition_split_bob; ALTER TABLE t SPLIT PARTITION tp_0_2 INTO (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); RESET SESSION AUTHORIZATION; -ALTER TABLE t OWNER TO regress_partition_merge_bob; -SET SESSION AUTHORIZATION regress_partition_merge_bob; +ALTER TABLE t OWNER TO regress_partition_split_bob; +SET SESSION AUTHORIZATION regress_partition_split_bob; ALTER TABLE t SPLIT PARTITION tp_0_2 INTO (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), PARTITION tp_1_2 FOR VALUES FROM (1) TO (2)); RESET SESSION AUTHORIZATION; -ALTER TABLE tp_0_2 OWNER TO regress_partition_merge_bob; -SET SESSION AUTHORIZATION regress_partition_merge_bob; +ALTER TABLE tp_0_2 OWNER TO regress_partition_split_bob; +SET SESSION AUTHORIZATION regress_partition_split_bob; ALTER TABLE t SPLIT PARTITION tp_0_2 INTO (PARTITION tp_0_1 FOR VALUES FROM (0) TO (1), PARTITION
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Hi! Attached draft version of fix for [1]. [1] https://www.postgresql.org/message-id/86b4f1e3-0b5d-315c-9225-19860d64d685%40gmail.com -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.comFrom ece01564aeb848bab2a61617412a1d175e45b934 Mon Sep 17 00:00:00 2001 From: Koval Dmitry Date: Sun, 12 May 2024 17:17:10 +0300 Subject: [PATCH v1 3/3] Fix for the search of temporary partition for the SPLIT SECTION operation --- src/backend/commands/tablecmds.c | 1 + src/test/regress/expected/partition_split.out | 8 src/test/regress/sql/partition_split.sql | 8 3 files changed, 17 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index fe66d9e58d..a5babcfbc6 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -21389,6 +21389,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo *tab, Relation rel, * against concurrent drop, and mark stmt->relation as * RELPERSISTENCE_TEMP if a temporary namespace is selected. */ + sps->name->relpersistence = rel->rd_rel->relpersistence; namespaceId = RangeVarGetAndCheckCreationNamespace(sps->name, NoLock, NULL); diff --git a/src/test/regress/expected/partition_split.out b/src/test/regress/expected/partition_split.out index 461318db86..b1108c92a2 100644 --- a/src/test/regress/expected/partition_split.out +++ b/src/test/regress/expected/partition_split.out @@ -1569,6 +1569,14 @@ Partition constraint: ((i IS NOT NULL) AND (i >= 0) AND (i < 1)) Partition of: t FOR VALUES FROM (1) TO (2) Partition constraint: ((i IS NOT NULL) AND (i >= 1) AND (i < 2)) +DROP TABLE t; +-- Check that the search for a temporary partition is correct during +-- the SPLIT PARTITION operation. +CREATE TEMP TABLE t (a int) PARTITION BY RANGE (a); +CREATE TEMP TABLE tp_0 PARTITION OF t FOR VALUES FROM (0) TO (2) ; +ALTER TABLE t SPLIT PARTITION tp_0 INTO + (PARTITION tp_0 FOR VALUES FROM (0) TO (1), + PARTITION tp_1 FOR VALUES FROM (1) TO (2)); DROP TABLE t; -- DROP SCHEMA partition_split_schema; diff --git a/src/test/regress/sql/partition_split.sql b/src/test/regress/sql/partition_split.sql index dc7424256e..7f231b0d39 100644 --- a/src/test/regress/sql/partition_split.sql +++ b/src/test/regress/sql/partition_split.sql @@ -939,6 +939,14 @@ ALTER TABLE t SPLIT PARTITION tp_0_2 INTO \d+ tp_1_2 DROP TABLE t; +-- Check that the search for a temporary partition is correct during +-- the SPLIT PARTITION operation. +CREATE TEMP TABLE t (a int) PARTITION BY RANGE (a); +CREATE TEMP TABLE tp_0 PARTITION OF t FOR VALUES FROM (0) TO (2) ; +ALTER TABLE t SPLIT PARTITION tp_0 INTO + (PARTITION tp_0 FOR VALUES FROM (0) TO (1), + PARTITION tp_1 FOR VALUES FROM (1) TO (2)); +DROP TABLE t; -- DROP SCHEMA partition_split_schema; -- 2.34.1
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Hi! 11.05.2024 12:00, Alexander Lakhin wrote: Please look at one more anomaly with temporary tables: Thank you, Alexander! The problem affects the SPLIT PARTITION command. CREATE TEMP TABLE t (a int) PARTITION BY RANGE (a); CREATE TEMP TABLE tp_0 PARTITION OF t FOR VALUES FROM (0) TO (2) ; -- ERROR: relation "tp_0" already exists ALTER TABLE t SPLIT PARTITION tp_0 INTO (PARTITION tp_0 FOR VALUES FROM (0) TO (1), PARTITION tp_1 FOR VALUES FROM (1) TO (2)); I'll try to fix it soon. -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Hi! 30.04.2024 23:15, Justin Pryzby пишет: Is this issue already fixed ? I wasn't able to reproduce it. Maybe it only happened with earlier patch versions applied ? I think this was fixed in commit [1]. [1] https://github.com/postgres/postgres/commit/fcf80c5d5f0f3787e70fca8fd029d2e08a923f91 -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Hi! 30.04.2024 6:00, Alexander Lakhin пишет: Maybe I'm doing something wrong, but the following script: CREATE TABLE t (i int, PRIMARY KEY(i)) PARTITION BY RANGE (i); CREATE TABLE tp_0 PARTITION OF t FOR VALUES FROM (0) TO (1); CREATE TABLE tp_1 PARTITION OF t FOR VALUES FROM (1) TO (2); CREATE TABLE t2 (LIKE t INCLUDING ALL); CREATE TABLE tp2 (LIKE tp_0 INCLUDING ALL); creates tables t2, tp2 without not-null constraints. To create partitions is used the "CREATE TABLE ... LIKE ..." command with the "EXCLUDING INDEXES" modifier (to speed up the insertion of values). CREATE TABLE t (i int, PRIMARY KEY(i)) PARTITION BY RANGE(i); CREATE TABLE t2 (LIKE t INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY); \d+ t2; ... Not-null constraints: "t2_i_not_null" NOT NULL "i" Access method: heap [1] https://github.com/postgres/postgres/blob/d12b4ba1bd3eedd862064cf1dad5ff107c5cba90/src/backend/commands/tablecmds.c#L21215 -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Hi! 1. 29.04.2024 21:00, Alexander Lakhin wrote: I still wonder, why that constraint (now with a less questionable name) is created during MERGE? The SPLIT/MERGE PARTITION(S) commands for creating partitions reuse the existing code of CREATE TABLE .. LIKE ... command. A new partition was created with the name "merge-16385-26BCB0-tmp" (since there was an old partition with the same name). The constraint "merge-16385-26BCB0-tmp_i_not_null" was created too together with the partition. Subsequently, the table was renamed, but the constraint was not. Now a new partition is immediately created with the correct name (the old partition is renamed). 2. Just in case, I am attaching a small fix v9_fix.diff for situation [1]. [1] https://www.postgresql.org/message-id/0520c72e-8d97-245e-53f9-173beca2ab2e%40gmail.com -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.comdiff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index fef084f5d5..e918a623c5 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -3446,6 +3446,11 @@ checkPartition(Relation rel, Oid partRelOid) RelationGetRelationName(partRel), RelationGetRelationName(rel; + /* Permissions checks */ + if (!object_ownercheck(RelationRelationId, RelationGetRelid(partRel), GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, get_relkind_objtype(partRel->rd_rel->relkind), + RelationGetRelationName(partRel)); + relation_close(partRel, AccessShareLock); }
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Hi!
18.04.2024 19:00, Alexander Lakhin wrote:
leaves a strange constraint:
\d+ t*
Table "public.tp_0"
...
Not-null constraints:
"merge-16385-26BCB0-tmp_i_not_null" NOT NULL "i"
Thanks!
Attached fix (with test) for this case.
The patch should be applied after patches
v6-0001- ... .patch ... v6-0004- ... .patch
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.comFrom 58e4b7fb1d3b15cdf1c742c28690392dda34915d Mon Sep 17 00:00:00 2001
From: Koval Dmitry
Date: Fri, 19 Apr 2024 01:57:49 +0300
Subject: [PATCH v6] Fix
---
src/backend/commands/tablecmds.c | 49 ++-
src/test/regress/expected/partition_merge.out | 13 -
src/test/regress/sql/partition_merge.sql | 8 ++-
3 files changed, 33 insertions(+), 37 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 72874295cb..8985747180 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -21508,9 +21508,6 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo
*tab, Relation rel,
ListCell *listptr;
List *mergingPartitionsList = NIL;
Oid defaultPartOid;
- chartmpRelName[NAMEDATALEN];
- RangeVar *mergePartName = cmd->name;
- boolisSameName = false;
/*
* Lock all merged partitions, check them and create list with
partitions
@@ -21532,8 +21529,22 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo
*tab, Relation rel,
* function transformPartitionCmdForMerge().
*/
if (equal(name, cmd->name))
+ {
/* One new partition can have the same name as merged
partition. */
- isSameName = true;
+ chartmpRelName[NAMEDATALEN];
+
+ /* Generate temporary name. */
+ sprintf(tmpRelName, "merge-%u-%X-tmp",
RelationGetRelid(rel), MyProcPid);
+
+ /* Rename partition. */
+
RenameRelationInternal(RelationGetRelid(mergingPartition),
+ tmpRelName,
false, false);
+ /*
+* We must bump the command counter to make the new
partition tuple
+* visible for rename.
+*/
+ CommandCounterIncrement();
+ }
/* Store a next merging partition into the list. */
mergingPartitionsList = lappend(mergingPartitionsList,
@@ -21553,16 +21564,8 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo
*tab, Relation rel,
DetachPartitionFinalize(rel, mergingPartition, false,
defaultPartOid);
}
- /* Create table for new partition, use partitioned table as model. */
- if (isSameName)
- {
- /* Create partition table with generated temporary name. */
- sprintf(tmpRelName, "merge-%u-%X-tmp", RelationGetRelid(rel),
MyProcPid);
- mergePartName = makeRangeVar(cmd->name->schemaname, tmpRelName,
-1);
- }
-
newPartRel = createPartitionTable(rel,
-
mergePartName,
+
cmd->name,
makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
RelationGetRelationName(rel), -1),
context);
@@ -21588,26 +21591,6 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo
*tab, Relation rel,
}
list_free(mergingPartitionsList);
- /* Rename new partition if it is needed. */
- if (isSameName)
- {
- /*
-* We must bump the command counter to make the new partition
tuple
-* visible for rename.
-*/
- CommandCounterIncrement();
-
- /* Rename partition. */
- RenameRelationInternal(RelationGetRelid(newPartRel),
- cmd->name->relname,
false, false);
-
- /*
-* Bump the command counter to make the tuple of renamed
partition
-* visible for attach partition operation.
-*/
- CommandCounterIncrement();
- }
-
/*
* Attach a new partition to the partitioned table. wqueue = NULL:
* verification for each cloned constraint is not ne
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Hi!
Please, find a my version of this fix attached.
Is it possible to make a small addition to the file v6-0001 ... .patch
(see attachment)?
Most important:
1) Line 19:
+ mergePartName = makeRangeVar(cmd->name->schemaname, tmpRelName, -1);
(temporary table should use the same schema as the partition);
2) Lines 116-123:
+RESET search_path;
+
+-- Can't merge persistent partitions into a temporary partition
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO pg_temp.tp_0_2;
+
+SET search_path = pg_temp, public;
(Alexandr Lakhin's test for using of pg_temp schema explicitly).
The rest of the changes in v6_afterfix.diff are not very important and
can be ignored.
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.comdiff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index c6ce7b94d9..bce5e39b64 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -21130,6 +21130,7 @@ moveSplitTableRows(Relation rel, Relation splitRel,
List *partlist, List *newPar
*
* Emulates command: CREATE [TEMP] TABLE (LIKE
* INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)
+ * Function locks created relation in AccessExclusiveLock mode and returns it.
*/
static Relation
createPartitionTable(Relation rel, RangeVar *newPartName, RangeVar
*modelRelName,
@@ -21500,8 +21501,7 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo
*tab, Relation rel,
{
/* Create partition table with generated temparary name. */
sprintf(tmpRelName, "merge-%u-%X-tmp", RelationGetRelid(rel),
MyProcPid);
- mergePartName =
makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
-
tmpRelName, -1);
+ mergePartName = makeRangeVar(cmd->name->schemaname, tmpRelName,
-1);
}
newPartRel = createPartitionTable(rel,
diff --git a/src/test/regress/expected/partition_merge.out
b/src/test/regress/expected/partition_merge.out
index b1d0b50b0b..0a4022f714 100644
--- a/src/test/regress/expected/partition_merge.out
+++ b/src/test/regress/expected/partition_merge.out
@@ -793,23 +793,58 @@ SELECT indexname FROM pg_indexes WHERE tablename =
'tp_1_2';
DROP TABLE t;
--
--- Try creating a partition in the temporary schema.
+-- Try mixing permanent and temporary partitions.
--
SET search_path = public, pg_temp;
CREATE TABLE t (i int) PARTITION BY RANGE (i);
CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
+SELECT c.oid::pg_catalog.regclass, c.relpersistence FROM pg_catalog.pg_class c
WHERE c.oid = 't'::regclass;
+ oid | relpersistence
+-+
+ t | p
+(1 row)
+
+SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound,
c.oid), c.relpersistence
+ FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
+ WHERE c.oid = i.inhrelid AND i.inhparent = 't'::regclass
+ ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT',
c.oid::pg_catalog.regclass::pg_catalog.text;
+ oid |pg_get_expr | relpersistence
+++
+ tp_0_1 | FOR VALUES FROM (0) TO (1) | p
+ tp_1_2 | FOR VALUES FROM (1) TO (2) | p
+(2 rows)
+
SET search_path = pg_temp, public;
-- Can't merge persistent partitions into a temporary partition
ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO tp_0_2;
ERROR: cannot create a temporary relation as partition of permanent relation
"t"
+RESET search_path;
+-- Can't merge persistent partitions into a temporary partition
+ALTER TABLE t MERGE PARTITIONS (tp_0_1, tp_1_2) INTO pg_temp.tp_0_2;
+ERROR: cannot create a temporary relation as partition of permanent relation
"t"
DROP TABLE t;
-DROP TAble tp_0_2;
-ERROR: table "tp_0_2" does not exist
+SET search_path = pg_temp, public;
BEGIN;
CREATE TABLE t (i int) PARTITION BY RANGE (i);
CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
+SELECT c.oid::pg_catalog.regclass, c.relpersistence FROM pg_catalog.pg_class c
WHERE c.oid = 't'::regclass;
+ oid | relpersistence
+-+
+ t | t
+(1 row)
+
+SELECT c.oid::pg_catalog.regclass, pg_catalog.pg_get_expr(c.relpartbound,
c.oid), c.relpersistence
+ FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
+ WHERE c.oid = i.inhrelid AND i.inhparent = 't'::regclass
+ ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT',
c.oid::pg_catalog.regclass::pg_catalog.text;
+ oid |pg_get_expr | relpersistence
+++
+ tp_0_1 | FOR VALUES FROM (0) TO (1) | t
+ tp_1_2 | FOR VALUES FROM (1) TO (2) | t
+(2 rows)
+
SET search_path = public, pg_temp;
-- Can't merge temporary partitions into a
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Thanks, Alexander!
Still now we're able to create a partition in the pg_temp schema
explicitly.
Attached patches with fix.
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.comFrom 2b68bbdb068e881e8ca6e34dec735f7ce656374f Mon Sep 17 00:00:00 2001
From: Koval Dmitry
Date: Wed, 10 Apr 2024 18:54:05 +0300
Subject: [PATCH v5 1/2] Fix for SPLIT/MERGE partitions of temporary table
---
src/backend/commands/tablecmds.c | 32 +++--
src/backend/parser/parse_utilcmd.c| 36 -
src/test/regress/expected/partition_merge.out | 124 +-
src/test/regress/expected/partition_split.out | 29
src/test/regress/sql/partition_merge.sql | 101 +-
src/test/regress/sql/partition_split.sql | 23
6 files changed, 324 insertions(+), 21 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8a98a0af48..b59e1dda03 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -21145,17 +21145,19 @@ moveSplitTableRows(Relation rel, Relation splitRel,
List *partlist, List *newPar
* createPartitionTable: create table for a new partition with given name
* (newPartName) like table (modelRelName)
*
- * Emulates command: CREATE TABLE (LIKE
+ * Emulates command: CREATE [TEMP] TABLE (LIKE
* INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)
*/
static void
-createPartitionTable(RangeVar *newPartName, RangeVar *modelRelName,
+createPartitionTable(Relation rel, RangeVar *newPartName, RangeVar
*modelRelName,
AlterTableUtilityContext *context)
{
CreateStmt *createStmt;
TableLikeClause *tlc;
PlannedStmt *wrapper;
+ newPartName->relpersistence = rel->rd_rel->relpersistence;
+
createStmt = makeNode(CreateStmt);
createStmt->relation = newPartName;
createStmt->tableElts = NIL;
@@ -21291,7 +21293,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo
*tab, Relation rel,
SinglePartitionSpec *sps = (SinglePartitionSpec *)
lfirst(listptr);
RelationnewPartRel;
- createPartitionTable(sps->name, parentName, context);
+ createPartitionTable(rel, sps->name, parentName, context);
/* Open the new partition and acquire exclusive lock on it. */
newPartRel = table_openrv(sps->name, AccessExclusiveLock);
@@ -21488,10 +21490,10 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo
*tab, Relation rel,
{
/* Create partition table with generated temparary name. */
sprintf(tmpRelName, "merge-%u-%X-tmp", RelationGetRelid(rel),
MyProcPid);
- mergePartName =
makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
-
tmpRelName, -1);
+ mergePartName = makeRangeVar(cmd->name->schemaname, tmpRelName,
-1);
}
- createPartitionTable(mergePartName,
+ createPartitionTable(rel,
+mergePartName,
makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
RelationGetRelationName(rel), -1),
context);
@@ -21507,12 +21509,6 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo
*tab, Relation rel,
/* Copy data from merged partitions to new partition. */
moveMergedTablesRows(rel, mergingPartitionsList, newPartRel);
- /*
-* Attach a new partition to the partitioned table. wqueue = NULL:
-* verification for each cloned constraint is not need.
-*/
- attachPartitionTable(NULL, rel, newPartRel, cmd->bound);
-
/* Unlock and drop merged partitions. */
foreach(listptr, mergingPartitionsList)
{
@@ -21542,7 +21538,19 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo
*tab, Relation rel,
/* Rename partition. */
RenameRelationInternal(RelationGetRelid(newPartRel),
cmd->name->relname,
false, false);
+ /*
+* Bump the command counter to make the tuple of renamed
partition
+* visible for attach partition operation.
+*/
+ CommandCounterIncrement();
}
+
+ /*
+* Attach a new partition to the partitioned table. wqueue = NULL:
+* verification for each cloned constraint is not needed.
+*/
+ attachPartitionTable(NULL, rel, newPartRel, cmd->bound);
+
/* Keep the lock until commit. */
table_close(newPartRel, NoLock);
}
diff --git a/src/backend/parser/parse_u
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Hi!
Attached is a patch with corrections based on comments in previous
letters (I think these corrections are not final).
I'll be very grateful for feedbacks and bug reports.
11.04.2024 20:00, Alexander Lakhin wrote:
> may be an attempt to merge into implicit
> pg_temp should fail just like CREATE TABLE ... PARTITION OF ... does?
Corrected. Result is:
\d+ s1.*
Table "s1.tp0"
...
Table "s1.tp1"
...
\d+ tp*
Did not find any relation named "tp*".
12.04.2024 4:53, Alexander Korotkov wrote:
> I think we shouldn't unconditionally copy schema name and
> relpersistence from the parent table. Instead we should throw the
> error on a mismatch like CREATE TABLE ... PARTITION OF ... does.
12.04.2024 5:20, Robert Haas wrote:
> We definitely shouldn't copy the schema name from the parent table.
Fixed.
12.04.2024 5:20, Robert Haas wrote:
> One of the things I dislike about this type of feature -- not this
> implementation specifically, but just this kind of idea in general --
> is that the syntax mentions a whole bunch of tables but in a way where
> you can't set their properties. Persistence, reloptions, whatever.
In next releases I want to allow specifying options (probably, first of
all, specifying tablespace of the partitions).
But before that, I would like to get a users reaction - what options
they really need?
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.comFrom 707d15cf9ee4673a1deed2825f48ffbc09a34e9d Mon Sep 17 00:00:00 2001
From: Koval Dmitry
Date: Wed, 10 Apr 2024 18:54:05 +0300
Subject: [PATCH v4 1/2] Fix for SPLIT/MERGE partitions of temporary table
---
src/backend/commands/tablecmds.c | 32 +++--
src/backend/parser/parse_utilcmd.c| 19 ++-
src/test/regress/expected/partition_merge.out | 118 +-
src/test/regress/expected/partition_split.out | 29 +
src/test/regress/sql/partition_merge.sql | 95 +-
src/test/regress/sql/partition_split.sql | 23
6 files changed, 297 insertions(+), 19 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8a98a0af48..b59e1dda03 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -21145,17 +21145,19 @@ moveSplitTableRows(Relation rel, Relation splitRel,
List *partlist, List *newPar
* createPartitionTable: create table for a new partition with given name
* (newPartName) like table (modelRelName)
*
- * Emulates command: CREATE TABLE (LIKE
+ * Emulates command: CREATE [TEMP] TABLE (LIKE
* INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)
*/
static void
-createPartitionTable(RangeVar *newPartName, RangeVar *modelRelName,
+createPartitionTable(Relation rel, RangeVar *newPartName, RangeVar
*modelRelName,
AlterTableUtilityContext *context)
{
CreateStmt *createStmt;
TableLikeClause *tlc;
PlannedStmt *wrapper;
+ newPartName->relpersistence = rel->rd_rel->relpersistence;
+
createStmt = makeNode(CreateStmt);
createStmt->relation = newPartName;
createStmt->tableElts = NIL;
@@ -21291,7 +21293,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo
*tab, Relation rel,
SinglePartitionSpec *sps = (SinglePartitionSpec *)
lfirst(listptr);
RelationnewPartRel;
- createPartitionTable(sps->name, parentName, context);
+ createPartitionTable(rel, sps->name, parentName, context);
/* Open the new partition and acquire exclusive lock on it. */
newPartRel = table_openrv(sps->name, AccessExclusiveLock);
@@ -21488,10 +21490,10 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo
*tab, Relation rel,
{
/* Create partition table with generated temparary name. */
sprintf(tmpRelName, "merge-%u-%X-tmp", RelationGetRelid(rel),
MyProcPid);
- mergePartName =
makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
-
tmpRelName, -1);
+ mergePartName = makeRangeVar(cmd->name->schemaname, tmpRelName,
-1);
}
- createPartitionTable(mergePartName,
+ createPartitionTable(rel,
+mergePartName,
makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
RelationGetRelationName(rel), -1),
context);
@@ -21507,12 +21509,6 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo
*tab, Relation rel,
/* Copy data from merged partitions to new partition. */
moveMergedTablesRows(rel, mergingPartitionsList,
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Hi!
1.
Alexander Lakhin sent a question about index name after MERGE (partition
name is the same as one of the merged partitions):
start of quote
I'm also confused by an index name after MERGE:
CREATE TABLE t (i int) PARTITION BY RANGE (i);
CREATE TABLE tp_0_1 PARTITION OF t FOR VALUES FROM (0) TO (1);
CREATE TABLE tp_1_2 PARTITION OF t FOR VALUES FROM (1) TO (2);
CREATE INDEX tidx ON t(i);
ALTER TABLE t MERGE PARTITIONS (tp_1_2, tp_0_1) INTO tp_1_2;
\d+ t*
Table "public.tp_1_2"
Column | Type | Collation | Nullable | Default | Storage |
Compression | Stats target | Description
+-+---+--+-+-+-+--+-
i | integer | | | | plain |
| |
Partition of: t FOR VALUES FROM (0) TO (2)
Partition constraint: ((i IS NOT NULL) AND (i >= 0) AND (i < 2))
Indexes:
"merge-16385-3A14B2-tmp_i_idx" btree (i)
Is the name "merge-16385-3A14B2-tmp_i_idx" valid or it's something
temporary?
end of quote
Fix for this case added to file
v3-0001-Fix-for-SPLIT-MERGE-partitions-of-temporary-table.patch.
2.
>It seems to me that v2-0001-Fix-for-SPLIT-MERGE-partitions-of-
>temporary-table.patch is not complete either.
Added correction (and test), see
v3-0001-Fix-for-SPLIT-MERGE-partitions-of-temporary-table.patch.
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.comFrom 58eb4abd4f065b6aa423bbddf62acd1799eba22e Mon Sep 17 00:00:00 2001
From: Koval Dmitry
Date: Wed, 10 Apr 2024 18:54:05 +0300
Subject: [PATCH v3 1/2] Fix for SPLIT/MERGE partitions of temporary table
---
src/backend/commands/tablecmds.c | 30 ---
src/backend/parser/parse_utilcmd.c| 2 +-
src/test/regress/expected/partition_merge.out | 87 ++-
src/test/regress/expected/partition_split.out | 29 +++
src/test/regress/sql/partition_merge.sql | 70 ++-
src/test/regress/sql/partition_split.sql | 23 +
6 files changed, 227 insertions(+), 14 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8a98a0af48..2769be55be 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -21145,17 +21145,20 @@ moveSplitTableRows(Relation rel, Relation splitRel,
List *partlist, List *newPar
* createPartitionTable: create table for a new partition with given name
* (newPartName) like table (modelRelName)
*
- * Emulates command: CREATE TABLE (LIKE
+ * Emulates command: CREATE [TEMP] TABLE (LIKE
* INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)
*/
static void
-createPartitionTable(RangeVar *newPartName, RangeVar *modelRelName,
+createPartitionTable(Relation rel, RangeVar *newPartName, RangeVar
*modelRelName,
AlterTableUtilityContext *context)
{
CreateStmt *createStmt;
TableLikeClause *tlc;
PlannedStmt *wrapper;
+ newPartName->relpersistence = rel->rd_rel->relpersistence;
+ newPartName->schemaname = modelRelName->schemaname;
+
createStmt = makeNode(CreateStmt);
createStmt->relation = newPartName;
createStmt->tableElts = NIL;
@@ -21291,7 +21294,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo
*tab, Relation rel,
SinglePartitionSpec *sps = (SinglePartitionSpec *)
lfirst(listptr);
RelationnewPartRel;
- createPartitionTable(sps->name, parentName, context);
+ createPartitionTable(rel, sps->name, parentName, context);
/* Open the new partition and acquire exclusive lock on it. */
newPartRel = table_openrv(sps->name, AccessExclusiveLock);
@@ -21491,7 +21494,8 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo
*tab, Relation rel,
mergePartName =
makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
tmpRelName, -1);
}
- createPartitionTable(mergePartName,
+ createPartitionTable(rel,
+mergePartName,
makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
RelationGetRelationName(rel), -1),
context);
@@ -21507,12 +21511,6 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo
*tab, Relation rel,
/* Copy data from merged partitions to new partition. */
moveMergedTablesRows(rel, mergingPartitionsList, newPartRel);
- /*
-* Attach a new partition to the partitioned table. wqueue = NULL:
-* verification for each cloned co
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Hi!
FWIW, I also proposed a patch earlier that fixes error messages and
comments in the split partition code
Sorry, I thought all the fixes you suggested were already included in
v1-0002-Fixes-for-english-text.patch (but they are not).
Added missing lines to v2-0002-Fixes-for-english-text.patch.
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.comFrom cfd5a56d15e7ead6dd7ae66cd382de3fa38150d7 Mon Sep 17 00:00:00 2001
From: Koval Dmitry
Date: Wed, 10 Apr 2024 18:54:05 +0300
Subject: [PATCH v2 1/2] Fix for SPLIT/MERGE partitions of temporary table
---
src/backend/commands/tablecmds.c | 11 ---
src/backend/parser/parse_utilcmd.c| 2 +-
src/test/regress/expected/partition_merge.out | 32 +--
src/test/regress/expected/partition_split.out | 29 +
src/test/regress/sql/partition_merge.sql | 24 +-
src/test/regress/sql/partition_split.sql | 23 +
6 files changed, 113 insertions(+), 8 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8a98a0af48..3da9f6389d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -21145,17 +21145,19 @@ moveSplitTableRows(Relation rel, Relation splitRel,
List *partlist, List *newPar
* createPartitionTable: create table for a new partition with given name
* (newPartName) like table (modelRelName)
*
- * Emulates command: CREATE TABLE (LIKE
+ * Emulates command: CREATE [TEMP] TABLE (LIKE
* INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)
*/
static void
-createPartitionTable(RangeVar *newPartName, RangeVar *modelRelName,
+createPartitionTable(Relation rel, RangeVar *newPartName, RangeVar
*modelRelName,
AlterTableUtilityContext *context)
{
CreateStmt *createStmt;
TableLikeClause *tlc;
PlannedStmt *wrapper;
+ newPartName->relpersistence = rel->rd_rel->relpersistence;
+
createStmt = makeNode(CreateStmt);
createStmt->relation = newPartName;
createStmt->tableElts = NIL;
@@ -21291,7 +21293,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo
*tab, Relation rel,
SinglePartitionSpec *sps = (SinglePartitionSpec *)
lfirst(listptr);
RelationnewPartRel;
- createPartitionTable(sps->name, parentName, context);
+ createPartitionTable(rel, sps->name, parentName, context);
/* Open the new partition and acquire exclusive lock on it. */
newPartRel = table_openrv(sps->name, AccessExclusiveLock);
@@ -21491,7 +21493,8 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo
*tab, Relation rel,
mergePartName =
makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
tmpRelName, -1);
}
- createPartitionTable(mergePartName,
+ createPartitionTable(rel,
+mergePartName,
makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
RelationGetRelationName(rel), -1),
context);
diff --git a/src/backend/parser/parse_utilcmd.c
b/src/backend/parser/parse_utilcmd.c
index ceba069905..a3bbcc99c0 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -3430,7 +3430,7 @@ checkPartition(Relation rel, Oid partRelOid)
if (partRel->rd_rel->relkind != RELKIND_RELATION)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-errmsg("\"%s\" is not a table",
+errmsg("\"%s\" is not an ordinary table",
RelationGetRelationName(partRel;
if (!partRel->rd_rel->relispartition)
diff --git a/src/test/regress/expected/partition_merge.out
b/src/test/regress/expected/partition_merge.out
index 60eacf6bf3..9fe97a801d 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: "sales_apr2022" is not a table
+-- ERROR: "sales_apr2022" is not an ordinary table
ALTER TABLE sales_range MERGE PARTITIONS (sales_feb2022, sales_mar2022,
sales_apr2022) INTO sales_feb_mar_apr2022;
-ERROR: "sales_apr2022&qu
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Hi!
Alexander Korotkov, thanks for the commit of previous fix.
Alexander Lakhin, thanks for the problem you found.
There are two corrections attached to the letter:
1) v1-0001-Fix-for-SPLIT-MERGE-partitions-of-temporary-table.patch - fix
for the problem [1].
2) v1-0002-Fixes-for-english-text.patch - fixes for English text
(comments, error messages etc.).
Links:
[1]
https://www.postgresql.org/message-id/dbc8b96c-3cf0-d1ee-860d-0e491da20485%40gmail.com
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.comFrom 3c443a57c334c74e9218fd4e2f1ced45e6d4141d Mon Sep 17 00:00:00 2001
From: Koval Dmitry
Date: Wed, 10 Apr 2024 18:54:05 +0300
Subject: [PATCH v1 1/2] Fix for SPLIT/MERGE partitions of temporary table
---
src/backend/commands/tablecmds.c | 11 ---
src/backend/parser/parse_utilcmd.c| 2 +-
src/test/regress/expected/partition_merge.out | 32 +--
src/test/regress/expected/partition_split.out | 29 +
src/test/regress/sql/partition_merge.sql | 24 +-
src/test/regress/sql/partition_split.sql | 23 +
6 files changed, 113 insertions(+), 8 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8a98a0af48..3da9f6389d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -21145,17 +21145,19 @@ moveSplitTableRows(Relation rel, Relation splitRel,
List *partlist, List *newPar
* createPartitionTable: create table for a new partition with given name
* (newPartName) like table (modelRelName)
*
- * Emulates command: CREATE TABLE (LIKE
+ * Emulates command: CREATE [TEMP] TABLE (LIKE
* INCLUDING ALL EXCLUDING INDEXES EXCLUDING IDENTITY)
*/
static void
-createPartitionTable(RangeVar *newPartName, RangeVar *modelRelName,
+createPartitionTable(Relation rel, RangeVar *newPartName, RangeVar
*modelRelName,
AlterTableUtilityContext *context)
{
CreateStmt *createStmt;
TableLikeClause *tlc;
PlannedStmt *wrapper;
+ newPartName->relpersistence = rel->rd_rel->relpersistence;
+
createStmt = makeNode(CreateStmt);
createStmt->relation = newPartName;
createStmt->tableElts = NIL;
@@ -21291,7 +21293,7 @@ ATExecSplitPartition(List **wqueue, AlteredTableInfo
*tab, Relation rel,
SinglePartitionSpec *sps = (SinglePartitionSpec *)
lfirst(listptr);
RelationnewPartRel;
- createPartitionTable(sps->name, parentName, context);
+ createPartitionTable(rel, sps->name, parentName, context);
/* Open the new partition and acquire exclusive lock on it. */
newPartRel = table_openrv(sps->name, AccessExclusiveLock);
@@ -21491,7 +21493,8 @@ ATExecMergePartitions(List **wqueue, AlteredTableInfo
*tab, Relation rel,
mergePartName =
makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
tmpRelName, -1);
}
- createPartitionTable(mergePartName,
+ createPartitionTable(rel,
+mergePartName,
makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
RelationGetRelationName(rel), -1),
context);
diff --git a/src/backend/parser/parse_utilcmd.c
b/src/backend/parser/parse_utilcmd.c
index ceba069905..ef1a2a97c0 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -3430,7 +3430,7 @@ checkPartition(Relation rel, Oid partRelOid)
if (partRel->rd_rel->relkind != RELKIND_RELATION)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-errmsg("\"%s\" is not a table",
+errmsg("\"%s\" is not a ordinary table",
RelationGetRelationName(partRel;
if (!partRel->rd_rel->relispartition)
diff --git a/src/test/regress/expected/partition_merge.out
b/src/test/regress/expected/partition_merge.out
index 60eacf6bf3..c69a717aaa 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: "sales_apr2022" is not a table
+-- ERROR: "sales_apr2022" is no
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
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.comFrom 578b3fae50baffa3626570447d55ce4177fc6e7d Mon Sep 17 00:00:00 2001
From: Koval Dmitry
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 ACCESS EXCLUSIVE 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:
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)
+{
+ RelationpartRel;
+
+ 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))
+
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Alexander Lakhin, thanks for the problems you found! Unfortunately I can't watch them immediately (event [1]). I will try to start solving them in 12-14 hours. [1] https://pgconf.ru/2024 -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
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
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Hi! > I've fixed that by skipping a copy of the identity of another > partition (remove CREATE_TABLE_LIKE_IDENTITY from > TableLikeClause.options). Thanks for correction! Probably I should have looked at the code more closely after commit [1]. I'm also very glad that situation [2] was reproduced. > When merging partitions you're creating a merged partition like the > parent table. But when splitting a partition you're creating new > partitions like the partition being split. What motivates this > difference? When splitting a partition, I planned to set parameters for each of the new partitions (for example, tablespace parameter). It would make sense if we want to transfer part of the data of splitting partition to a slower (archive) storage device. Right now I haven't seen any interest in this functionality, so it hasn't been implemented yet. But I think this will be needed in the future. Special thanks for the hint that new structures should be added to the list src\tools\pgindent\typedefs.list. Links. [1] https://github.com/postgres/postgres/commit/699586315704a8268808e3bdba4cb5924a038c49 [2] https://www.postgresql.org/message-id/171085360143.2046436.7217841141682511557.pgcf%40coridan.postgresql.org -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Hi! The following review has been posted through the commitfest application: make installcheck-world: tested, passed Thanks for info! I was unable to reproduce the problem and I wanted to ask for clarification. But your message was ahead of my question. -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: collect_corrupt_items_vacuum.patch
Hi! Thank you, there is one small point left (in the comment): can you replace "guarantteed to be to be newer" to "guaranteed to be newer", file src/backend/storage/ipc/procarray.c? -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: collect_corrupt_items_vacuum.patch
Hi! I agree with Alexander Lakhin about PROC_IN_VACUUM and VISHORIZON_DATA_STRICT: 1) probably manipulations with the PROC_IN_VACUUM flag in pg_visibility.c were needed for condition [1] and can be removed now; 2) the VISHORIZON_DATA_STRICT macro is probably unnecessary too (since we are not going to use it in the GlobalVisHorizonKindForRel() function). Also It would be nice to remove the get_strict_xid_horizon() function from the comment (replace to GetStrictOldestNonRemovableTransactionId()?). [1] https://github.com/postgres/postgres/blob/4d0cf0b05defcee985d5af38cb0db2b9c2f8dbae/src/backend/storage/ipc/procarray.c#L1812 -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: Operation log for major operations
Kirk, I'm sorry about the long pause in my reply. >We need some kind of semaphore flag that tells us something awkward >happened. When it happened, and a little bit of extra information. I agree that we do not have this kind of information. Additionally, legal events like start of pg_rewind, pg_reset, ... are interesting. >You also make the point that if such things have happened, it would >probably be a good idea to NOT allow pg_upgrade to run. >It might even be a reason to constantly bother someone until >the issue is repaired. I think no reason to forbid the run of pg_upgrade for the user (especially in automatic mode). If we automatically do NOT allow pg_upgrade, what should the user do for allow pg_upgrade? Unfortunately, PostgreSQL does not have the utilities to correct errors in the database (in case of errors users uses copies of the DB or corrects errors manually). An ordinary user cannot correct errors on his own ... So we cannot REQUIRE the user to correct database errors, we can only INFORM about them. >To that point, this feels like a "postgresql_panic.log" file (within >the postgresql files?)... Something that would prevent pg_upgrade, >etc. That everyone recognizes is serious. Especially 3rd party vendors. >I see the need for such a thing. I have to agree with others about >questioning the proper place to write this. >Are there other places that make sense, that you could use, especially >if knowing it exists means there was a serious issue? The location of the operation log (like a "postgresql_panic.log") is not easy question. Our technical support is sure that the large number of failures are caused by "human factor" (actions of database administrators). It is not difficult for database administrators to delete the "postgresql_panic.log" file or edit it (for example, replacing it with an old version; CRC will not save you from such an action). Therefore, our technical support decided to place the operation log at the end of the pg_control file, at an offset of 8192 bytes (and protect this log with CRC). About writing to the pg_control file what worries Tom Lane: information in pg_control is written once at system startup (twice in case of "promote"). Also, some utilities write information to the operation log too - pg_resetwal, pg_rewind, pg_upgrade (these utilities also modify the pg_control file without the operation log). If you are interested, I can attach the current patch (for info - I think it makes no sense to offer this patch at commitfest). -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: Operation log for major operations
I'll try to expand my explanation. I fully understand and accept the arguments about "limited sense to go into the control file" and "about recording *anything* in the control file". This is totally correct for vanilla. But vendors have forks of PostgreSQL with custom features and extensions. Sometimes (especially at the first releases) these custom components have bugs which can causes rare problems in data. These problems can migrate with using pg_upgrade and "lazy" upgrade of pages to higher versions of PostgreSQL fork. So in error cases "recording crash information" etc. is not the only important information. Very important is history of this database (pg_upgrades, promotions, pg_resets, pg_rewinds etc.). Often these "history" allows you to determine from which version of the PostgreSQL fork the error came from and what causes of errors we can discard immediately. This "history" is the information that our technical support wants (and reason of this patch), but this information is not needed for vanilla... Another important condition is that the user should not have easy ways to delete information about "history" (about reason to use pg_control file as "history" storage, but write into it from position 4kB, 8kB,...). -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: Operation log for major operations
Hi! These changes did not interest the community. It was expected (topic is very specifiс: vendor's technical support). So no sense to distract developers ... I'll move patch to Withdrawn. -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: Operation log for major operations
Hi! Maybe another discussion thread can be created for the consolidation of XX_file_exists functions. Usually XX_file_exists functions are simple. They contain single call stat() or open() and specific error processing after this call. Likely the unified function will be too complex to cover all the uses of XX_file_exists functions. -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: Operation log for major operations
Thanks, Ted Yu!
> Please update year for the license in pg_controllog.c
License updated for files pg_controllog.c, controllog_utils.c,
pg_controllog.h, controllog_utils.h.
> +check_file_exists(const char *datadir, const char *path)
> There is existing helper function such as:
> src/backend/utils/fmgr/dfmgr.c:static bool file_exists(const char *name);
> Is it possible to reuse that code ?
There are a lot of functions that check the file existence:
1) src/backend/utils/fmgr/dfmgr.c:static bool file_exists(const char *name);
2) src/backend/jit/jit.c:static bool file_exists(const char *name);
3) src/test/regress/pg_regress.c:bool file_exists(const char *file);
4) src/bin/pg_upgrade/exec.c:bool pid_lock_file_exists(const char *datadir);
5) src/backend/commands/extension.c:bool extension_file_exists(const
char *extensionName);
But there is no unified function: different components use their own
function with their own specific.
Probably we can not reuse code of dfmgr.c:file_exists() because this
function skip "errno == EACCES" (this error is critical for us).
I copied for src/bin/pg_rewind/file_ops.c:check_file_exists() code of
function jit.c:file_exists() (with adaptation for the utility).
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.comFrom aabff7fa8e51e760c558efa7b53fb675f996c7ff Mon Sep 17 00:00:00 2001
From: Koval Dmitry
Date: Mon, 14 Nov 2022 21:39:14 +0300
Subject: [PATCH v5] Operation log
---
src/backend/access/transam/xlog.c | 10 +
src/backend/backup/basebackup.c | 1 +
src/backend/storage/lmgr/lwlocknames.txt | 1 +
src/backend/utils/misc/Makefile | 1 +
src/backend/utils/misc/meson.build| 1 +
src/backend/utils/misc/pg_controllog.c| 142
src/bin/pg_checksums/pg_checksums.c | 1 +
src/bin/pg_resetwal/pg_resetwal.c | 4 +
src/bin/pg_rewind/file_ops.c | 20 +
src/bin/pg_rewind/file_ops.h | 2 +
src/bin/pg_rewind/pg_rewind.c | 59 ++
src/bin/pg_upgrade/controldata.c | 52 ++
src/bin/pg_upgrade/exec.c | 9 +-
src/bin/pg_upgrade/pg_upgrade.c | 2 +
src/bin/pg_upgrade/pg_upgrade.h | 2 +
src/common/Makefile | 1 +
src/common/controllog_utils.c | 667 ++
src/common/meson.build| 1 +
src/include/catalog/pg_controllog.h | 142
src/include/catalog/pg_proc.dat | 9 +
src/include/common/controllog_utils.h | 27 +
src/test/modules/test_misc/meson.build| 1 +
.../modules/test_misc/t/004_operation_log.pl | 136
src/tools/msvc/Mkvcbuild.pm | 4 +-
24 files changed, 1290 insertions(+), 5 deletions(-)
create mode 100644 src/backend/utils/misc/pg_controllog.c
create mode 100644 src/common/controllog_utils.c
create mode 100644 src/include/catalog/pg_controllog.h
create mode 100644 src/include/common/controllog_utils.h
create mode 100644 src/test/modules/test_misc/t/004_operation_log.pl
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 8f47fb7570..dd3c4c7ac4 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -68,6 +68,7 @@
#include "catalog/pg_control.h"
#include "catalog/pg_database.h"
#include "common/controldata_utils.h"
+#include "common/controllog_utils.h"
#include "common/file_utils.h"
#include "executor/instrument.h"
#include "miscadmin.h"
@@ -4775,6 +4776,9 @@ BootStrapXLOG(void)
/* some additional ControlFile fields are set in WriteControlFile() */
WriteControlFile();
+ /* Put information into operation log. */
+ put_operation_log_element(DataDir, OLT_BOOTSTRAP);
+
/* Bootstrap the commit log, too */
BootStrapCLOG();
BootStrapCommitTs();
@@ -5743,8 +5747,14 @@ StartupXLOG(void)
SpinLockRelease(>info_lck);
UpdateControlFile();
+
LWLockRelease(ControlFileLock);
+ /* Put information into operation log. */
+ if (promoted)
+ put_operation_log_element(DataDir, OLT_PROMOTED);
+ put_operation_log_element(DataDir, OLT_STARTUP);
+
/*
* Shutdown the recovery environment. This must occur after
* RecoverPreparedTransactions() (see notes in lock_twophase_recover())
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index 3fb9451643..0ca709b5b2 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -211,6 +211,7 @@ static const struct exclude_list_item excludeFiles[] =
*/
static const struct exclude_list_item noChecksumFiles[] = {
{"pg_control", false},
+ {"pg_control_log"
Re: Operation log for major operations
>The patch does not apply on top of HEAD ...
Thanks!
Here is a fixed version.
Additional changes:
1) get_operation_log() function doesn't create empty operation log file;
2) removed extra unlink() call.
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.comFrom d713a32499802395639645412a7c605870280f3a Mon Sep 17 00:00:00 2001
From: Koval Dmitry
Date: Mon, 14 Nov 2022 21:39:14 +0300
Subject: [PATCH v4] Operation log
---
src/backend/access/transam/xlog.c | 10 +
src/backend/backup/basebackup.c | 1 +
src/backend/storage/lmgr/lwlocknames.txt | 1 +
src/backend/utils/misc/Makefile | 1 +
src/backend/utils/misc/meson.build| 1 +
src/backend/utils/misc/pg_controllog.c| 142
src/bin/pg_checksums/pg_checksums.c | 1 +
src/bin/pg_resetwal/pg_resetwal.c | 4 +
src/bin/pg_rewind/file_ops.c | 28 +
src/bin/pg_rewind/file_ops.h | 2 +
src/bin/pg_rewind/pg_rewind.c | 59 ++
src/bin/pg_upgrade/controldata.c | 52 ++
src/bin/pg_upgrade/exec.c | 9 +-
src/bin/pg_upgrade/pg_upgrade.c | 2 +
src/bin/pg_upgrade/pg_upgrade.h | 2 +
src/common/Makefile | 1 +
src/common/controllog_utils.c | 667 ++
src/common/meson.build| 1 +
src/include/catalog/pg_controllog.h | 142
src/include/catalog/pg_proc.dat | 9 +
src/include/common/controllog_utils.h | 27 +
src/test/modules/test_misc/meson.build| 1 +
.../modules/test_misc/t/004_operation_log.pl | 136
src/tools/msvc/Mkvcbuild.pm | 4 +-
24 files changed, 1298 insertions(+), 5 deletions(-)
create mode 100644 src/backend/utils/misc/pg_controllog.c
create mode 100644 src/common/controllog_utils.c
create mode 100644 src/include/catalog/pg_controllog.h
create mode 100644 src/include/common/controllog_utils.h
create mode 100644 src/test/modules/test_misc/t/004_operation_log.pl
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 8f47fb7570..dd3c4c7ac4 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -68,6 +68,7 @@
#include "catalog/pg_control.h"
#include "catalog/pg_database.h"
#include "common/controldata_utils.h"
+#include "common/controllog_utils.h"
#include "common/file_utils.h"
#include "executor/instrument.h"
#include "miscadmin.h"
@@ -4775,6 +4776,9 @@ BootStrapXLOG(void)
/* some additional ControlFile fields are set in WriteControlFile() */
WriteControlFile();
+ /* Put information into operation log. */
+ put_operation_log_element(DataDir, OLT_BOOTSTRAP);
+
/* Bootstrap the commit log, too */
BootStrapCLOG();
BootStrapCommitTs();
@@ -5743,8 +5747,14 @@ StartupXLOG(void)
SpinLockRelease(>info_lck);
UpdateControlFile();
+
LWLockRelease(ControlFileLock);
+ /* Put information into operation log. */
+ if (promoted)
+ put_operation_log_element(DataDir, OLT_PROMOTED);
+ put_operation_log_element(DataDir, OLT_STARTUP);
+
/*
* Shutdown the recovery environment. This must occur after
* RecoverPreparedTransactions() (see notes in lock_twophase_recover())
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index 3fb9451643..0ca709b5b2 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -211,6 +211,7 @@ static const struct exclude_list_item excludeFiles[] =
*/
static const struct exclude_list_item noChecksumFiles[] = {
{"pg_control", false},
+ {"pg_control_log", false},
{"pg_filenode.map", false},
{"pg_internal.init", true},
{"PG_VERSION", false},
diff --git a/src/backend/storage/lmgr/lwlocknames.txt
b/src/backend/storage/lmgr/lwlocknames.txt
index 6c7cf6c295..5673de1669 100644
--- a/src/backend/storage/lmgr/lwlocknames.txt
+++ b/src/backend/storage/lmgr/lwlocknames.txt
@@ -53,3 +53,4 @@ XactTruncationLock44
# 45 was XactTruncationLock until removal of BackendRandomLock
WrapLimitsVacuumLock 46
NotifyQueueTailLock47
+ControlLogFileLock 48
diff --git a/src/backend/utils/misc/Makefile b/src/backend/utils/misc/Makefile
index b9ee4eb48a..3fa20e0368 100644
--- a/src/backend/utils/misc/Makefile
+++ b/src/backend/utils/misc/Makefile
@@ -23,6 +23,7 @@ OBJS = \
help_config.o \
pg_config.o \
pg_controldata.o \
+ pg_controllog.o \
pg_rusage.o \
ps_statu
Re: Operation log for major operations
Hi!
>The patch does not apply on top of HEAD ...
Here is a fixed version.
Small additional fixes:
1) added CRC calculation for empty 'pg_control_log' file;
2) added saving 'errno' before calling LWLockRelease and restoring after
that;
3) corrected pg_upgrade for case old cluster does not have
'pg_control_log' file.
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.comFrom 69faac5e9fc11d17d641a0cff3b26806c2930e3f Mon Sep 17 00:00:00 2001
From: Koval Dmitry
Date: Mon, 14 Nov 2022 21:39:14 +0300
Subject: [PATCH v3] Operation log
---
src/backend/access/transam/xlog.c | 10 +
src/backend/backup/basebackup.c | 1 +
src/backend/storage/lmgr/lwlocknames.txt | 1 +
src/backend/utils/misc/Makefile | 1 +
src/backend/utils/misc/meson.build| 1 +
src/backend/utils/misc/pg_controllog.c| 142
src/bin/pg_checksums/pg_checksums.c | 1 +
src/bin/pg_resetwal/pg_resetwal.c | 4 +
src/bin/pg_rewind/file_ops.c | 28 +
src/bin/pg_rewind/file_ops.h | 2 +
src/bin/pg_rewind/pg_rewind.c | 59 ++
src/bin/pg_upgrade/controldata.c | 67 ++
src/bin/pg_upgrade/exec.c | 9 +-
src/bin/pg_upgrade/pg_upgrade.c | 2 +
src/bin/pg_upgrade/pg_upgrade.h | 2 +
src/common/Makefile | 1 +
src/common/controllog_utils.c | 683 ++
src/common/meson.build| 1 +
src/include/catalog/pg_controllog.h | 142
src/include/catalog/pg_proc.dat | 9 +
src/include/common/controllog_utils.h | 27 +
src/test/modules/test_misc/meson.build| 1 +
.../modules/test_misc/t/004_operation_log.pl | 136
src/tools/msvc/Mkvcbuild.pm | 4 +-
24 files changed, 1329 insertions(+), 5 deletions(-)
create mode 100644 src/backend/utils/misc/pg_controllog.c
create mode 100644 src/common/controllog_utils.c
create mode 100644 src/include/catalog/pg_controllog.h
create mode 100644 src/include/common/controllog_utils.h
create mode 100644 src/test/modules/test_misc/t/004_operation_log.pl
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 0070d56b0b..6b82acb46b 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -68,6 +68,7 @@
#include "catalog/pg_control.h"
#include "catalog/pg_database.h"
#include "common/controldata_utils.h"
+#include "common/controllog_utils.h"
#include "common/file_utils.h"
#include "executor/instrument.h"
#include "miscadmin.h"
@@ -4774,6 +4775,9 @@ BootStrapXLOG(void)
/* some additional ControlFile fields are set in WriteControlFile() */
WriteControlFile();
+ /* Put information into operation log. */
+ put_operation_log_element(DataDir, OLT_BOOTSTRAP);
+
/* Bootstrap the commit log, too */
BootStrapCLOG();
BootStrapCommitTs();
@@ -5740,8 +5744,14 @@ StartupXLOG(void)
SpinLockRelease(>info_lck);
UpdateControlFile();
+
LWLockRelease(ControlFileLock);
+ /* Put information into operation log. */
+ if (promoted)
+ put_operation_log_element(DataDir, OLT_PROMOTED);
+ put_operation_log_element(DataDir, OLT_STARTUP);
+
/*
* Shutdown the recovery environment. This must occur after
* RecoverPreparedTransactions() (see notes in lock_twophase_recover())
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index 3fb9451643..0ca709b5b2 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -211,6 +211,7 @@ static const struct exclude_list_item excludeFiles[] =
*/
static const struct exclude_list_item noChecksumFiles[] = {
{"pg_control", false},
+ {"pg_control_log", false},
{"pg_filenode.map", false},
{"pg_internal.init", true},
{"PG_VERSION", false},
diff --git a/src/backend/storage/lmgr/lwlocknames.txt
b/src/backend/storage/lmgr/lwlocknames.txt
index 6c7cf6c295..5673de1669 100644
--- a/src/backend/storage/lmgr/lwlocknames.txt
+++ b/src/backend/storage/lmgr/lwlocknames.txt
@@ -53,3 +53,4 @@ XactTruncationLock44
# 45 was XactTruncationLock until removal of BackendRandomLock
WrapLimitsVacuumLock 46
NotifyQueueTailLock47
+ControlLogFileLock 48
diff --git a/src/backend/utils/misc/Makefile b/src/backend/utils/misc/Makefile
index b9ee4eb48a..3fa20e0368 100644
--- a/src/backend/utils/misc/Makefile
+++ b/src/backend/utils/misc/Makefile
@@ -23,6 +23,7 @@ OBJS = \
help_c
Re: Operation log for major operations
Hi!
>I think storing this in pg_control is a bad idea. That file is
>extremely critical and if you break it, you're pretty much SOL on
>recovering your data. I suggest that this should use a separate file.
Thanks. Operation log location changed to 'global/pg_control_log' (if
the name is not pretty, it can be changed).
I attached the patch (v2-0001-Operation-log.patch) and description of
operation log (Operation-log.txt).
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.com-
Introduction.
-
Operation log is designed to store information about critical system events
(like pg_upgrade, pg_resetwal, pg_resetwal, etc.).
This information is not interesting to the ordinary user, but it is very
important for the vendor's technical support.
An example: client complains about DB damage to technical support (damage was
caused by pg_resetwal which was "silent" executed by one of administrators).
Concepts.
-
* operation log is placed in the separate file 'global/pg_control_log' (log
size is 8kB);
* user can not modify the operation log; log can be changed by function call
only (from postgres or from postgres utilities);
* operation log is a ring buffer (with CRC-32 protection), deleting entries
from the operation log is possible only when the buffer is overflowed;
* SQL-function is used to read data of operation log.
Example of operation log data.
--
>select * from pg_operation_log();
event|edition|version| lsn | last |count
+---+---+-+--+--
startup|vanilla|10.22.0|0/828|2022-11-18 23:06:27+03|1
pg_upgrade |vanilla|15.0.0 |0/828|2022-11-18 23:06:27+03|1
startup|vanilla|15.0.0 |0/80001F8|2022-11-18 23:11:53+03|3
pg_resetwal|vanilla|15.0.0 |0/80001F8|2022-11-18 23:09:53+03|2
(4 rows)
Some details about inserting data to operation log.
---
There are two modes of inserting information about events in the operation log.
* MERGE mode (events "startup", "pg_resetwal", "pg_rewind").
We searches in ring buffer of operation log an event with the same type
("startup" for example) with the same version number.
If event was found, we will increment event counter by 1 and update the
date/time of event ("last" field) with the current value.
If event was not found, we will add this event to the ring buffer (see INSERT
mode).
* INSERT mode (events "bootstrap", "pg_upgrade", "promoted").
We will add an event to the ring buffer (without searching).
-
Operation log structures.
-
The operation log is placed in the pg_control_log file (file size
PG_OPERATION_LOG_FULL_SIZE=8192).
Operation log is a ring buffer of 341 elements (24 bytes each) with header (8
bytes).
а) Operation log element:
typedef struct OperationLogData
{
uint8 ol_type;/* operation type */
uint8 ol_edition; /* postgres edition */
uint16 ol_count; /* number of operations */
uint32 ol_version; /* postgres version */
pg_time_t ol_timestamp; /* = int64, operation date/time */
XLogRecPtr ol_lsn; /* = uint64, last check point
record ptr */
} OperationLogData;
Description of fields:
* ol_type - event type. The types are contained in an enum:
typedef enum
{
OLT_BOOTSTRAP = 1, /* bootstrap */
OLT_STARTUP,/* server startup */
OLT_RESETWAL, /* pg_resetwal */
OLT_REWIND, /* pg_rewind */
OLT_UPGRADE,/* pg_upgrade */
OLT_PROMOTED, /* promoted */
OLT_NumberOfTypes /* should be last */
} ol_type_enum;
* ol_edition - PostgreSQL edition (this field is important for custom
PostgreSQL editions).
The editions are contained in an enum:
typedef enum
{
ED_PG_ORIGINAL = 0
/* Here can be custom editions */
} PgNumEdition;
* ol_count - the number of fired events in case events of this type can be
merged (otherwise 1). Maximum is 65536;
* ol_version - version formed with using PG_VERSION_NUM (PG_VERSION_NUM*100;
for example: 13000800 for v13.8). Two last digits can be used for patch version;
* ol_timestamp - date/time of the last fired event in case events of this type
can be merged (otherwise - the date/time of the event);
* ol_lsn - "Latest checkpoint location" value (ControlFile->checkPoint) which
contains in pg_control file at the time of the event.
б) Operation l
Re: Operation log for major operations
Thanks for references, Justin! Couple comments about these references. 1) "Make unlogged table resets detectable". https://www.postgresql.org/message-id/flat/62750df5b126e1d8ee039a79ef3cc64ac3d47cd5.camel%40j-davis.com This conversation is about specific problem (unlogged table repairing). Operation log has another use - it is primary a helper for diagnostics. 2) "RFC: Add 'taint' field to pg_control." https://www.postgresql.org/message-id/flat/20180228214311.jclah37cwh572t2c%40alap3.anarazel.de This is almost the same problem that we want to solve with operation log. Differences between the operation log and what is discussed in the thread: * there suggested to store operation log in pg_control file - but separate from pg_control main data (and write data separately too); * operation log data can be represented in relational form (not flags), this is more usable for RDBMS; * number of registered event types can be increased easy (without changes of storage). -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Operation log for major operations
Hi, hackers!
It is important for customer support to know what system operations
(pg_resetwal, pg_rewind, pg_upgrade, ...) have been executed on the
database. A variant of implementation of the log for system operations
(operation log) is attached to this email.
Introduction.
-
Operation log is designed to store information about critical system
events (like pg_upgrade, pg_resetwal, pg_resetwal, etc.).
This information is not interesting to the ordinary user, but it is very
important for the vendor's technical support.
An example: client complains about DB damage to technical support
(damage was caused by pg_resetwal which was "silent" executed by one of
administrators).
Concepts.
-
* operation log is placed in the file 'global/pg_control', starting from
position 4097 (log size is 4kB);
* user can not modify the operation log; log can be changed by
function call only (from postgres or from postgres utilities);
* operation log is a ring buffer (with CRC-32 protection), deleting
entries from the operation log is possible only when the buffer is
overflowed;
* SQL-function is used to read data of operation log.
Example of operation log data.
--
>select * from pg_operation_log();
event|edition|version| lsn | last |count
+---+---+-+--+--
startup|vanilla|10.22.0|0/828|2022-11-18 23:06:27+03|1
pg_upgrade |vanilla|15.0.0 |0/828|2022-11-18 23:06:27+03|1
startup|vanilla|15.0.0 |0/80001F8|2022-11-18 23:11:53+03|3
pg_resetwal|vanilla|15.0.0 |0/80001F8|2022-11-18 23:09:53+03|2
(4 rows)
Some details about inserting data to operation log.
---
There are two modes of inserting information about events in the
operation log.
* MERGE mode (events "startup", "pg_resetwal", "pg_rewind").
We searches in ring buffer of operation log an event with the same type
("startup" for example) with the same version number.
If event was found, we will increment event counter by 1 and update the
date/time of event ("last" field) with the current value.
If event was not found, we will add this event to the ring buffer (see
INSERT mode).
* INSERT mode (events "bootstrap", "pg_upgrade", "promoted").
We will add an event to the ring buffer (without searching).
P.S. File 'global/pg_control' was chosen as operation log storage
because data of this file cannot be removed or modified in a simple way
and no need to change any extensions and utilities to support this file.
I attached the patch (v1-0001-Operation-log.patch) and extended
description of operation log (Operation-log.txt).
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.com-
Operation log structures.
-
The operation log is placed in the pg_control file (file size
PG_CONTROL_FILE_SIZE=8192).
Operation log size is PG_OPERATION_LOG_FULL_SIZE=4096.
Operation log is a ring buffer of 170 elements (24 bytes each) with header (16
bytes).
а) Operation log element:
typedef struct OperationLogData
{
uint8 ol_type;/* operation type */
uint8 ol_edition; /* postgres edition */
uint16 ol_count; /* number of operations */
uint32 ol_version; /* postgres version */
pg_time_t ol_timestamp; /* = int64, operation date/time */
XLogRecPtr ol_lsn; /* = uint64, last check point
record ptr */
} OperationLogData;
Description of fields:
* ol_type - event type. The types are contained in an enum:
typedef enum
{
OLT_BOOTSTRAP = 1, /* bootstrap */
OLT_STARTUP,/* server startup */
OLT_RESETWAL, /* pg_resetwal */
OLT_REWIND, /* pg_rewind */
OLT_UPGRADE,/* pg_upgrade */
OLT_PROMOTED, /* promoted */
OLT_NumberOfTypes /* should be last */
} ol_type_enum;
* ol_edition - PostgreSQL edition (this field is important for custom
PostgreSQL editions).
The editions are contained in an enum:
typedef enum
{
ED_PG_ORIGINAL = 0
/* Here can be custom editions */
} PgNumEdition;
* ol_count - the number of fired events in case events of this type can be
merged (otherwise 1). Maximum is 65536;
* ol_version - version formed with using PG_VERSION_NUM (PG_VERSION_NUM*100;
for example: 13000800 for v13.8). Two last digits can be used for patch version;
* ol_timestamp - date/time of the last fired event in case events of this type
c
Re: Frontend error logging style
Hi! Commit 9a374b77fb53 (Improve frontend error logging style.) replaced a line of file src/include/common/logging.h ``` extern PGDLLIMPORT enum pg_log_level __pg_log_level; ``` to ``` extern enum pg_log_level __pg_log_level; ``` i.e. it is rollback of commit 8ec569479fc28 (Apply PGDLLIMPORT markings broadly.) Is it correct? -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Thanks for comments and advice! I thought about this problem and discussed about it with colleagues. Unfortunately, I don't know of a good general solution. 19.09.2022 22:56, Robert Haas пишет: If you know that a certain partition is not changing, and you would like to split it, you can create two or more new standalone tables and populate them from the original partition using INSERT .. SELECT. Then you can BEGIN a transaction, DETACH the existing partitions, and ATTACH the replacement ones. By doing this, you take an ACCESS EXCLUSIVE lock on the partitioned table only for a brief period. The same kind of idea can be used to merge partitions. But for specific situation like this (certain partition is not changing) we can add CONCURRENTLY modifier. Our DDL query can be like ALTER TABLE...SPLIT PARTITION [CONCURRENTLY]; With CONCURRENTLY modifier we can lock partitioned table in ShareUpdateExclusiveLock mode and split partition - in AccessExclusiveLock mode. So we don't lock partitioned table in AccessExclusiveLock mode and can modify other partitions during SPLIT operation (except split partition). If smb try to modify split partition, he will receive error "relation does not exist" at end of operation (because split partition will be drop). -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
Thanks for your advice, Justin and Alvaro! I'll try to reduce the size of this patch and split it into separate parts (for MERGE and SPLIT). -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: [PATCH] Improve amcheck to also check UNIQUE constraint in btree index.
Hi! I would make two cosmetic changes. 1. I suggest replace description of function bt_report_duplicate() from ``` /* * Prepare and print an error message for unique constrain violation in * a btree index under WARNING level. Also set a flag to report ERROR * at the end of the check. */ ``` to ``` /* * Prepare an error message for unique constrain violation in * a btree index and report ERROR. */ ``` 2. I think will be better to change test 004_verify_nbtree_unique.pl - replace ``` use Test::More tests => 6; ``` to ``` use Test::More; ... done_testing(); ``` (same as in the other three tests). -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.comFrom 93a10abd0afb14b264e4cf59f7e92f619dd9b11a Mon Sep 17 00:00:00 2001 From: Pavel Borisov Date: Wed, 11 May 2022 15:54:13 +0400 Subject: [PATCH v15] Add option for amcheck and pg_amcheck to check unique constraint for btree indexes. Add 'checkunique' argument to bt_index_check() and bt_index_parent_check(). When the flag is specified the procedures will check the unique constraint violation for unique indexes. Only one heap entry for all equal keys in the index should be visible (including posting list entries). Report an error otherwise. pg_amcheck called with --checkunique option will do the same check for all the indexes it checks. Author: Anastasia Lubennikova Author: Pavel Borisov Author: Maxim Orlov Reviewed-by: Mark Dilger Reviewed-by: Zhihong Yu Reviewed-by: Peter Geoghegan Reviewed-by: Aleksander Alekseev Discussion: https://postgr.es/m/CALT9ZEHRn5xAM5boga0qnrCmPV52bScEK2QnQ1HmUZDD301JEg%40mail.gmail.com --- contrib/amcheck/Makefile | 2 +- contrib/amcheck/amcheck--1.3--1.4.sql | 29 ++ contrib/amcheck/amcheck.control | 2 +- contrib/amcheck/expected/check_btree.out | 42 +++ contrib/amcheck/sql/check_btree.sql | 14 + contrib/amcheck/t/004_verify_nbtree_unique.pl | 235 + contrib/amcheck/verify_nbtree.c | 329 +- doc/src/sgml/amcheck.sgml | 14 +- doc/src/sgml/ref/pg_amcheck.sgml | 11 + src/bin/pg_amcheck/pg_amcheck.c | 50 ++- src/bin/pg_amcheck/t/003_check.pl | 45 +++ src/bin/pg_amcheck/t/005_opclass_damage.pl| 65 12 files changed, 814 insertions(+), 24 deletions(-) create mode 100644 contrib/amcheck/amcheck--1.3--1.4.sql create mode 100644 contrib/amcheck/t/004_verify_nbtree_unique.pl diff --git a/contrib/amcheck/Makefile b/contrib/amcheck/Makefile index b82f221e50..88271687a3 100644 --- a/contrib/amcheck/Makefile +++ b/contrib/amcheck/Makefile @@ -7,7 +7,7 @@ OBJS = \ verify_nbtree.o EXTENSION = amcheck -DATA = amcheck--1.2--1.3.sql amcheck--1.1--1.2.sql amcheck--1.0--1.1.sql amcheck--1.0.sql +DATA = amcheck--1.3--1.4.sql amcheck--1.2--1.3.sql amcheck--1.1--1.2.sql amcheck--1.0--1.1.sql amcheck--1.0.sql PGFILEDESC = "amcheck - function for verifying relation integrity" REGRESS = check check_btree check_heap diff --git a/contrib/amcheck/amcheck--1.3--1.4.sql b/contrib/amcheck/amcheck--1.3--1.4.sql new file mode 100644 index 00..75574eaa64 --- /dev/null +++ b/contrib/amcheck/amcheck--1.3--1.4.sql @@ -0,0 +1,29 @@ +/* contrib/amcheck/amcheck--1.3--1.4.sql */ + +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use "ALTER EXTENSION amcheck UPDATE TO '1.4'" to load this file. \quit + +-- In order to avoid issues with dependencies when updating amcheck to 1.4, +-- create new, overloaded versions of the 1.2 bt_index_parent_check signature, +-- and 1.1 bt_index_check signature. + +-- +-- bt_index_parent_check() +-- +CREATE FUNCTION bt_index_parent_check(index regclass, +heapallindexed boolean, rootdescend boolean, checkunique boolean) +RETURNS VOID +AS 'MODULE_PATHNAME', 'bt_index_parent_check' +LANGUAGE C STRICT PARALLEL RESTRICTED; +-- +-- bt_index_check() +-- +CREATE FUNCTION bt_index_check(index regclass, +heapallindexed boolean, checkunique boolean) +RETURNS VOID +AS 'MODULE_PATHNAME', 'bt_index_check' +LANGUAGE C STRICT PARALLEL RESTRICTED; + +-- We don't want this to be available to public +REVOKE ALL ON FUNCTION bt_index_parent_check(regclass, boolean, boolean, boolean) FROM PUBLIC; +REVOKE ALL ON FUNCTION bt_index_check(regclass, boolean, boolean) FROM PUBLIC; diff --git a/contrib/amcheck/amcheck.control b/contrib/amcheck/amcheck.control index ab50931f75..e67ace01c9 100644 --- a/contrib/amcheck/amcheck.control +++ b/contrib/amcheck/amcheck.control @@ -1,5 +1,5 @@ # amcheck extension comment = 'functions for verifying relation integrity' -default_version = '1.3' +default_version = '1.4' module_pathname = '$libdir/amcheck' relocatable = true diff --git a/contrib/amcheck/expected/check_btree.out b/contrib/amcheck/expected/check_btree.out index 38791bbc1f..86b38d93f4 100644 --- a/contrib/amcheck/expected/check_btree.ou
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
This is not a review, but I think the isolation tests should be expanded. At least, include the case of serializable transactions being involved. Thanks! I will expand the tests for the next commitfest. -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: enable/disable broken for statement triggers on partitioned tables
I agree that the patch shouldn't have changed that behavior, so I've fixed the patch so that EnableDisableTrigger() recurses even if the parent trigger is unchanged. Thanks, I think this patch is ready for committer. -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: enable/disable broken for statement triggers on partitioned tables
Hi!
I've looked through the code and everything looks good.
But there is one thing I doubt.
Patch changes result of test:
create function trig_nothing() returns trigger language plpgsql
as $$ begin return null; end $$;
create table parent (a int) partition by list (a);
create table child1 partition of parent for values in (1);
create trigger tg after insert on parent
for each row execute procedure trig_nothing();
select tgrelid::regclass, tgname, tgenabled from pg_trigger
where tgrelid in ('parent'::regclass, 'child1'::regclass)
order by tgrelid::regclass::text;
alter table only parent enable always trigger tg; -- no recursion
select tgrelid::regclass, tgname, tgenabled from pg_trigger
where tgrelid in ('parent'::regclass, 'child1'::regclass)
order by tgrelid::regclass::text;
alter table parent enable always trigger tg; -- recursion
select tgrelid::regclass, tgname, tgenabled from pg_trigger
where tgrelid in ('parent'::regclass, 'child1'::regclass)
order by tgrelid::regclass::text;
drop table parent, child1;
drop function trig_nothing();
Results of vanilla + patch:
CREATE FUNCTION
CREATE TABLE
CREATE TABLE
CREATE TRIGGER
tgrelid | tgname | tgenabled
-++---
child1 | tg | O
parent | tg | O
(2 rows)
ALTER TABLE
tgrelid | tgname | tgenabled
-++---
child1 | tg | O
parent | tg | A
(2 rows)
ALTER TABLE
tgrelid | tgname | tgenabled
-++---
child1 | tg | O
parent | tg | A
(2 rows)
DROP TABLE
DROP FUNCTION
Results of vanilla:
CREATE FUNCTION
CREATE TABLE
CREATE TABLE
CREATE TRIGGER
tgrelid | tgname | tgenabled
-++---
child1 | tg | O
parent | tg | O
(2 rows)
ALTER TABLE
tgrelid | tgname | tgenabled
-++---
child1 | tg | O
parent | tg | A
(2 rows)
ALTER TABLE
tgrelid | tgname | tgenabled
-++---
child1 | tg | A
parent | tg | A
(2 rows)
DROP TABLE
DROP FUNCTION
The patch doesn't start recursion in case 'tgenabled' flag of parent
table is not changes.
Probably vanilla result is more correct.
--
With best regards,
Dmitry Koval
Postgres Professional: http://postgrespro.com
Re: Add SPLIT PARTITION/MERGE PARTITIONS commands
I didn't read the patch, but what lock level does that place on the partitioned table? Anything more than ACCESS SHARE? Current patch locks a partitioned table with ACCESS EXCLUSIVE lock. Unfortunately only this lock guarantees that other session can not work with partitions that are splitting or merging. I want add CONCURRENTLY mode in future. With this mode partitioned table during SPLIT/MERGE operation will be locked with SHARE UPDATE EXCLUSIVE (as ATTACH/DETACH PARTITION commands in CONCURRENTLY mode). But in this case queries from other sessions that want to work with partitions that are splitting/merging at this time should receive an error (like "Partition data is moving. Repeat the operation later") because old partitions will be deleted at the end of SPLIT/MERGE operation. I hope exists a better solution, but I don't know it now... -- With best regards, Dmitry Koval Postgres Professional: http://postgrespro.com
Re: WIN32 pg_import_system_collations
Hi Juan José, I a bit tested this feature and have small doubts about block: +/* + * Windows will use hyphens between language and territory, where POSIX + * uses an underscore. Simply make it POSIX looking. + */ + hyphen = strchr(localebuf, '-'); + if (hyphen) +*hyphen = '_'; After this block modified collation name is used in function GetNLSVersionEx(COMPARE_STRING, wide_collcollate, ) (see win32_read_locale() -> CollationFromLocale() -> CollationCreate() call). Is it correct to use (wide_collcollate = "en_NZ") instead of (wide_collcollate = "en-NZ") in GetNLSVersionEx() function? 1) Documentation [1], [2], quote: If it is a neutral locale for which the script is significant, the pattern is
Re: WIN32 pg_import_system_collations
Hi Juan José, I a bit tested this feature and have small doubts about block: +/* + * Windows will use hyphens between language and territory, where POSIX + * uses an underscore. Simply make it POSIX looking. + */ + hyphen = strchr(localebuf, '-'); + if (hyphen) +*hyphen = '_'; After this block modified collation name is used in function GetNLSVersionEx(COMPARE_STRING, wide_collcollate, ) (see win32_read_locale() -> CollationFromLocale() -> CollationCreate() call). Is it correct to use (wide_collcollate = "en_NZ") instead of (wide_collcollate = "en-NZ") in GetNLSVersionEx() function? 1) Documentation [1], [2], quote: If it is a neutral locale for which the script is significant, the pattern is
