Re: Column Filtering in Logical Replication

Tomas Vondra <tomas.vondra@enterprisedb.com>

From: Tomas Vondra <tomas.vondra@enterprisedb.com>

To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Amit Kapila <amit.kapila16@gmail.com>

Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Justin Pryzby <pryzby@telsasoft.com>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Rahila Syed <rahilasyed90@gmail.com>, Peter Smith <smithpb2250@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, shiy.fnst@fujitsu.com

Date: 2022-03-10T18:20:10Z

Lists: pgsql-hackers

On 3/9/22 10:20, Peter Eisentraut wrote:
> 
> On 07.03.22 16:18, Tomas Vondra wrote:
>> AFAICS these issues should be resolved by the adoption of the row-filter
>> approach (i.e. it should fail the same way as for row filter).
> 
> The first two patches (additional testing for row filtering feature)
> look okay to me.
> 
> Attached is a fixup patch for your main feature patch (the third one).
> 
> It's a bit of code and documentation cleanup, but mainly I removed the
> term "column filter" from the patch.  Half the code was using "column
> list" or similar and half the code "column filter", which was confusing.
>  Also, there seemed to be a bit of copy-and-pasting from row-filter code
> going on, with some code comments not quite sensible, so I rewrote some
> of them.  Also some code used "rf" and "cf" symbols which were a bit
> hard to tell apart.  A few more letters can increase readability.
> 
> Note in publicationcmds.c OpenTableList() the wrong if condition was used.
> 

Thanks, I've merged these changes into the patch.

> I'm still confused about the intended replica identity handling.  This
> patch still checks whether the column list contains the replica identity
> at DDL time.  And then it also checks at execution time.  I thought the
> latest understanding was that the DDL-time checking would be removed.  I
> think it's basically useless now, since as the test cases show, you can
> subvert those checks by altering the replica identity later.

Are you sure? Which part of the patch does that? AFAICS we only do those
checks in CheckCmdReplicaIdentity now, but maybe I'm missing something.
Are you sure you're not looking at some older patch version?


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Commits

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

Doc: Explain about Column List feature.
- f98d07424523 16.0 landed
- 25996a82a446 15.0 landed
Doc: fix column list vs. replica identity rules.
- 376af686111e 16.0 landed
- ab3131880d8a 15.0 landed
Prohibit combining publications with different column lists.
- fd0b9dcebda7 15.0 cited
Fix the check to limit sync workers.
- be46985bed8a 10.21 landed
- a90de822e483 11.16 landed
- 59348fbdeb7b 12.11 landed
- 82d4a17a1750 13.7 landed
- c9dea58e2702 14.3 landed
- dd4ab6fd6528 15.0 landed
Wait for subscription to sync in t/031_column_list.sql
- 404f49338fef 15.0 landed
Move prattrs to the pg_publication_rel section in docs
- 41b00f8e601f 15.0 landed
Allow specifying column lists for logical replication
- 923def9a533a 15.0 landed
Fix row filters with multiple publications
- 5a079662256e 15.0 landed
Fix publish_as_relid with multiple publications
- 27fafee72d17 13.7 landed
- 677a1dc0ca0f 14.3 landed
- c91f71b9dc91 15.0 landed
Add some additional tests for row filters in logical replication.
- ceb57afd3ce1 15.0 cited
Add index on pg_publication_rel.prpubid
- 025b920a3d45 15.0 landed
Avoid using DefElemAction in AlterPublicationStmt
- 9623d8999603 15.0 landed
Small cleanups related to PUBLICATION framework code
- c9105dd3660f 15.0 landed
Add PublicationTable and PublicationRelInfo structs
- 0c6828fa987b 15.0 landed
Fix various concurrency issues in logical replication worker launching
- de4389712206 10.0 cited