Re: Column Filtering in Logical Replication

> + bool am_partition = false;
>...
> Assert(!isnull);
> lrel->relkind = DatumGetChar(slot_getattr(slot, 3, &isnull));
> Assert(!isnull);
> + am_partition = DatumGetChar(slot_getattr(slot, 4, &isnull));

I think this needs to be GetBool.
You should Assert(!isnull) like the others.
Also, I think it doesn't need to be initialized to "false".

> + /*
> + * Even if the user listed all columns in the column list, we cannot
> + * allow a column list to be specified when REPLICA IDENTITY is FULL;
> + * that would cause problems if a new column is added later, because
> + * that could would have to be included (because of being part of the

could would is wrong

> + /*
> + * Translate list of columns to attnums. We prohibit system attributes and
> + * make sure there are no duplicate columns.
> + *
> + */

extraneous line

> +/*
> + * Gets a list of OIDs of all column-partial publications of the given
> + * relation, that is, those that specify a column list.

I would call this a "partial-column" publication.

> + errmsg("cannot set REPLICA IDENTITY FULL when column-partial publications exist"));
> + * Check column-partial publications. All publications have to include all

same

> + /*
> + * Store the column names only if they are contained in column filter

period(.)

> + * LogicalRepRelation will only contain attributes corresponding to those
> + * specficied in column filters.

specified

> --- a/src/include/catalog/pg_publication_rel.h
> +++ b/src/include/catalog/pg_publication_rel.h
> @@ -31,6 +31,9 @@ CATALOG(pg_publication_rel,6106,PublicationRelRelationId)
> Oid oid; /* oid */
> Oid prpubid BKI_LOOKUP(pg_publication); /* Oid of the publication */
> Oid prrelid BKI_LOOKUP(pg_class); /* Oid of the relation */
> +#ifdef CATALOG_VARLEN
> + int2vector prattrs; /* Variable length field starts here */
> +#endif

The language in the pre-existing comments is better:
/* variable-length fields start here */

> @@ -791,12 +875,13 @@ fetch_remote_table_info(char *nspname, char *relname,
>
> ExecClearTuple(slot);
> }
> +
> ExecDropSingleTupleTableSlot(slot);
> + walrcv_clear_result(res);
> + pfree(cmd.data);
>
> lrel->natts = natt;
>
> - walrcv_clear_result(res);
> - pfree(cmd.data);
> }

The blank line after "lrel->natts = natt;" should be removed.