Re: Added schema level support for publication.

Hi,

On Mon, Oct 18, 2021 at 3:14 PM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Saturday, October 16, 2021 1:57 PM houzj(dot)fnst(at)fujitsu(dot)com wrote:
> > Based on the V40 patchset, attaching the Top-up patch which try to fix the
> > partition issue in a cleaner way.
>
> Attach the new version patch set which merge the partition fix into it.
> Besides, instead of introducing new function and parameter, just add the
> partition filter in pg_get_publication_tables which makes the code cleaner.
>
> Only 0001 and 0003 was changed.

I've reviewed 0001 and 0002 patch and here are comments:

0001 patch:

+/*
+ * Get the list of publishable relation oids for a specified schema.
+ *
+ * Schema will be having both ordinary('r') relkind tables and partitioned('p')
+ * relkind tables, so two rounds of scan are required.
+ */
+List *
+GetSchemaPublicationRelations(Oid schemaid, PublicationPartOpt pub_partopt)
+{
+ Relation classRel;
+ ScanKeyData key[3];
+ TableScanDesc scan;

I think it's enough to have key[2], not key[3].

BTW, this function does the table scan on pg_class twice in order to
get OIDs of both normal tables and partitioned tables. But can't we do
that by the single table scan? I think we can set a scan key for
relnamespace, and check relkind inside a scan loop.

---
+ ObjectsInPublicationToOids(stmt->pubobjects, pstate,
&relations,
+
&schemaidlist);
+
+ if (list_length(relations) > 0)
+ {
+ List *rels;
+
+ rels = OpenTableList(relations);
+ CheckObjSchemaNotAlreadyInPublication(rels,
schemaidlist,
+
PUBLICATIONOBJ_TABLE);
+ PublicationAddTables(puboid, rels, true, NULL);
+ CloseTableList(rels);
+ }
+
+ if (list_length(schemaidlist) > 0)
+ {
+ /* FOR ALL TABLES IN SCHEMA requires superuser */
+ if (!superuser())
+ ereport(ERROR,
+
errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("must be
superuser to create FOR ALL TABLES IN SCHEMA publication"));
+

Perhaps we can do a superuser check before handling "relations"? If
the user doesn't have the permission, we don't need to do anything for
relations.

0002 patch:

postgres(1:13619)=# create publication pub for all TABLES in schema
CURRENT_SCHEMA pg_catalog public s2
information_schema pg_toast s1

Since pg_catalog and pg_toast cannot be added to the schema
publication can we exclude them from the completion list?

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/