Horton
Menu

Re: Column Filtering in Logical Replication

Peter Eisentraut <peter.eisentraut@enterprisedb.com>

View thread
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>, Justin Pryzby <pryzby@telsasoft.com>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>, Amit Kapila <amit.kapila16@gmail.com>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Rahila Syed <rahilasyed90@gmail.com>, Peter Smith <smithpb2250@gmail.com>, pgsql-hackers@postgresql.org
Date: 2022-01-07T11:46:35Z
Lists: pgsql-hackers 
I think this is getting pretty good now.  I like the overall behavior now.

Some details:

There are still a few references to "filter", but I see most of the
patch now uses column list or something.  Maybe do another cleanup
pass before finalizing the patch.

doc/src/sgml/catalogs.sgml needs to be updated.

doc/src/sgml/ref/alter_publication.sgml:

"allows to change" -> "allows changing"

src/backend/catalog/pg_publication.c:

publication_translate_columns(): I find the style of having a couple
of output arguments plus a return value that is actually another
output value confusing.  (It would be different if the return value
was some kind of success value.)  Let's make it all output arguments.

About the XXX question there: I would make the column numbers always
sorted.  I don't have a strong reason for this, but otherwise we might
get version differences, unstable dumps etc.  It doesn't seem
complicated to keep this a bit cleaner.

I think publication_translate_columns() also needs to prohibit
generated columns.  We already exclude those implicitly throughout the
logical replication code, but if a user explicitly set one here,
things would probably break.

src/backend/commands/tablecmds.c:

ATExecReplicaIdentity(): Regarding the question of how to handle
REPLICA_IDENTITY_NOTHING: I see two ways to do this.  Right now, the
approach is that the user can set the replica identity freely, and we
decide later based on that what we can replicate (e.g., no updates).
For this patch, that would mean we don't restrict what columns can be
in the column list, but we check what actions we can replicate based
on the column list.  The alternative is that we require the column
list to include the replica identity, as the patch is currently doing,
which would mean that REPLICA_IDENTITY_NOTHING can be allowed since
it's essentially a set of zero columns.

I find the current behavior a bit weird on reflection.  If a user
wants to replicate on some columns and only INSERTs, that should be
allowed regardless of what the replica identity columns are.

src/backend/replication/pgoutput/pgoutput.c:

In get_rel_sync_entry(), why did you remove the block

-           if (entry->pubactions.pubinsert && 
entry->pubactions.pubupdate &&
-               entry->pubactions.pubdelete && 
entry->pubactions.pubtruncate)
-               break;

Maybe this is intentional, but it's not clear to me.

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