Horton
Menu

Re: Column Filtering in Logical Replication

Justin Pryzby <pryzby@telsasoft.com>

View thread
From: Justin Pryzby <pryzby@telsasoft.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>, Amit Kapila <amit.kapila16@gmail.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Rahila Syed <rahilasyed90@gmail.com>, Peter Smith <smithpb2250@gmail.com>, pgsql-hackers@postgresql.org
Date: 2021-12-30T22:16:34Z
Lists: pgsql-hackers 
> +	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.

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Doc: Explain about Column List feature.

  2. Doc: fix column list vs. replica identity rules.

  3. Prohibit combining publications with different column lists.

  4. Fix the check to limit sync workers.

  5. Wait for subscription to sync in t/031_column_list.sql

  6. Move prattrs to the pg_publication_rel section in docs

  7. Allow specifying column lists for logical replication

  8. Fix row filters with multiple publications

  9. Fix publish_as_relid with multiple publications

  10. Add some additional tests for row filters in logical replication.

  11. Add index on pg_publication_rel.prpubid

  12. Avoid using DefElemAction in AlterPublicationStmt

  13. Small cleanups related to PUBLICATION framework code

  14. Add PublicationTable and PublicationRelInfo structs

  15. Fix various concurrency issues in logical replication worker launching