public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v5 2/3] Allow CREATE INDEX CONCURRENTLY on partitioned table 3+ messages / 3 participants [nested] [flat]
* [PATCH v5 2/3] Allow CREATE INDEX CONCURRENTLY on partitioned table @ 2020-06-06 22:42 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Justin Pryzby @ 2020-06-06 22:42 UTC (permalink / raw) Note, this effectively reverts 050098b14, so take care to not reintroduce the bug it fixed. --- doc/src/sgml/ref/create_index.sgml | 9 --- src/backend/commands/indexcmds.c | 104 +++++++++++++++++++------ src/test/regress/expected/indexing.out | 57 +++++++++++++- src/test/regress/sql/indexing.sql | 16 +++- 4 files changed, 148 insertions(+), 38 deletions(-) diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml index 33aa64e81d..c780dc9547 100644 --- a/doc/src/sgml/ref/create_index.sgml +++ b/doc/src/sgml/ref/create_index.sgml @@ -657,15 +657,6 @@ Indexes: cannot. </para> - <para> - Concurrent builds for indexes on partitioned tables are currently not - supported. However, you may concurrently build the index on each - partition individually and then finally create the partitioned index - non-concurrently in order to reduce the time where writes to the - partitioned table will be locked out. In this case, building the - partitioned index is a metadata only operation. - </para> - </refsect2> </refsect1> diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 68d75916ae..cdf31a47d8 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -655,17 +655,6 @@ DefineIndex(Oid relationId, partitioned = rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE; if (partitioned) { - /* - * Note: we check 'stmt->concurrent' rather than 'concurrent', so that - * the error is thrown also for temporary tables. Seems better to be - * consistent, even though we could do it on temporary table because - * we're not actually doing it concurrently. - */ - if (stmt->concurrent) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot create index on partitioned table \"%s\" concurrently", - RelationGetRelationName(rel)))); if (stmt->excludeOpNames) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), @@ -1100,6 +1089,9 @@ DefineIndex(Oid relationId, if (pd->nparts != 0) flags |= INDEX_CREATE_INVALID; } + else if (concurrent && OidIsValid(parentIndexId)) + /* Initial concurrent build of index partition is invalid */ + flags |= INDEX_CREATE_INVALID; if (stmt->deferrable) constr_flags |= INDEX_CONSTR_CREATE_DEFERRABLE; @@ -1152,8 +1144,17 @@ DefineIndex(Oid relationId, */ if (!stmt->relation || stmt->relation->inh) { + /* + * Need to close the relation before recursing into children, so + * copy needed data into a longlived context. + */ + + MemoryContext ind_context = AllocSetContextCreate(PortalContext, "CREATE INDEX", + ALLOCSET_DEFAULT_SIZES); + MemoryContext oldcontext = MemoryContextSwitchTo(ind_context); PartitionDesc partdesc = RelationGetPartitionDesc(rel); int nparts = partdesc->nparts; + char *relname = pstrdup(RelationGetRelationName(rel)); Oid *part_oids = palloc(sizeof(Oid) * nparts); bool invalidate_parent = false; TupleDesc parentDesc; @@ -1163,8 +1164,10 @@ DefineIndex(Oid relationId, nparts); memcpy(part_oids, partdesc->oids, sizeof(Oid) * nparts); + parentDesc = CreateTupleDescCopy(RelationGetDescr(rel)); + table_close(rel, NoLock); + MemoryContextSwitchTo(oldcontext); - parentDesc = RelationGetDescr(rel); opfamOids = palloc(sizeof(Oid) * numberOfKeyAttributes); for (i = 0; i < numberOfKeyAttributes; i++) opfamOids[i] = get_opclass_family(classObjectId[i]); @@ -1199,18 +1202,20 @@ DefineIndex(Oid relationId, ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("cannot create unique index on partitioned table \"%s\"", - RelationGetRelationName(rel)), + relname), errdetail("Table \"%s\" contains partitions that are foreign tables.", - RelationGetRelationName(rel)))); + relname))); table_close(childrel, lockmode); continue; } + oldcontext = MemoryContextSwitchTo(ind_context); childidxs = RelationGetIndexList(childrel); attmap = build_attrmap_by_name(RelationGetDescr(childrel), parentDesc); + MemoryContextSwitchTo(oldcontext); foreach(cell, childidxs) { @@ -1336,10 +1341,17 @@ DefineIndex(Oid relationId, createdConstraintId, is_alter_table, check_rights, check_not_in_use, skip_build, quiet); + if (concurrent) + { + PushActiveSnapshot(GetTransactionSnapshot()); + invalidate_parent = true; + } } - pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, - i + 1); + /* For concurrent build, this is a catalog-only stage */ + if (!concurrent) + pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, + i + 1); free_attrmap(attmap); } @@ -1366,23 +1378,72 @@ DefineIndex(Oid relationId, table_close(pg_index, RowExclusiveLock); heap_freetuple(newtup); } + } else + table_close(rel, NoLock); + + if (concurrent) + { + List *childs; + ListCell *lc; + int npart = 0; + + /* Reindex invalid child indexes created earlier */ + MemoryContext ind_context = AllocSetContextCreate(PortalContext, "CREATE INDEX", + ALLOCSET_DEFAULT_SIZES); + MemoryContext oldcontext = MemoryContextSwitchTo(ind_context); + childs = find_inheritance_children(indexRelationId, NoLock); + MemoryContextSwitchTo(oldcontext); + + /* Make the catalog changes visible to get_partition_parent */ + CommandCounterIncrement(); + + foreach (lc, childs) + { + Oid indexrelid = lfirst_oid(lc); + Relation cldidx; + bool isvalid; + + if (!OidIsValid(parentIndexId)) + pgstat_progress_update_param(PROGRESS_CREATEIDX_PARTITIONS_DONE, + npart++); + + cldidx = index_open(indexrelid, ShareUpdateExclusiveLock); + isvalid = cldidx->rd_index->indisvalid; + index_close(cldidx, NoLock); + if (isvalid) + continue; + + /* This may be a partitioned index, which is fine too */ + ReindexRelationConcurrently(indexrelid, 0); + PushActiveSnapshot(GetTransactionSnapshot()); + } + + PopActiveSnapshot(); + CommitTransactionCommand(); + StartTransactionCommand(); + + /* + * CIC needs to mark a partitioned index as VALID, which itself + * requires setting READY, which is unset for CIC (even though + * it's meaningless for an index without storage). + */ + index_set_state_flags(indexRelationId, INDEX_CREATE_SET_READY); + CommandCounterIncrement(); + index_set_state_flags(indexRelationId, INDEX_CREATE_SET_VALID); } /* * Indexes on partitioned tables are not themselves built, so we're * done here. */ - table_close(rel, NoLock); if (!OidIsValid(parentIndexId)) pgstat_progress_end_command(); return address; } + table_close(rel, NoLock); if (!concurrent) { - /* Close the heap and we're done, in the non-concurrent case */ - table_close(rel, NoLock); - /* If this is the top-level index, we're done. */ if (!OidIsValid(parentIndexId)) pgstat_progress_end_command(); @@ -1390,10 +1451,9 @@ DefineIndex(Oid relationId, return address; } - /* save lockrelid and locktag for below, then close rel */ + /* save lockrelid and locktag for below */ heaprelid = rel->rd_lockInfo.lockRelId; SET_LOCKTAG_RELATION(heaplocktag, heaprelid.dbId, heaprelid.relId); - table_close(rel, NoLock); /* * For a concurrent build, it's important to make the catalog entries diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out index f78865ef81..1ae58e110f 100644 --- a/src/test/regress/expected/indexing.out +++ b/src/test/regress/expected/indexing.out @@ -50,11 +50,60 @@ select relname, relkind, relhassubclass, inhparent::regclass (8 rows) drop table idxpart; --- Some unsupported features +-- CIC on partitioned table create table idxpart (a int, b int, c text) partition by range (a); -create table idxpart1 partition of idxpart for values from (0) to (10); -create index concurrently on idxpart (a); -ERROR: cannot create index on partitioned table "idxpart" concurrently +create table idxpart1 partition of idxpart for values from (0) to (10) partition by range(a); +create table idxpart11 partition of idxpart1 for values from (0) to (10) partition by range(a); +create table idxpart2 partition of idxpart for values from (10) to (20); +insert into idxpart2 values(10),(10); -- not unique +create index concurrently on idxpart (a); -- partitioned +create index concurrently on idxpart1 (a); -- partitioned and partition +create index concurrently on idxpart11 (a); -- partitioned and partition, with no leaves +create index concurrently on idxpart2 (a); -- leaf +create unique index concurrently on idxpart (a); -- partitioned, unique failure +ERROR: could not create unique index "idxpart2_a_idx2" +DETAIL: Key (a)=(10) is duplicated. +\d idxpart + Partitioned table "public.idxpart" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | + c | text | | | +Partition key: RANGE (a) +Indexes: + "idxpart_a_idx" btree (a) + "idxpart_a_idx1" UNIQUE, btree (a) INVALID +Number of partitions: 2 (Use \d+ to list them.) + +\d idxpart1 + Partitioned table "public.idxpart1" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | + c | text | | | +Partition of: idxpart FOR VALUES FROM (0) TO (10) +Partition key: RANGE (a) +Indexes: + "idxpart1_a_idx" btree (a) + "idxpart1_a_idx1" btree (a) + "idxpart1_a_idx2" UNIQUE, btree (a) +Number of partitions: 1 (Use \d+ to list them.) + +\d idxpart2 + Table "public.idxpart2" + Column | Type | Collation | Nullable | Default +--------+---------+-----------+----------+--------- + a | integer | | | + b | integer | | | + c | text | | | +Partition of: idxpart FOR VALUES FROM (10) TO (20) +Indexes: + "idxpart2_a_idx" btree (a) + "idxpart2_a_idx1" btree (a) + "idxpart2_a_idx2" UNIQUE, btree (a) INVALID + drop table idxpart; -- Verify bugfix with query on indexed partitioned table with no partitions -- https://postgr.es/m/[email protected] diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql index 35d159f41b..d908ee6d17 100644 --- a/src/test/regress/sql/indexing.sql +++ b/src/test/regress/sql/indexing.sql @@ -29,10 +29,20 @@ select relname, relkind, relhassubclass, inhparent::regclass where relname like 'idxpart%' order by relname; drop table idxpart; --- Some unsupported features +-- CIC on partitioned table create table idxpart (a int, b int, c text) partition by range (a); -create table idxpart1 partition of idxpart for values from (0) to (10); -create index concurrently on idxpart (a); +create table idxpart1 partition of idxpart for values from (0) to (10) partition by range(a); +create table idxpart11 partition of idxpart1 for values from (0) to (10) partition by range(a); +create table idxpart2 partition of idxpart for values from (10) to (20); +insert into idxpart2 values(10),(10); -- not unique +create index concurrently on idxpart (a); -- partitioned +create index concurrently on idxpart1 (a); -- partitioned and partition +create index concurrently on idxpart11 (a); -- partitioned and partition, with no leaves +create index concurrently on idxpart2 (a); -- leaf +create unique index concurrently on idxpart (a); -- partitioned, unique failure +\d idxpart +\d idxpart1 +\d idxpart2 drop table idxpart; -- Verify bugfix with query on indexed partitioned table with no partitions -- 2.17.0 --hYooF8G/hrfVAmum Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v5-0003-Implement-CLUSTER-of-partitioned-table.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Restrict publishing of partitioned table with a foreign table as partition @ 2025-04-07 12:39 Álvaro Herrera <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Álvaro Herrera @ 2025-04-07 12:39 UTC (permalink / raw) To: Shlok Kyal <[email protected]>; +Cc: vignesh C <[email protected]>; Sergey Tatarintsev <[email protected]>; [email protected] Here's the additional changes I made here before giving up on this. I think it needs some additional rethinking, not going to happen for 18. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "The Gord often wonders why people threaten never to come back after they've been told never to return" (www.actsofgord.com) Attachments: [text/x-diff] 0001-some-more-changes.patch.txt (7.4K, ../../[email protected]/2-0001-some-more-changes.patch.txt) download | inline diff: From cc7b4021d5661aa8dbb8bd3a8c8569824dbbbc36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <[email protected]> Date: Mon, 7 Apr 2025 14:31:44 +0200 Subject: [PATCH] some more changes --- src/backend/catalog/pg_publication.c | 63 ++++++++++++----------- src/test/regress/expected/publication.out | 16 ++++++ src/test/regress/sql/publication.sql | 10 ++++ 3 files changed, 59 insertions(+), 30 deletions(-) diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index 51e463c112b..ec76c0c8eb2 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -34,6 +34,7 @@ #include "commands/publicationcmds.h" #include "funcapi.h" #include "partitioning/partdesc.h" +#include "storage/lmgr.h" #include "utils/array.h" #include "utils/builtins.h" #include "utils/catcache.h" @@ -130,53 +131,56 @@ check_publication_add_schema(Oid schemaid) /* * publication_check_foreign_parts - * Helper function to ensure we don't publish foreign tables + * Helper function to ensure we don't indirectly publish foreign tables * - * Foreign tables may not be published. + * DML data changes are not published for data in foreign tables, + * and yet the tablesync worker is not smart enough to omit data from + * foreign tables when they are partitions of partitioned tables. To + * avoid the inconsistencies that would result, we disallow foreign + * tables from being published generally. However, it's possible for + * partitioned tables to have foreign tables as partitions, and we would + * like to allow publishing those partitioned tables so that the other + * partitions are replicated. * - * Protect against including foreign tables that are partitions of partitioned - * tables published by the given publication when publish_via_root_partition is - * true. This will not work correctly as the initial data from the foreign - * table can be replicated by the tablesync worker even though replication of - * foreign table is not supported because when publish_via_root_partition is - * true, the root table is included in pg_subscription_rel catalog table and - * tablesync worker cannot distinguish data from foreign partition. So we - * disallow the case here and in all DDL commands that would end up creating - * such a case indirectly. + * This function is in charge of detecting if a partitioned table has a + * foreign table as a partition -- either in the whole database (useful + * for FOR ALL TABLES publications) or in a particular schema (useful + * for FOR TABLES IN SCHEMA publications). This function must be called + * only for publications with publish_via_partition_root=true. * - * When publish_via_root_partition is set to false, leaf partitions are included - * in pg_subscription_rel catalog table. So, when we include a partition table - * with foreign partition in a publication, we skip including foreign partitions - * to pg_subscription_rel catalog table. So, the foreign partitions are not - * replicated. + * When publish_via_partition_root is false, each partition published for + * replication is listed individually in pg_replication_rel, and we + * don't add partitions that are foreign tables, so this check is not + * needed. * + * If a valid schemaid is provided, check if that schema has any + * partitioned table with a foreign table as partition. * + * If no valid schemaid is provided, check all partitioned tables. * - * If valid schemaid provided, check if the schema has a partition table which - * has a foreign partition. The partition tables in a schema can have partitions - * in other schema. We also need to check if such partitions are foreign - * partition. + * We take a lock on partition tables so no new foreign partitions are + * added concurrently. * - * If valid schemaid is not provided, we get all partition tables and check if - * it has any foreign partition. We take a lock on partition tables so no new - * foreign partitions are added concurrently. - * - * We take a ShareLock on pg_partitioned_table to restrict addition of new - * partitioned table which may contain a foreign partition while publication is - * being created. + * We also take a ShareLock on pg_partitioned_table to restrict addition + * of new partitioned table which may contain a foreign partition while + * publication is being created. XXX this is quite weird actually. */ void publication_check_foreign_parts(Oid schemaid, char *pubname) { Relation classRel; - Relation partRel; ScanKeyData key[3]; int keycount = 0; TableScanDesc scan; HeapTuple tuple; + /* + * Take lock on pg_partitioned_rel. This prevents new publications + * from being created. + */ + LockRelationOid(PartitionedRelationId, ShareLock); + classRel = table_open(RelationRelationId, AccessShareLock); - partRel = table_open(PartitionedRelationId, ShareLock); /* Get the root nodes of partitioned table */ ScanKeyInit(&key[keycount++], @@ -222,7 +226,6 @@ publication_check_foreign_parts(Oid schemaid, char *pubname) table_endscan(scan); table_close(classRel, AccessShareLock); - table_close(partRel, NoLock); } /* diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out index 4395563fcd5..678f8b52e5b 100644 --- a/src/test/regress/expected/publication.out +++ b/src/test/regress/expected/publication.out @@ -2006,6 +2006,22 @@ SELECT pubname, tablename FROM pg_publication_tables WHERE schemaname in ('sch3' ALTER PUBLICATION pub1 SET (publish_via_partition_root); ERROR: cannot set parameter "publish_via_partition_root" to "true" for publication "pub1" DETAIL: Published partitioned table "tmain" contains a partition that is a foreign table. +CREATE SCHEMA sch5; +CREATE SCHEMA sch6; +CREATE TABLE sch6.tmain(id int) PARTITION BY RANGE (id); +CREATE TABLE sch5.part1 PARTITION OF sch6.tmain FOR VALUES FROM (0) TO (10) PARTITION BY RANGE(id); +CREATE FOREIGN TABLE sch6.part2 PARTITION OF sch5.part1 FOR VALUES FROM (0) TO (5) SERVER fdw_server; +CREATE PUBLICATION pub4 FOR TABLES IN SCHEMA sch5 + WITH (publish_via_partition_root); -- should fail +ERROR: cannot add table ... +SELECT pubname, tablename FROM pg_publication_tables WHERE pubname = 'pub4' ORDER BY pubname, tablename; + pubname | tablename +---------+----------- + pub4 | part1 +(1 row) + +DROP SCHEMA sch5, sch6 CASCADE; +DROP PUBLICATION pub4; DROP PUBLICATION pub1; DROP PUBLICATION pub2; DROP PUBLICATION pub3; diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql index 49c9d98b668..43755b973cb 100644 --- a/src/test/regress/sql/publication.sql +++ b/src/test/regress/sql/publication.sql @@ -1296,6 +1296,16 @@ SELECT pubname, tablename FROM pg_publication_tables WHERE schemaname in ('sch3' -- foreign partition ALTER PUBLICATION pub1 SET (publish_via_partition_root); +CREATE SCHEMA sch5; +CREATE SCHEMA sch6; +CREATE TABLE sch6.tmain(id int) PARTITION BY RANGE (id); +CREATE TABLE sch5.part1 PARTITION OF sch6.tmain FOR VALUES FROM (0) TO (10) PARTITION BY RANGE(id); +CREATE FOREIGN TABLE sch6.part2 PARTITION OF sch5.part1 FOR VALUES FROM (0) TO (5) SERVER fdw_server; +CREATE PUBLICATION pub4 FOR TABLES IN SCHEMA sch5 WITH (publish_via_partition_root); +SELECT pubname, tablename FROM pg_publication_tables WHERE pubname = 'pub4' ORDER BY pubname, tablename; +DROP SCHEMA sch5, sch6 CASCADE; +DROP PUBLICATION pub4; + DROP PUBLICATION pub1; DROP PUBLICATION pub2; DROP PUBLICATION pub3; -- 2.39.5 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Restrict publishing of partitioned table with a foreign table as partition @ 2025-04-28 12:48 Shlok Kyal <[email protected]> parent: Álvaro Herrera <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Shlok Kyal @ 2025-04-28 12:48 UTC (permalink / raw) To: Álvaro Herrera <[email protected]>; +Cc: vignesh C <[email protected]>; Sergey Tatarintsev <[email protected]>; [email protected] On Mon, 7 Apr 2025 at 18:09, Álvaro Herrera <[email protected]> wrote: > > Here's the additional changes I made here before giving up on this. > I think it needs some additional rethinking, not going to happen for 18. > Hi Alvaro, Thanks for reviewing the patch. The changes shared by you in [1], look good to me and I have added it to the latest patch. I have also included the changes shared by you in [2]. I have also addressed the issue reported by Sergey in [3]. Additionally, I made some changes to the comments, commit message and documentation to reflect the changes in the function name and to reduce the usage of 'foreign partition' term. I saw some comments by you in the patches: 1. + /* + * Keep lock till end of transaction: must prevent this table from + * being attached a foreign table until we're done. XXX does this + * prevent addition of a partition in a partitioned child? + */ This is same as the issue reported by Sergey in [3]. I have addressed the issue in the latest patch. So, I have modified this comment. 2. + * We also take a ShareLock on pg_partitioned_table to restrict addition + * of new partitioned table which may contain a foreign partition while + * publication is being created. XXX this is quite weird actually. This change was added to resolve the concurrency issue shared by Vignesh in [4]. I tried with different locks and found that lock with severity >= ShareLock was needed to avoid the concurrency issue. Initially I added ShareLock to pg_class, but to reduce the scope I added it to pg_partitioned_table instead. I cannot think of an alternate approach. Do you have any suggestions for this? [1]: https://www.postgresql.org/message-id/202504062027.tqmabk2h353o%40alvherre.pgsql [2]: https://www.postgresql.org/message-id/202504071239.kuj6m5a5wqxg%40alvherre.pgsql [3]: https://www.postgresql.org/message-id/c64352fa-9a30-4e0a-853a-a6b5b6d07f4e%40postgrespro.ru [4]: https://www.postgresql.org/message-id/CALDaNm2%2BeL22Sbvj74uS37xvt%3DhaQWcOwP15QnDuVeYsjHiffw%40mail... Thanks and Regards, Shlok Kyal Attachments: [application/octet-stream] v13-0001-Restrict-publishing-of-partitioned-table-with-fo.patch (35.9K, ../../CANhcyEW0QMiJXMqpPFRHni-q0Rm4R0hpZ0LdaqA=F3wvDUU6sQ@mail.gmail.com/2-v13-0001-Restrict-publishing-of-partitioned-table-with-fo.patch) download | inline diff: From 534eee027b02e5cda9b502f9f624a5fb426d7f84 Mon Sep 17 00:00:00 2001 From: Shlok Kyal <[email protected]> Date: Fri, 28 Mar 2025 11:15:09 +0530 Subject: [PATCH v13] Restrict publishing of partitioned table with foreign table as its partition Logical replication of foreign table is not supported and we throw an error in this case. But when we create a publication on a partitioned table that has a foreign table as its partition, the initial sync of such table is successful and we should avoid such cases. Current Behaviour in HEAD, when publication is created: 1. with publish_via_partition_root = true The root table is published, and initial data from partitions that are foreign tables is replicated. 2. with publish_via_partition_root = false and FOR ALL TABLES All leaf tables except partitions that are foreign tables are published. 3. with publish_via_partition_root = false and FOR TABLE/ FOR TABLES IN SCHEMA All leaf tables are published, including initial data from partitions that are foreign tables. With this patch we have following behaviour: 1. with publish_via_partition_root = true We throw an error when we try to publish a partition that is a foreign table. Error is thrown when we try to create a publication on (or add to existing publication) a partitioned table that has foreign tables as its partitions, when try to create a partition that is a foreign table and when we try to attach foreign table (or a partitioned table with foreign tables as partitions) to existing published tables. 2. with publish_via_partition_root = false We skip publishing partitions that are foreign tables. This is done by avoid adding such partitions in pg_subscription_rel catalog table. We have introduced two functions 'RelationHasForeignPartition' and 'publication_check_foreign_parts'. In 'RelationHasForeignPartition' we go through the child nodes of a partition and check if it has a foreign table. While doing so, we take an AccessShareLock on each partition table to prevent concurrent creation of a foreign table as a partition. In 'publication_check_foreign_parts' if schema id is provided we check for each partitioned table in that schema if it has a foreign table as its partition, or if schema id is not provided we check for each partitioned table in the database if it has a partition that's a foreign table. While doing so we take a ShareLock on pg_partitioned_table so no partition table is created concurrently after this check. --- doc/src/sgml/logical-replication.sgml | 10 +- doc/src/sgml/ref/create_publication.sgml | 18 +- src/backend/catalog/pg_publication.c | 221 ++++++++++++++++++++-- src/backend/commands/publicationcmds.c | 47 +++++ src/backend/commands/tablecmds.c | 112 +++++++++++ src/backend/partitioning/partdesc.c | 32 ++++ src/include/catalog/pg_publication.h | 3 + src/include/partitioning/partdesc.h | 1 + src/test/regress/expected/publication.out | 99 ++++++++++ src/test/regress/sql/publication.sql | 90 +++++++++ 10 files changed, 611 insertions(+), 22 deletions(-) diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml index f288c049a5c..57236a452e9 100644 --- a/doc/src/sgml/logical-replication.sgml +++ b/doc/src/sgml/logical-replication.sgml @@ -2154,10 +2154,18 @@ CONTEXT: processing remote data for replication origin "pg_16395" during "INSER <listitem> <para> - Replication is only supported by tables, including partitioned tables. + Replication is only supported for tables, including partitioned tables. Attempts to replicate other types of relations, such as views, materialized views, or foreign tables, will result in an error. </para> + <para> + Replication is not supported for foreign tables. When used as partitions + of partitioned tables, publishing of the partitioned table is only allowed + if the <literal>publish_via_partition_root</literal> is set to + <literal>false</literal>. In this mode, changes to a partition that is a + foreign table are ignored for the purposes of replication, and data + contained in them is not included during initial synchronization. + </para> </listitem> <listitem> diff --git a/doc/src/sgml/ref/create_publication.sgml b/doc/src/sgml/ref/create_publication.sgml index 73f0c8d89fb..ad2c3b69d6b 100644 --- a/doc/src/sgml/ref/create_publication.sgml +++ b/doc/src/sgml/ref/create_publication.sgml @@ -249,13 +249,23 @@ CREATE PUBLICATION <replaceable class="parameter">name</replaceable> </para> <para> - This parameter also affects how row filters and column lists are - chosen for partitions; see below for details. + If this parameter is enabled, <literal>TRUNCATE</literal> + operations performed directly on partitions are not replicated. </para> <para> - If this is enabled, <literal>TRUNCATE</literal> operations performed - directly on partitions are not replicated. + If this parameter is enabled, foreign tables and partitioned tables + containing partitions that are foreign tables may not be + added to the publication. Conversely, foreign tables may not be + attached to a partitioned table that is included in a publication + with this parameter enabled. Lastly, this parameter may not be + changed on publications that include partitioned tables with foreign + tables as partitions. + </para> + + <para> + This parameter also affects how row filters and column lists are + chosen for partitions; see below for details. </para> </listitem> </varlistentry> diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c index d6f94db5d99..73a9bf22470 100644 --- a/src/backend/catalog/pg_publication.c +++ b/src/backend/catalog/pg_publication.c @@ -26,12 +26,15 @@ #include "catalog/partition.h" #include "catalog/pg_inherits.h" #include "catalog/pg_namespace.h" +#include "catalog/pg_partitioned_table.h" #include "catalog/pg_publication.h" #include "catalog/pg_publication_namespace.h" #include "catalog/pg_publication_rel.h" #include "catalog/pg_type.h" #include "commands/publicationcmds.h" #include "funcapi.h" +#include "partitioning/partdesc.h" +#include "storage/lmgr.h" #include "utils/array.h" #include "utils/builtins.h" #include "utils/catcache.h" @@ -53,7 +56,7 @@ typedef struct * error if not. */ static void -check_publication_add_relation(Relation targetrel) +check_publication_add_relation(Publication *pub, Relation targetrel) { /* Must be a regular or partitioned table */ if (RelationGetForm(targetrel)->relkind != RELKIND_RELATION && @@ -64,6 +67,21 @@ check_publication_add_relation(Relation targetrel) RelationGetRelationName(targetrel)), errdetail_relkind_not_supported(RelationGetForm(targetrel)->relkind))); + /* + * publish_via_root_partition cannot be true if it is a partitioned table + * and has any partition that's a foreign table. See + * publication_check_foreign_parts for details. + */ + if (pub->pubviaroot && + targetrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE && + RelationHasForeignPartition(targetrel)) + ereport(ERROR, + errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("cannot add partitioned table \"%s\" to publication \"%s\", which has \"%s\" set to \"%s\"", + RelationGetRelationName(targetrel), pub->name, + "publish_via_partition_root", "true"), + errdetail("The table contains a partition that's a foreign table.")); + /* Can't be system table */ if (IsCatalogRelation(targetrel)) ereport(ERROR, @@ -111,6 +129,172 @@ check_publication_add_schema(Oid schemaid) errdetail("Temporary schemas cannot be replicated."))); } +/* + * Returns true if the ancestor is in the list of relations + * Otherwise, returns false. + */ +static bool +is_ancestor_member_relids(Oid ancestor, List *relids) +{ + foreach_oid(relid, relids) + { + if (relid == ancestor) + return true; + } + + return false; +} + +/* + * Filter out the partitions whose parent tables are also present in the list + * of relations. + */ +static void +filter_partition_rels(List *relids) +{ + foreach_oid(relid, relids) + { + bool skip = false; + List *ancestors = NIL; + ListCell *lc2; + + if (get_rel_relispartition(relid)) + ancestors = get_partition_ancestors(relid); + + foreach_oid(ancestor, ancestors) + { + if (is_ancestor_member_relids(ancestor, relids)) + { + skip = true; + break; + } + } + + if (skip) + relids = list_delete_oid(relids, relid); + } +} + +/* + * publication_check_foreign_parts + * Helper function to ensure we don't indirectly publish foreign tables + * + * DML data changes are not published for data in foreign tables, + * and yet the tablesync worker is not smart enough to omit data from + * foreign tables when they are partitions of partitioned tables. To + * avoid the inconsistencies that would result, we disallow foreign + * tables from being published generally. However, it's possible for + * partitioned tables to have foreign tables as partitions, and we would + * like to allow publishing those partitioned tables so that the other + * partitions are replicated. + * + * This function is in charge of detecting if a partitioned table has a + * foreign table as a partition -- either in the whole database (useful + * for FOR ALL TABLES publications) or in a particular schema (useful + * for FOR TABLES IN SCHEMA publications). This function must be called + * only for publications with publish_via_partition_root=true. + * + * When publish_via_partition_root is false, each partition published for + * replication is listed individually in pg_subscription_rel, and we + * don't add partitions that are foreign tables, so this check is not + * needed. + * + * If a valid schemaid is provided, check if that schema has any + * partitioned table with a foreign table as partition. + * + * If no valid schemaid is provided, check all partitioned tables. + * + * We take a lock on partition tables so no new foreign table are added + * concurrently as a partition. + * + * We also take a ShareLock on pg_partitioned_table to restrict addition + * of new partitioned table which may include a foreign table as partition + * while publication is being created. XXX this is quite weird actually. + */ +void +publication_check_foreign_parts(Oid schemaid, char *pubname) +{ + Relation classRel; + ScanKeyData key[3]; + int keycount = 0; + TableScanDesc scan; + HeapTuple tuple; + List *relids = NIL; + + /* + * Take lock on pg_partitioned_rel. This prevents new publications from + * being created. + */ + LockRelationOid(PartitionedRelationId, ShareLock); + + classRel = table_open(RelationRelationId, AccessShareLock); + + /* Get the root nodes of partitioned table */ + ScanKeyInit(&key[keycount++], + Anum_pg_class_relkind, + BTEqualStrategyNumber, F_CHAREQ, + CharGetDatum(RELKIND_PARTITIONED_TABLE)); + + /* If schema id is provided check partitioned table in that schema */ + if (OidIsValid(schemaid)) + ScanKeyInit(&key[keycount++], + Anum_pg_class_relnamespace, + BTEqualStrategyNumber, F_OIDEQ, + schemaid); + + /* + * If schema id is not provided, take relations for which relispartition + * is false. This will give only the root partitioned tables. We donot + * include this for the case when schema id is specified because there can + * be cases when root partitioned tables are not part of schema and one of + * the child partition can still have a foreign table as its partition. + */ + else + ScanKeyInit(&key[keycount++], + Anum_pg_class_relispartition, + BTEqualStrategyNumber, F_BOOLEQ, + BoolGetDatum(false)); + + scan = table_beginscan_catalog(classRel, keycount, key); + while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL) + { + Form_pg_class relForm = (Form_pg_class) GETSTRUCT(tuple); + + relids = lappend_oid(relids, relForm->oid); + } + + /* + * If schema id is provided filter partitions list to have only topmost + * partitioned tables in that schema, to avoid repeated check. + */ + if (OidIsValid(schemaid)) + filter_partition_rels(relids); + + foreach_oid(relid, relids) + { + Relation pubrel = table_open(relid, AccessShareLock); + + if (pubrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE && + RelationHasForeignPartition(pubrel)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("cannot set parameter \"%s\" to \"%s\" for publication \"%s\"", + "publish_via_partition_root", "true", pubname), + errtable(pubrel), + errdetail("Published partitioned table \"%s\" contains a partition that is a foreign table.", + get_rel_name(relid)))); + + /* + * Keep lock till end of transaction: must prevent this table from + * being attached a foreign table until we're done. + */ + table_close(pubrel, NoLock); + } + + table_endscan(scan); + table_close(classRel, AccessShareLock); +} + /* * Returns if relation represented by oid and Form_pg_class entry * is publishable. @@ -304,7 +488,7 @@ check_and_fetch_column_list(Publication *pub, Oid relid, MemoryContext mcxt, /* * Gets the relations based on the publication partition option for a specified - * relation. + * relation. Foreign tables are not included. */ List * GetPubPartitionOptionRelations(List *result, PublicationPartOpt pub_partopt, @@ -313,25 +497,21 @@ GetPubPartitionOptionRelations(List *result, PublicationPartOpt pub_partopt, if (get_rel_relkind(relid) == RELKIND_PARTITIONED_TABLE && pub_partopt != PUBLICATION_PART_ROOT) { - List *all_parts = find_all_inheritors(relid, NoLock, - NULL); + List *all_parts = find_all_inheritors(relid, NoLock, NULL); - if (pub_partopt == PUBLICATION_PART_ALL) - result = list_concat(result, all_parts); - else if (pub_partopt == PUBLICATION_PART_LEAF) + foreach_oid(partOid, all_parts) { - ListCell *lc; + char relkind = get_rel_relkind(partOid); - foreach(lc, all_parts) - { - Oid partOid = lfirst_oid(lc); + if (relkind == RELKIND_FOREIGN_TABLE) + continue; - if (get_rel_relkind(partOid) != RELKIND_PARTITIONED_TABLE) - result = lappend_oid(result, partOid); - } + if (pub_partopt == PUBLICATION_PART_LEAF && + relkind == RELKIND_PARTITIONED_TABLE) + continue; + + result = lappend_oid(result, partOid); } - else - Assert(false); } else result = lappend_oid(result, relid); @@ -463,7 +643,7 @@ publication_add_relation(Oid pubid, PublicationRelInfo *pri, RelationGetRelationName(targetrel), pub->name))); } - check_publication_add_relation(targetrel); + check_publication_add_relation(pub, targetrel); /* Validate and translate column names into a Bitmapset of attnums. */ attnums = pub_collist_validate(pri->relation, pri->columns); @@ -703,6 +883,13 @@ publication_add_schema(Oid pubid, Oid schemaid, bool if_not_exists) check_publication_add_schema(schemaid); + /* + * If publish_via_partition_root is true, check if schema has any foreign + * partition + */ + if (pub->pubviaroot) + publication_check_foreign_parts(schemaid, pub->name); + /* Form a tuple */ memset(values, 0, sizeof(values)); memset(nulls, false, sizeof(nulls)); diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index 0b23d94c38e..a2411fbebda 100644 --- a/src/backend/commands/publicationcmds.c +++ b/src/backend/commands/publicationcmds.c @@ -35,6 +35,7 @@ #include "commands/publicationcmds.h" #include "miscadmin.h" #include "nodes/nodeFuncs.h" +#include "partitioning/partdesc.h" #include "parser/parse_clause.h" #include "parser/parse_collate.h" #include "parser/parse_relation.h" @@ -915,6 +916,12 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt) /* Associate objects with the publication. */ if (stmt->for_all_tables) { + /* + * Check if any partitioned table has a foreign table as its partition + */ + if (publish_via_partition_root) + publication_check_foreign_parts(InvalidOid, stmt->pubname); + /* Invalidate relcache so that publication info is rebuilt. */ CacheInvalidateRelcacheAll(); } @@ -1080,6 +1087,46 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt, } } + /* + * If publish_via_partition_root is set to true, check if the publication + * has any partition that's a foreign table. See + * publication_check_foreign_parts for details. + */ + if (publish_via_partition_root_given && publish_via_partition_root) + { + char *pubname = stmt->pubname; + List *schemaoids; + List *relids; + + if (pubform->puballtables) + publication_check_foreign_parts(InvalidOid, pubname); + + schemaoids = GetPublicationSchemas(pubform->oid); + foreach_oid(schemaoid, schemaoids) + publication_check_foreign_parts(schemaoid, pubname); + + relids = GetPublicationRelations(pubform->oid, PUBLICATION_PART_ROOT); + foreach_oid(relid, relids) + { + Relation pubrel; + + if (get_rel_relkind(relid) != RELKIND_PARTITIONED_TABLE) + continue; + + pubrel = table_open(relid, AccessShareLock); + + if (RelationHasForeignPartition(pubrel)) + ereport(ERROR, + errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("cannot set parameter \"%s\" to \"%s\" for publication \"%s\"", + "publish_via_partition_root", "true", pubname), + errdetail("Published partitioned table \"%s\" contains a partition that is a foreign table.", + RelationGetRelationName(pubrel))); + + table_close(pubrel, NoLock); + } + } + /* Everything ok, form a new tuple. */ memset(values, 0, sizeof(values)); memset(nulls, false, sizeof(nulls)); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 2705cf11330..3587e9960d8 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1133,6 +1133,53 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId, errmsg("\"%s\" is not partitioned", RelationGetRelationName(parent)))); + /* + * If we're creating a partition that's a foreign table, verify that + * the parent table is not in a publication with + * publish_via_partition_root enabled. For details, see + * publication_check_foreign_parts. + */ + if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE) + { + Oid schemaid; + List *puboids; + List *ancestors; + + /* Start with publications of all tables */ + puboids = GetAllTablesPublications(); + + /* capture all publications that include this relation directly */ + puboids = GetRelationPublications(parent->rd_id); + schemaid = RelationGetNamespace(parent); + puboids = list_concat(puboids, GetSchemaPublications(schemaid)); + + /* and do the same for its ancestors, if any */ + ancestors = get_partition_ancestors(parent->rd_id); + foreach_oid(ancestor, ancestors) + { + puboids = list_concat(puboids, GetRelationPublications(ancestor)); + schemaid = get_rel_namespace(ancestor); + puboids = list_concat(puboids, GetSchemaPublications(schemaid)); + } + + /* Check the publish_via_partition_root bit for each of those */ + list_sort(puboids, list_oid_cmp); + list_deduplicate_oid(puboids); + foreach_oid(puboid, puboids) + { + Publication *pub = GetPublication(puboid); + + if (pub->pubviaroot) + ereport(ERROR, + errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("cannot create foreign table \"%s\" as a partition of \"%s\"", + RelationGetRelationName(rel), RelationGetRelationName(parent)), + errdetail("Partitioned table \"%s\" is published with option \"%s\" in publication \"%s\".", + RelationGetRelationName(parent), + "publish_via_partition_root", pub->name)); + } + } + /* * The partition constraint of the default partition depends on the * partition bounds of every other partition. It is possible that @@ -20304,6 +20351,71 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("cannot attach temporary relation of another session as partition"))); + /* + * If the relation to attach is a foreign table, or a partitioned table + * that contains a foreign table as partition, then verify that the parent + * table is not in a publication with publish_via_partition_root enabled. + * See publication_check_foreign_parts. + */ + if (attachrel->rd_rel->relkind == RELKIND_FOREIGN_TABLE || + (attachrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE && + RelationHasForeignPartition(attachrel))) + { + Oid schemaid; + List *puboids; + List *ancestors; + + /* Start with publications of all tables */ + puboids = GetAllTablesPublications(); + + /* capture all publications that include this relation directly */ + puboids = list_concat(puboids, GetRelationPublications(rel->rd_id)); + schemaid = RelationGetNamespace(rel); + puboids = list_concat(puboids, GetSchemaPublications(schemaid)); + + /* and do the same for its ancestors, if any */ + ancestors = get_partition_ancestors(rel->rd_id); + foreach_oid(ancestor, ancestors) + { + puboids = list_concat(puboids, GetRelationPublications(ancestor)); + schemaid = get_rel_namespace(ancestor); + puboids = list_concat(puboids, GetSchemaPublications(schemaid)); + } + + /* Now check the publish_via_partition_root bit for each of those */ + list_sort(puboids, list_oid_cmp); + list_deduplicate_oid(puboids); + foreach_oid(puboid, puboids) + { + Publication *pub; + + pub = GetPublication(puboid); + if (pub->pubviaroot) + { + if (attachrel->rd_rel->relkind == RELKIND_FOREIGN_TABLE) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("cannot attach foreign table \"%s\" to partition table \"%s\"", + RelationGetRelationName(attachrel), + RelationGetRelationName(rel)), + errdetail("Partitioned table \"%s\" is published with option \"%s\" in publication \"%s\".", + RelationGetRelationName(rel), + "publish_via_partition_root", + pub->name))); + else + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("cannot attach table \"%s\" with a partition that's a foreign table to partition table \"%s\"", + RelationGetRelationName(attachrel), + RelationGetRelationName(rel)), + errdetail("Partitioned table \"%s\" is published with option \"%s\" in publication \"%s\".", + RelationGetRelationName(rel), + "publish_via_partition_root", + pub->name))); + } + } + } + /* * Check if attachrel has any identity columns or any columns that aren't * in the parent. diff --git a/src/backend/partitioning/partdesc.c b/src/backend/partitioning/partdesc.c index 328b4d450e4..b53139bafdd 100644 --- a/src/backend/partitioning/partdesc.c +++ b/src/backend/partitioning/partdesc.c @@ -506,3 +506,35 @@ get_default_oid_from_partdesc(PartitionDesc partdesc) return InvalidOid; } + +/* + * Return true if the given partitioned table ultimately contains a + * partition that is a foreign table, false otherwise. + */ +bool +RelationHasForeignPartition(Relation rel) +{ + PartitionDesc pd = RelationGetPartitionDesc(rel, true); + + for (int i = 0; i < pd->nparts; i++) + { + if (pd->is_leaf[i]) + { + if (get_rel_relkind(pd->oids[i]) == RELKIND_FOREIGN_TABLE) + return true; + } + else + { + Relation part; + bool ret; + + part = table_open(pd->oids[i], AccessShareLock); + ret = RelationHasForeignPartition(part); + table_close(part, NoLock); + if (ret) + return true; + } + } + + return false; +} diff --git a/src/include/catalog/pg_publication.h b/src/include/catalog/pg_publication.h index 48c7d1a8615..71ad5a6f846 100644 --- a/src/include/catalog/pg_publication.h +++ b/src/include/catalog/pg_publication.h @@ -19,6 +19,7 @@ #include "catalog/genbki.h" #include "catalog/objectaddress.h" +#include "catalog/pg_class.h" #include "catalog/pg_publication_d.h" /* IWYU pragma: export */ /* ---------------- @@ -178,6 +179,8 @@ extern Oid GetTopMostAncestorInPublication(Oid puboid, List *ancestors, extern bool is_publishable_relation(Relation rel); extern bool is_schema_publication(Oid pubid); +extern void publication_check_foreign_parts(Oid schemaid, char *pubname); + extern bool check_and_fetch_column_list(Publication *pub, Oid relid, MemoryContext mcxt, Bitmapset **cols); extern ObjectAddress publication_add_relation(Oid pubid, PublicationRelInfo *pri, diff --git a/src/include/partitioning/partdesc.h b/src/include/partitioning/partdesc.h index 34533f7004c..5fbafdc06f9 100644 --- a/src/include/partitioning/partdesc.h +++ b/src/include/partitioning/partdesc.h @@ -71,5 +71,6 @@ extern PartitionDesc PartitionDirectoryLookup(PartitionDirectory, Relation); extern void DestroyPartitionDirectory(PartitionDirectory pdir); extern Oid get_default_oid_from_partdesc(PartitionDesc partdesc); +extern bool RelationHasForeignPartition(Relation rel); #endif /* PARTDESC_H */ diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out index 4de96c04f9d..f57addab23d 100644 --- a/src/test/regress/expected/publication.out +++ b/src/test/regress/expected/publication.out @@ -1924,6 +1924,105 @@ DROP PUBLICATION pub1; DROP PUBLICATION pub2; DROP TABLE gencols; RESET client_min_messages; +-- ====================================================== +-- Test when foreign table is a partition of a partitioned table on which +-- publication is created +SET client_min_messages = 'ERROR'; +CREATE FOREIGN DATA WRAPPER test_fdw; +CREATE SERVER fdw_server FOREIGN DATA WRAPPER test_fdw; +CREATE SCHEMA sch3; +CREATE TABLE sch3.tmain(id int) PARTITION BY RANGE(id); +CREATE TABLE sch3.part1 PARTITION OF sch3.tmain FOR VALUES FROM (0) TO (5); +CREATE TABLE sch3.part2(id int) PARTITION BY RANGE(id); +CREATE FOREIGN TABLE sch3.part2_1 PARTITION OF sch3.part2 FOR VALUES FROM (5) TO (10) SERVER fdw_server; +ALTER TABLE sch3.tmain ATTACH PARTITION sch3.part2 FOR VALUES FROM (5) TO (10); +-- Can't create publications with publish_via_partition_root = true, if table +-- has a partition that's a foreign table +CREATE PUBLICATION pub1 FOR TABLE sch3.tmain WITH (publish_via_partition_root); +ERROR: cannot add partitioned table "tmain" to publication "pub1", which has "publish_via_partition_root" set to "true" +DETAIL: The table contains a partition that's a foreign table. +CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA sch3 WITH (publish_via_partition_root); +ERROR: cannot set parameter "publish_via_partition_root" to "true" for publication "pub1" +DETAIL: Published partitioned table "tmain" contains a partition that is a foreign table. +CREATE PUBLICATION pub1 FOR ALL TABLES WITH (publish_via_partition_root); +ERROR: cannot set parameter "publish_via_partition_root" to "true" for publication "pub1" +DETAIL: Published partitioned table "tmain" contains a partition that is a foreign table. +-- Test when a partitioned table with foreign table as a partition is attached +-- to partitioned table which is already published +ALTER TABLE sch3.tmain DETACH PARTITION sch3.part2; +CREATE PUBLICATION pub1 FOR TABLE sch3.tmain WITH (publish_via_partition_root); +ALTER TABLE sch3.tmain ATTACH PARTITION sch3.part2 FOR VALUES FROM (5) TO (10); +ERROR: cannot attach table "part2" with a partition that's a foreign table to partition table "tmain" +DETAIL: Partitioned table "tmain" is published with option "publish_via_partition_root" in publication "pub1". +-- Can't create a foreign table that is partition of table published with +-- publish_via_partition_root = true +CREATE FOREIGN TABLE sch3.part3_1 PARTITION OF sch3.tmain FOR VALUES FROM (10) TO (15) SERVER fdw_server; +ERROR: cannot create foreign table "part3_1" as a partition of "tmain" +DETAIL: Partitioned table "tmain" is published with option "publish_via_partition_root" in publication "pub1". +-- Can't attach a foreign table as partition to table published with +-- publish_via_partition_root = true +CREATE FOREIGN TABLE sch3.part3_2(id int) SERVER fdw_server; +ALTER TABLE sch3.tmain ATTACH PARTITION sch3.part3_2 FOR VALUES FROM (15) TO (20); +ERROR: cannot attach foreign table "part3_2" to partition table "tmain" +DETAIL: Partitioned table "tmain" is published with option "publish_via_partition_root" in publication "pub1". +CREATE SCHEMA sch4; +CREATE TABLE sch4.tmain(id int) PARTITION BY RANGE(id); +-- publication created with FOR TABLES IN SCHEMA +DROP PUBLICATION pub1; +CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA sch4 WITH (publish_via_partition_root); +-- Can't create a foreign table that is partition of table published with +-- publish_via_partition_root = true +CREATE FOREIGN TABLE sch3.part3_1 PARTITION OF sch4.tmain FOR VALUES FROM (10) TO (15) SERVER fdw_server; +ERROR: cannot create foreign table "part3_1" as a partition of "tmain" +DETAIL: Partitioned table "tmain" is published with option "publish_via_partition_root" in publication "pub1". +-- Can't attach a foreign table as partition to table published with +-- publish_via_partition_root = true +ALTER TABLE sch4.tmain ATTACH PARTITION sch3.part3_2 FOR VALUES FROM (15) TO (20); +ERROR: cannot attach foreign table "part3_2" to partition table "tmain" +DETAIL: Partitioned table "tmain" is published with option "publish_via_partition_root" in publication "pub1". +DROP PUBLICATION pub1; +-- Can't create publication with publish_via_partition_root = true on +-- partitioned table(which is not root) with a partition that's a foreign table +-- on other schema +CREATE SCHEMA sch5; +CREATE TABLE sch5.part1 PARTITION OF sch4.tmain FOR VALUES FROM (0) TO (10) PARTITION BY RANGE(id); +CREATE FOREIGN TABLE sch4.part1_1 PARTITION OF sch5.part1 FOR VALUES FROM (0) TO (5) SERVER fdw_server; +CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA sch5 WITH (publish_via_partition_root); +ERROR: cannot set parameter "publish_via_partition_root" to "true" for publication "pub1" +DETAIL: Published partitioned table "part1" contains a partition that is a foreign table. +-- Test with publish_via_partition_root = false +-- Partition that are foreign tables are skipped by default +CREATE PUBLICATION pub1 FOR TABLE sch3.tmain; +CREATE PUBLICATION pub2 FOR TABLES IN SCHEMA sch3; +CREATE PUBLICATION pub3 FOR ALL TABLES; +-- Create a partition that's a foreign table of published table with +-- publish_via_partition_root = false +CREATE FOREIGN TABLE sch3.part3_1 PARTITION OF sch3.tmain FOR VALUES FROM (10) TO (15) SERVER fdw_server; +-- Attach partition that's a foreign table to published table +-- publish_via_partition_root = false +ALTER TABLE sch3.tmain ATTACH PARTITION sch3.part3_2 FOR VALUES FROM (15) TO (20); +-- Check the published tables +SELECT pubname, tablename FROM pg_publication_tables WHERE schemaname in ('sch3', 'sch4') ORDER BY pubname, tablename; + pubname | tablename +---------+----------- + pub1 | part1 + pub2 | part1 + pub3 | part1 +(3 rows) + +-- Can't alter publish_via_partition_root to true, if publication already have +-- a partition that's a foreign table +ALTER PUBLICATION pub1 SET (publish_via_partition_root); +ERROR: cannot set parameter "publish_via_partition_root" to "true" for publication "pub1" +DETAIL: Published partitioned table "tmain" contains a partition that is a foreign table. +DROP PUBLICATION pub1; +DROP PUBLICATION pub2; +DROP PUBLICATION pub3; +DROP SCHEMA sch3 CASCADE; +DROP SCHEMA sch4 CASCADE; +DROP SCHEMA sch5 CASCADE; +DROP SERVER fdw_server; +DROP FOREIGN DATA WRAPPER test_fdw; RESET SESSION AUTHORIZATION; DROP ROLE regress_publication_user, regress_publication_user2; DROP ROLE regress_publication_user_dummy; diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql index 68001de4000..55c0dff3601 100644 --- a/src/test/regress/sql/publication.sql +++ b/src/test/regress/sql/publication.sql @@ -1223,6 +1223,96 @@ DROP PUBLICATION pub2; DROP TABLE gencols; RESET client_min_messages; +-- ====================================================== + +-- Test when foreign table is a partition of a partitioned table on which +-- publication is created +SET client_min_messages = 'ERROR'; +CREATE FOREIGN DATA WRAPPER test_fdw; +CREATE SERVER fdw_server FOREIGN DATA WRAPPER test_fdw; + +CREATE SCHEMA sch3; +CREATE TABLE sch3.tmain(id int) PARTITION BY RANGE(id); +CREATE TABLE sch3.part1 PARTITION OF sch3.tmain FOR VALUES FROM (0) TO (5); +CREATE TABLE sch3.part2(id int) PARTITION BY RANGE(id); +CREATE FOREIGN TABLE sch3.part2_1 PARTITION OF sch3.part2 FOR VALUES FROM (5) TO (10) SERVER fdw_server; +ALTER TABLE sch3.tmain ATTACH PARTITION sch3.part2 FOR VALUES FROM (5) TO (10); + +-- Can't create publications with publish_via_partition_root = true, if table +-- has a partition that's a foreign table +CREATE PUBLICATION pub1 FOR TABLE sch3.tmain WITH (publish_via_partition_root); +CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA sch3 WITH (publish_via_partition_root); +CREATE PUBLICATION pub1 FOR ALL TABLES WITH (publish_via_partition_root); + +-- Test when a partitioned table with foreign table as a partition is attached +-- to partitioned table which is already published +ALTER TABLE sch3.tmain DETACH PARTITION sch3.part2; +CREATE PUBLICATION pub1 FOR TABLE sch3.tmain WITH (publish_via_partition_root); +ALTER TABLE sch3.tmain ATTACH PARTITION sch3.part2 FOR VALUES FROM (5) TO (10); + +-- Can't create a foreign table that is partition of table published with +-- publish_via_partition_root = true +CREATE FOREIGN TABLE sch3.part3_1 PARTITION OF sch3.tmain FOR VALUES FROM (10) TO (15) SERVER fdw_server; + +-- Can't attach a foreign table as partition to table published with +-- publish_via_partition_root = true +CREATE FOREIGN TABLE sch3.part3_2(id int) SERVER fdw_server; +ALTER TABLE sch3.tmain ATTACH PARTITION sch3.part3_2 FOR VALUES FROM (15) TO (20); + +CREATE SCHEMA sch4; +CREATE TABLE sch4.tmain(id int) PARTITION BY RANGE(id); + +-- publication created with FOR TABLES IN SCHEMA +DROP PUBLICATION pub1; +CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA sch4 WITH (publish_via_partition_root); + +-- Can't create a foreign table that is partition of table published with +-- publish_via_partition_root = true +CREATE FOREIGN TABLE sch3.part3_1 PARTITION OF sch4.tmain FOR VALUES FROM (10) TO (15) SERVER fdw_server; + +-- Can't attach a foreign table as partition to table published with +-- publish_via_partition_root = true +ALTER TABLE sch4.tmain ATTACH PARTITION sch3.part3_2 FOR VALUES FROM (15) TO (20); +DROP PUBLICATION pub1; + +-- Can't create publication with publish_via_partition_root = true on +-- partitioned table(which is not root) with a partition that's a foreign table +-- on other schema +CREATE SCHEMA sch5; +CREATE TABLE sch5.part1 PARTITION OF sch4.tmain FOR VALUES FROM (0) TO (10) PARTITION BY RANGE(id); +CREATE FOREIGN TABLE sch4.part1_1 PARTITION OF sch5.part1 FOR VALUES FROM (0) TO (5) SERVER fdw_server; +CREATE PUBLICATION pub1 FOR TABLES IN SCHEMA sch5 WITH (publish_via_partition_root); + +-- Test with publish_via_partition_root = false +-- Partition that are foreign tables are skipped by default +CREATE PUBLICATION pub1 FOR TABLE sch3.tmain; +CREATE PUBLICATION pub2 FOR TABLES IN SCHEMA sch3; +CREATE PUBLICATION pub3 FOR ALL TABLES; + +-- Create a partition that's a foreign table of published table with +-- publish_via_partition_root = false +CREATE FOREIGN TABLE sch3.part3_1 PARTITION OF sch3.tmain FOR VALUES FROM (10) TO (15) SERVER fdw_server; + +-- Attach partition that's a foreign table to published table +-- publish_via_partition_root = false +ALTER TABLE sch3.tmain ATTACH PARTITION sch3.part3_2 FOR VALUES FROM (15) TO (20); + +-- Check the published tables +SELECT pubname, tablename FROM pg_publication_tables WHERE schemaname in ('sch3', 'sch4') ORDER BY pubname, tablename; + +-- Can't alter publish_via_partition_root to true, if publication already have +-- a partition that's a foreign table +ALTER PUBLICATION pub1 SET (publish_via_partition_root); + +DROP PUBLICATION pub1; +DROP PUBLICATION pub2; +DROP PUBLICATION pub3; +DROP SCHEMA sch3 CASCADE; +DROP SCHEMA sch4 CASCADE; +DROP SCHEMA sch5 CASCADE; +DROP SERVER fdw_server; +DROP FOREIGN DATA WRAPPER test_fdw; + RESET SESSION AUTHORIZATION; DROP ROLE regress_publication_user, regress_publication_user2; DROP ROLE regress_publication_user_dummy; -- 2.34.1 ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2025-04-28 12:48 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-06-06 22:42 [PATCH v5 2/3] Allow CREATE INDEX CONCURRENTLY on partitioned table Justin Pryzby <[email protected]> 2025-04-07 12:39 Re: Restrict publishing of partitioned table with a foreign table as partition Álvaro Herrera <[email protected]> 2025-04-28 12:48 ` Re: Restrict publishing of partitioned table with a foreign table as partition Shlok Kyal <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox