> 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
>From f205a06c9aa6061dd1bac01ef63017aa6811b5f9 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@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.c
index 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.c
index 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.h
index 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
>From 2d3d7c49fa8fcd14a5dcaddfffb7b4239b9cab62 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvhe...@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.c
index 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.c
index 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.out
index 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.sql
index 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
