Horton
Menu

Re: Column Filtering in Logical Replication

Rahila Syed <rahilasyed90@gmail.com>

View thread
From: Rahila Syed <rahilasyed90@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2021-07-12T09:38:23Z
Lists: pgsql-hackers

Attachments

Hi Alvaro,

Thank you for comments.

The patch adds a function get_att_num_by_name; but we have a lsyscache.c
> function for that purpose, get_attnum.  Maybe that one should be used
> instead?
>
> Thank you for pointing that out, I agree it makes sense to reuse the
existing function.
Changed it accordingly in the attached patch.


> get_tuple_columns_map() returns a bitmapset of the attnos of the columns
> in the given list, so its name feels wrong.  I propose
> get_table_columnset().  However, this function is invoked for every
> insert/update change, so it's going to be far too slow to be usable.  I
> think you need to cache the bitmapset somewhere, so that the function is
> only called on first use.  I didn't look very closely, but it seems that
> struct RelationSyncEntry may be a good place to cache it.
>
> Makes sense, changed accordingly.


> The patch adds a new parse node PublicationTable, but doesn't add
> copyfuncs.c, equalfuncs.c, readfuncs.c, outfuncs.c support for it.
> Maybe try a compile with WRITE_READ_PARSE_PLAN_TREES and/or
> COPY_PARSE_PLAN_TREES enabled to make sure everything is covered.
> (I didn't verify that this actually catches anything ...)
>

I will test this and include these changes in the next version.


> The new column in pg_publication_rel is prrel_attr.  This name seems at
> odds with existing column names (we don't use underscores in catalog
> column names).  Maybe prattrs is good enough?  prrelattrs?  We tend to
> use plurals for columns that are arrays.
>
> Renamed it to prattrs as per suggestion.

It's not super clear to me that strlist_to_textarray() and related
> processing will behave sanely when the column names contain weird
> characters such as commas or quotes, or just when used with uppercase
> column names.  Maybe it's worth having tests that try to break such
> cases.
>
> Sure, I will include these tests in the next version.


> You seem to have left a debugging "elog(LOG)" line in OpenTableList.
>
> Removed.


> I got warnings from "git am" about trailing whitespace being added by
> the patch in two places.
>
> Should be fixed now.

Thank you,
Rahila Syed

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