From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: propagating replica identity to partitions
Date: Mon, 4 Feb 2019 14:20:58 -0300
Message-ID: <201902041720.gv2f3hw5rfhe@alvherre.pgsql> (raw)
In-Reply-To: <201902041630.gpadougzab7v@alvherre.pgsql>
> I propose to change the behavior to:
... this patch.
I'll now look more carefully at the cases involving indexes; thus far I
was looking at the ones using FULL. Those seem to work as intended.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:download all (2 files, .tar.gz)0001-index_get_partition.patch (text/x-diff, 4.3K) download
From f205a06c9aa6061dd1bac01ef63017aa6811b5f9 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 22 Jan 2019 18:00:31 -0300
Subject: [PATCH 1/2] index_get_partition
---
src/backend/catalog/partition.c | 35 +++++++++++++++++++++++++++++++++++
src/backend/commands/tablecmds.c | 40 +++++++++++-----------------------------
src/include/catalog/partition.h | 1 +
3 files changed, 47 insertions(+), 29 deletions(-)
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.cindex 0d3bc3a2c7..66012bb28a 100644--- a/src/backend/catalog/partition.c+++ b/src/backend/catalog/partition.c@@ -146,6 +146,41 @@ get_partition_ancestors_worker(Relation inhRel, Oid relid, List **ancestors)
}
/*
+ * Return the OID of the index, in the given partition, that is a child of the+ * given index or InvalidOid if there isn't one.+ */+Oid+index_get_partition(Relation partition, Oid indexId)+{+ List *idxlist = RelationGetIndexList(partition);+ ListCell *l;++ foreach(l, idxlist)+ {+ Oid partIdx = lfirst_oid(l);+ HeapTuple tup;+ Form_pg_class classForm;+ bool ispartition;++ tup = SearchSysCache1(RELOID, ObjectIdGetDatum(partIdx));+ if (!tup)+ elog(ERROR, "cache lookup failed for relation %u", partIdx);+ classForm = (Form_pg_class) GETSTRUCT(tup);+ ispartition = classForm->relispartition;+ ReleaseSysCache(tup);+ if (!ispartition)+ continue;+ if (get_partition_parent(lfirst_oid(l)) == indexId)+ {+ list_free(idxlist);+ return partIdx;+ }+ }++ return InvalidOid;+}++/*
* map_partition_varattnos - maps varattno of any Vars in expr from the
* attno's of 'from_rel' to the attno's of 'to_rel' partition, each of which
* may be either a leaf partition or a partitioned table, but both of which
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 35a9ade059..877bac506f 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -15427,36 +15427,18 @@ ATExecAttachPartitionIdx(List **wqueue, Relation parentIdx, RangeVar *name)
static void
refuseDupeIndexAttach(Relation parentIdx, Relation partIdx, Relation partitionTbl)
{
- Relation pg_inherits;- ScanKeyData key;- HeapTuple tuple;- SysScanDesc scan;+ Oid existingIdx;- pg_inherits = table_open(InheritsRelationId, AccessShareLock);- ScanKeyInit(&key, Anum_pg_inherits_inhparent,- BTEqualStrategyNumber, F_OIDEQ,- ObjectIdGetDatum(RelationGetRelid(parentIdx)));- scan = systable_beginscan(pg_inherits, InheritsParentIndexId, true,- NULL, 1, &key);- while (HeapTupleIsValid(tuple = systable_getnext(scan)))- {- Form_pg_inherits inhForm;- Oid tab;-- inhForm = (Form_pg_inherits) GETSTRUCT(tuple);- tab = IndexGetRelation(inhForm->inhrelid, false);- if (tab == RelationGetRelid(partitionTbl))- ereport(ERROR,- (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),- errmsg("cannot attach index \"%s\" as a partition of index \"%s\"",- RelationGetRelationName(partIdx),- RelationGetRelationName(parentIdx)),- errdetail("Another index is already attached for partition \"%s\".",- RelationGetRelationName(partitionTbl))));- }-- systable_endscan(scan);- table_close(pg_inherits, AccessShareLock);+ existingIdx = index_get_partition(partitionTbl,+ RelationGetRelid(parentIdx));+ if (OidIsValid(existingIdx))+ ereport(ERROR,+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),+ errmsg("cannot attach index \"%s\" as a partition of index \"%s\"",+ RelationGetRelationName(partIdx),+ RelationGetRelationName(parentIdx)),+ errdetail("Another index is already attached for partition \"%s\".",+ RelationGetRelationName(partitionTbl))));
}
/*
diff --git a/src/include/catalog/partition.h b/src/include/catalog/partition.hindex 5685d2fd57..7f0b198bcb 100644--- a/src/include/catalog/partition.h+++ b/src/include/catalog/partition.h@@ -35,6 +35,7 @@ typedef struct PartitionDescData
extern Oid get_partition_parent(Oid relid);
extern List *get_partition_ancestors(Oid relid);
+extern Oid index_get_partition(Relation partition, Oid indexId);
extern List *map_partition_varattnos(List *expr, int fromrel_varno,
Relation to_rel, Relation from_rel,
bool *found_whole_row);
--
2.11.0
0002-Propagate-replica-identity-to-partitions.patch (text/x-diff, 14.8K) download
From 2d3d7c49fa8fcd14a5dcaddfffb7b4239b9cab62 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Mon, 4 Feb 2019 13:43:58 -0300
Subject: [PATCH 2/2] Propagate replica identity to partitions
---
src/backend/commands/tablecmds.c | 59 +++++++++++++--
src/bin/psql/describe.c | 3 +-
src/test/regress/expected/replica_identity.out | 99 ++++++++++++++++++++++++++
src/test/regress/sql/replica_identity.sql | 33 +++++++++
4 files changed, 189 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 877bac506f..4b2ae01f15 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -300,7 +300,7 @@ static void truncate_check_activity(Relation rel);
static void RangeVarCallbackForTruncate(const RangeVar *relation,
Oid relId, Oid oldRelId, void *arg);
static List *MergeAttributes(List *schema, List *supers, char relpersistence,
- bool is_partition, List **supconstr);+ bool is_partition, List **supconstr, char *ri_type);
static bool MergeCheckConstraint(List *constraints, char *name, Node *expr);
static void MergeAttributesIntoExisting(Relation child_rel, Relation parent_rel);
static void MergeConstraintsIntoExisting(Relation child_rel, Relation parent_rel);
@@ -485,6 +485,7 @@ static void AttachPartitionEnsureIndexes(Relation rel, Relation attachrel);
static void QueuePartitionConstraintValidation(List **wqueue, Relation scanrel,
List *partConstraint,
bool validate_default);
+static void MatchReplicaIdentity(Relation tgtrel, Relation srcrel);
static void CloneRowTriggersToPartition(Relation parent, Relation partition);
static ObjectAddress ATExecDetachPartition(Relation rel, RangeVar *name);
static ObjectAddress ATExecAttachPartitionIdx(List **wqueue, Relation rel,
@@ -527,6 +528,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
TupleDesc descriptor;
List *inheritOids;
List *old_constraints;
+ char ri_type;
List *rawDefaults;
List *cookedDefaults;
Datum reloptions;
@@ -708,7 +710,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
MergeAttributes(stmt->tableElts, inheritOids,
stmt->relation->relpersistence,
stmt->partbound != NULL,
- &old_constraints);+ &old_constraints, &ri_type);
/*
* Create a tuple descriptor from the relation schema. Note that this
@@ -1014,6 +1016,10 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
*/
CloneForeignKeyConstraints(parentId, relationId, NULL);
+ /* Propagate REPLICA IDENTITY information too */+ if (ri_type != REPLICA_IDENTITY_DEFAULT)+ MatchReplicaIdentity(rel, parent);+
table_close(parent, NoLock);
}
@@ -1873,6 +1879,8 @@ storage_name(char c)
* Output arguments:
* 'supconstr' receives a list of constraints belonging to the parents,
* updated as necessary to be valid for the child.
+ * 'ri_type' receives the replica identity type of the last parent seen,+ * or default if none.
*
* Return value:
* Completed schema list.
@@ -1914,11 +1922,15 @@ storage_name(char c)
* (4) Otherwise the inherited default is used.
* Rule (3) is new in Postgres 7.1; in earlier releases you got a
* rather arbitrary choice of which parent default to use.
+ *+ * It only makes sense to use the returned 'ri_type' when there's a single+ * parent, such as in declarative partitioning.
*----------
*/
static List *
MergeAttributes(List *schema, List *supers, char relpersistence,
- bool is_partition, List **supconstr)+ bool is_partition, List **supconstr,+ char *ri_type)
{
ListCell *entry;
List *inhSchema = NIL;
@@ -2015,6 +2027,11 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
}
/*
+ * Initialize replica identity to default; parents may change it later+ */+ *ri_type = REPLICA_IDENTITY_DEFAULT;++ /*
* Scan the parents left-to-right, and merge their attributes to form a
* list of inherited attributes (inhSchema). Also check to see if we need
* to inherit an OID column.
@@ -2095,6 +2112,9 @@ MergeAttributes(List *schema, List *supers, char relpersistence,
? "cannot inherit from temporary relation of another session"
: "cannot create as partition of temporary relation of another session")));
+ /* Indicate replica identity back to caller */+ *ri_type = relation->rd_rel->relreplident;+
/*
* We should have an UNDER permission flag for this, but for now,
* demand that creator of a child table own the parent.
@@ -3935,7 +3955,9 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_ReplicaIdentity: /* REPLICA IDENTITY ... */
ATSimplePermissions(rel, ATT_TABLE | ATT_MATVIEW);
pass = AT_PASS_MISC;
- /* This command never recurses */+ /* Recurse only on partitioned tables */+ if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)+ ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
/* No command-specific prep needed */
break;
case AT_EnableTrig: /* ENABLE TRIGGER variants */
@@ -14664,6 +14686,10 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
/* and triggers */
CloneRowTriggersToPartition(rel, attachrel);
+ /* Propagate REPLICA IDENTITY information */+ if (rel->rd_rel->relreplident != REPLICA_IDENTITY_DEFAULT)+ MatchReplicaIdentity(attachrel, rel);+
/*
* Clone foreign key constraints, and setup for Phase 3 to verify them.
*/
@@ -14915,6 +14941,31 @@ AttachPartitionEnsureIndexes(Relation rel, Relation attachrel)
}
/*
+ * Set up partRel's (a partition) replica identity to match parentRel's (its+ * parent).+ */+static void+MatchReplicaIdentity(Relation partRel, Relation srcrel)+{+ Oid ri_index;++ if (srcrel->rd_rel->relreplident == REPLICA_IDENTITY_INDEX)+ {+ ri_index = index_get_partition(partRel,+ RelationGetReplicaIndex(srcrel));+ if (!OidIsValid(ri_index))+ ereport(ERROR,+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),+ errmsg("replica index does not exist in partition")));+ }+ else+ ri_index = InvalidOid;++ relation_mark_replica_identity(partRel, srcrel->rd_rel->relreplident,+ ri_index, true);+}++/*
* CloneRowTriggersToPartition
* subroutine for ATExecAttachPartition/DefineRelation to create row
* triggers on partitions
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.cindex 4da6719ce7..6145a000cb 100644--- a/src/bin/psql/describe.c+++ b/src/bin/psql/describe.c@@ -3113,7 +3113,8 @@ describeOneTableDetails(const char *schemaname,
if (verbose &&
(tableinfo.relkind == RELKIND_RELATION ||
- tableinfo.relkind == RELKIND_MATVIEW) &&+ tableinfo.relkind == RELKIND_MATVIEW ||+ tableinfo.relkind == RELKIND_PARTITIONED_TABLE) &&
/*
* No need to display default values; we already display a REPLICA
diff --git a/src/test/regress/expected/replica_identity.out b/src/test/regress/expected/replica_identity.outindex 175ecd2879..3051cf1551 100644--- a/src/test/regress/expected/replica_identity.out+++ b/src/test/regress/expected/replica_identity.out@@ -181,3 +181,102 @@ SELECT relreplident FROM pg_class WHERE oid = 'test_replica_identity'::regclass;
DROP TABLE test_replica_identity;
DROP TABLE test_replica_identity_othertable;
+----+-- Make sure it propagates to partitions+----+CREATE TABLE test_replica_identity_part (a int, b int) PARTITION BY RANGE (a);+CREATE TABLE test_replica_identity_part1 PARTITION OF test_replica_identity_part+ FOR VALUES FROM (0) TO (1000) PARTITION BY RANGE (a);+CREATE TABLE test_replica_identity_part2 PARTITION OF test_replica_identity_part+ FOR VALUES FROM (1000) TO (2000);+CREATE TABLE test_replica_identity_part11 PARTITION OF test_replica_identity_part1+ FOR VALUES FROM (1000) TO (1500);+ALTER TABLE test_replica_identity_part REPLICA IDENTITY FULL;+CREATE TABLE test_replica_identity_part3 PARTITION OF test_replica_identity_part+ FOR VALUES FROM (2000) TO (3000);+CREATE TABLE test_replica_identity_part4 (LIKE test_replica_identity_part);+ALTER TABLE test_replica_identity_part ATTACH PARTITION test_replica_identity_part4+ FOR VALUES FROM (3000) TO (4000);+\d+ test_replica_identity_part2+ Table "public.test_replica_identity_part2"+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+-------------+ a | integer | | | | plain | | + b | integer | | | | plain | | +Partition of: test_replica_identity_part FOR VALUES FROM (1000) TO (2000)+Partition constraint: ((a IS NOT NULL) AND (a >= 1000) AND (a < 2000))+Replica Identity: FULL++\d+ test_replica_identity_part11+ Table "public.test_replica_identity_part11"+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+-------------+ a | integer | | | | plain | | + b | integer | | | | plain | | +Partition of: test_replica_identity_part1 FOR VALUES FROM (1000) TO (1500)+Partition constraint: ((a IS NOT NULL) AND (a >= 0) AND (a < 1000) AND (a IS NOT NULL) AND (a >= 1000) AND (a < 1500))+Replica Identity: FULL++\d+ test_replica_identity_part+ Partitioned table "public.test_replica_identity_part"+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+-------------+ a | integer | | | | plain | | + b | integer | | | | plain | | +Partition key: RANGE (a)+Partitions: test_replica_identity_part1 FOR VALUES FROM (0) TO (1000), PARTITIONED,+ test_replica_identity_part2 FOR VALUES FROM (1000) TO (2000),+ test_replica_identity_part3 FOR VALUES FROM (2000) TO (3000),+ test_replica_identity_part4 FOR VALUES FROM (3000) TO (4000)+Replica Identity: FULL++\d+ test_replica_identity_part3+ Table "public.test_replica_identity_part3"+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+-------------+ a | integer | | | | plain | | + b | integer | | | | plain | | +Partition of: test_replica_identity_part FOR VALUES FROM (2000) TO (3000)+Partition constraint: ((a IS NOT NULL) AND (a >= 2000) AND (a < 3000))+Replica Identity: FULL++\d+ test_replica_identity_part4+ Table "public.test_replica_identity_part4"+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+-------------+ a | integer | | | | plain | | + b | integer | | | | plain | | +Partition of: test_replica_identity_part FOR VALUES FROM (3000) TO (4000)+Partition constraint: ((a IS NOT NULL) AND (a >= 3000) AND (a < 4000))+Replica Identity: FULL++----+-- Check behavior with inherited tables+----+CREATE TABLE test_replica_identity_inh (a int);+CREATE TABLE test_replica_identity_cld () INHERITS (test_replica_identity_inh);+ALTER TABLE test_replica_identity_inh REPLICA IDENTITY FULL;+CREATE TABLE test_replica_identity_cld2 () INHERITS (test_replica_identity_inh);+\d+ test_replica_identity_inh+ Table "public.test_replica_identity_inh"+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+-------------+ a | integer | | | | plain | | +Child tables: test_replica_identity_cld,+ test_replica_identity_cld2+Replica Identity: FULL++\d+ test_replica_identity_cld+ Table "public.test_replica_identity_cld"+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+-------------+ a | integer | | | | plain | | +Inherits: test_replica_identity_inh++\d+ test_replica_identity_cld2+ Table "public.test_replica_identity_cld2"+ Column | Type | Collation | Nullable | Default | Storage | Stats target | Description +--------+---------+-----------+----------+---------+---------+--------------+-------------+ a | integer | | | | plain | | +Inherits: test_replica_identity_inh+diff --git a/src/test/regress/sql/replica_identity.sql b/src/test/regress/sql/replica_identity.sqlindex b08a3623b8..a567f2b52d 100644--- a/src/test/regress/sql/replica_identity.sql+++ b/src/test/regress/sql/replica_identity.sql@@ -77,3 +77,36 @@ SELECT relreplident FROM pg_class WHERE oid = 'test_replica_identity'::regclass;
DROP TABLE test_replica_identity;
DROP TABLE test_replica_identity_othertable;
++----+-- Make sure it propagates to partitions+----+CREATE TABLE test_replica_identity_part (a int, b int) PARTITION BY RANGE (a);+CREATE TABLE test_replica_identity_part1 PARTITION OF test_replica_identity_part+ FOR VALUES FROM (0) TO (1000) PARTITION BY RANGE (a);+CREATE TABLE test_replica_identity_part2 PARTITION OF test_replica_identity_part+ FOR VALUES FROM (1000) TO (2000);+CREATE TABLE test_replica_identity_part11 PARTITION OF test_replica_identity_part1+ FOR VALUES FROM (1000) TO (1500);+ALTER TABLE test_replica_identity_part REPLICA IDENTITY FULL;+CREATE TABLE test_replica_identity_part3 PARTITION OF test_replica_identity_part+ FOR VALUES FROM (2000) TO (3000);+CREATE TABLE test_replica_identity_part4 (LIKE test_replica_identity_part);+ALTER TABLE test_replica_identity_part ATTACH PARTITION test_replica_identity_part4+ FOR VALUES FROM (3000) TO (4000);+\d+ test_replica_identity_part2+\d+ test_replica_identity_part11+\d+ test_replica_identity_part+\d+ test_replica_identity_part3+\d+ test_replica_identity_part4++----+-- Check behavior with inherited tables+----+CREATE TABLE test_replica_identity_inh (a int);+CREATE TABLE test_replica_identity_cld () INHERITS (test_replica_identity_inh);+ALTER TABLE test_replica_identity_inh REPLICA IDENTITY FULL;+CREATE TABLE test_replica_identity_cld2 () INHERITS (test_replica_identity_inh);+\d+ test_replica_identity_inh+\d+ test_replica_identity_cld+\d+ test_replica_identity_cld2--
2.11.0
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@2ndquadrant.com, pgsql-hackers@lists.postgresql.org
Subject: Re: propagating replica identity to partitions
In-Reply-To: <201902041720.gv2f3hw5rfhe@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox