Re: row filtering for logical replication

On Mon, Jan 17, 2022 at 8:58 PM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> Attach the V66 patch set which addressed Amit, Peter and Greg's comments.
>

Thanks, some more comments, and suggestions:

1.
/*
+ * If no record in publication, check if the table is the partition
+ * of a published partitioned table. If so, the table has no row
+ * filter.
+ */
+ else if (!pub->pubviaroot)
+ {
+ List *schemarelids;
+ List *relids;
+
+ schemarelids = GetAllSchemaPublicationRelations(pub->oid,
+ PUBLICATION_PART_LEAF);
+ relids = GetPublicationRelations(pub->oid,
+ PUBLICATION_PART_LEAF);
+
+ if (list_member_oid(schemarelids, entry->publish_as_relid) ||
+ list_member_oid(relids, entry->publish_as_relid))
+ pub_no_filter = true;
+
+ list_free(schemarelids);
+ list_free(relids);
+
+ if (!pub_no_filter)
+ continue;
+ }

As far as I understand this handling is required only for partition
tables but it seems to be invoked for non-partition tables as well.
Please move the comment inside else if block and expand a bit more to
say why it is necessary to not directly set pub_no_filter here. Note,
that I think this can be improved (avoid cache lookups) if we maintain
a list of pubids in relsyncentry but I am not sure that is required
because this is a rare case and needs to be done only one time.

2.
static HTAB *OpClassCache = NULL;

-
/* non-export function prototypes */

Spurious line removal. I have added back in the attached top-up patch.

Apart from the above, I have made some modifications to other comments.

--
With Regards,
Amit Kapila.