On Thu, 13 Feb 2025 at 15:50, Shlok Kyal <[email protected]> wrote:
>
>
> I have fixed the issue. Attached the updated v6 patch.
There is another concurrency issue:
In case of create publication for all tables with
publish_via_partition_root we will call check_foreign_tables:
@@ -876,6 +876,10 @@ CreatePublication(ParseState *pstate,
CreatePublicationStmt *stmt)
/* Associate objects with the publication. */
if (stmt->for_all_tables)
{
+ /* Check if any foreign table is a part of partitioned table */
+ if (publish_via_partition_root)
+ check_foreign_tables(stmt->pubname);
At the time of check in check_foreign_tables, there are no foreign
tables so this check will be successful:
+check_foreign_tables_in_schema(Oid schemaid, char *pubname)
+{
+ Relation classRel;
+ ScanKeyData key[2];
+ TableScanDesc scan;
+ HeapTuple tuple;
+
+ classRel = table_open(RelationRelationId, AccessShareLock);
+
+ ScanKeyInit(&key[0],
+ Anum_pg_class_relnamespace,
+ BTEqualStrategyNumber, F_OIDEQ,
+ schemaid);
+ ScanKeyInit(&key[1],
+ Anum_pg_class_relkind,
+ BTEqualStrategyNumber, F_CHAREQ,
+ CharGetDatum(RELKIND_PARTITIONED_TABLE));
+
+ scan = table_beginscan_catalog(classRel, 2, key);
+ while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
Now immediately after execution of this, create a foreign table:
postgres=# CREATE FOREIGN TABLE part22 PARTITION OF part2 FOR VALUES
FROM (10) TO (15) SERVER fdw;
CREATE FOREIGN TABLE
And then continue execution of create publication, it will also be successful:
postgres=# create publication pub1 for all tables with (
publish_via_partition_root =true);
CREATE PUBLICATION
One probable way to fix this is to do the search similar to
check_foreign_tables_in_schema where we can skip including schemaid
key for all tables.
How about something like the attached patch.
Regards,
Vignesh
diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
index 520fa2f382..3214104dec 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -1388,21 +1388,24 @@ check_foreign_tables_in_schema(Oid schemaid, char *pubname)
{
Relation classRel;
ScanKeyData key[2];
+ int keycount = 0;
TableScanDesc scan;
HeapTuple tuple;
classRel = table_open(RelationRelationId, AccessShareLock);
- ScanKeyInit(&key[0],
- Anum_pg_class_relnamespace,
- BTEqualStrategyNumber, F_OIDEQ,
- schemaid);
- ScanKeyInit(&key[1],
+ ScanKeyInit(&key[keycount++],
Anum_pg_class_relkind,
BTEqualStrategyNumber, F_CHAREQ,
CharGetDatum(RELKIND_PARTITIONED_TABLE));
-
- scan = table_beginscan_catalog(classRel, 2, key);
+
+ if (OidIsValid(schemaid))
+ ScanKeyInit(&key[keycount++],
+ Anum_pg_class_relnamespace,
+ BTEqualStrategyNumber, F_OIDEQ,
+ schemaid);
+
+ scan = table_beginscan_catalog(classRel, keycount, key);
while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
{
Form_pg_class relForm = (Form_pg_class) GETSTRUCT(tuple);
@@ -1436,46 +1439,3 @@ check_foreign_tables_in_schema(Oid schemaid, char *pubname)
table_endscan(scan);
table_close(classRel, AccessShareLock);
}
-
-/* Check if any foreign table is a partition table */
-void
-check_foreign_tables(char *pubname)
-{
- Relation classRel;
- ScanKeyData key[1];
- TableScanDesc scan;
- HeapTuple tuple;
-
- classRel = table_open(RelationRelationId, AccessShareLock);
-
- ScanKeyInit(&key[0],
- Anum_pg_class_relkind,
- BTEqualStrategyNumber, F_CHAREQ,
- CharGetDatum(RELKIND_FOREIGN_TABLE));
-
- scan = table_beginscan_catalog(classRel, 1, key);
- while ((tuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
- {
- Form_pg_class relForm = (Form_pg_class) GETSTRUCT(tuple);
-
- if (relForm->relispartition)
- {
- Oid parent_oid;
- char *parent_name;
- List *ancestors = get_partition_ancestors(relForm->oid);
-
- parent_oid = llast_oid(ancestors);
- parent_name = get_rel_name(parent_oid);
-
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("cannot set parameter \"%s\" to true for publication \"%s\"",
- "publish_via_partition_root", pubname),
- errdetail("partition table \"%s\" in publication contains a foreign partition",
- parent_name)));
- }
- }
-
- table_endscan(scan);
- table_close(classRel, AccessShareLock);
-}
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index e71d5408c7..b8da4ed130 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -878,7 +878,7 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt)
{
/* Check if any foreign table is a part of partitioned table */
if (publish_via_partition_root)
- check_foreign_tables(stmt->pubname);
+ check_foreign_tables_in_schema(InvalidOid, stmt->pubname);
/* Invalidate relcache so that publication info is rebuilt. */
CacheInvalidateRelcacheAll();
@@ -1057,7 +1057,7 @@ AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt,
char *pubname = stmt->pubname;
if (pubform->puballtables)
- check_foreign_tables(pubname);
+ check_foreign_tables_in_schema(InvalidOid, pubname);
schemaoids = GetPublicationSchemas(pubform->oid);
diff --git a/src/include/catalog/pg_publication.h b/src/include/catalog/pg_publication.h
index b1bb864d2f..fcd428465e 100644
--- a/src/include/catalog/pg_publication.h
+++ b/src/include/catalog/pg_publication.h
@@ -196,6 +196,4 @@ extern bool check_partrel_has_foreign_table(Form_pg_class relform);
extern void check_foreign_tables_in_schema(Oid schemaid, char *pubname);
-extern void check_foreign_tables(char *pubname);
-
#endif /* PG_PUBLICATION_H */
