Re: Restrict publishing of partitioned table with a foreign table as partition

On Thu, 13 Feb 2025 at 20:12, vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Thu, 13 Feb 2025 at 15:50, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> 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.
>

Hi Vignesh,

I have used the changes suggested by you. Also I have updated the
comments and the function name.

Thanks and Regards,
Shlok Kyal