Re: Column Filtering in Logical Replication

Peter Smith <smithpb2250@gmail.com>

From: Peter Smith <smithpb2250@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Rahila Syed <rahilasyed90@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Tomas Vondra <tomas.vondra@enterprisedb.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2021-09-07T00:29:49Z
Lists: pgsql-hackers
Here are some v5 review comments for your consideration:

------

1. src/backend/access/common/relation.c

@@ -215,3 +217,22 @@ relation_close(Relation relation, LOCKMODE lockmode)
  if (lockmode != NoLock)
  UnlockRelationId(&relid, lockmode);
 }
+
+/*
+ * Return a bitmapset of attributes given the list of column names
+ */
+Bitmapset*
+get_table_columnset(Oid relid, List *columns, Bitmapset *att_map)
+{


IIUC that 3rd parameter (att_map) is always passed as NULL to
get_table_columnset function because you are constructing this
Bitmapset from scratch. Maybe I am mistaken, but if not then what is
the purpose of that att_map parameter?

------

2. src/backend/catalog/pg_publication.c

+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("cannot add relation \"%s\" to publication",
+ RelationGetRelationName(targetrel)),
+ errdetail("Column filter must include REPLICA IDENTITY columns")));

Is ERRCODE_INVALID_COLUMN_REFERENCE a more appropriate errcode to use here?

------

3. src/backend/catalog/pg_publication.c

+ else
+ {
+ Bitmapset *filtermap = NULL;
+ idattrs = RelationGetIndexAttrBitmap(targetrel,
INDEX_ATTR_BITMAP_IDENTITY_KEY);

The RelationGetIndexAttrBitmap function comment says "should be
bms_free'd when not needed anymore" but it seems the patch code is not
freeing idattrs when finished using it.

------
Kind Regards,
Peter Smith.
Fujitsu Australia



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