Horton
Menu

RE: Column Filtering in Logical Replication

Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>

View thread
From: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.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>, Peter Smith <smithpb2250@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2021-12-17T09:46:14Z
Lists: pgsql-hackers 
On Friday, December 17, 2021 1:55 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> On 2021-Dec-16, houzj.fnst@fujitsu.com wrote:
> 
> > The patch ensures all columns of RT are in column list when
> > CREATE/ALTER publication, but it seems doesn't prevent user from
> > changing the replica identity or dropping the index used in replica
> > identity. Do we also need to check those cases ?
> 
> Yes, we do.  As it happens, I spent a couple of hours yesterday writing code for
> that, at least partially.  I haven't yet checked what happens with cases like
> REPLICA NOTHING, or REPLICA INDEX <xyz> and then dropping that index.
> 
> My initial ideas were a bit wrong BTW: I thought we should check the
> combination of column lists in all publications (a bitwise-OR of column bitmaps,
> so to speak).  But conceptually that's wrong: we need to check the column list
> of each publication individually instead.  Otherwise, if you wanted to hide a
> column from some publication but that column was part of the replica identity,
> there'd be no way to identify the tuple in the replica.  (Or, if the pgouput code
> disobeys the column list and sends the replica identity even if it's not in the
> column list, then you'd be potentially publishing data that you wanted to hide.)

Thanks for the explanation.

Apart from ALTER REPLICA IDENTITY and DROP INDEX, I think there could be
some other cases we need to handle for the replica identity check:

1)
When adding a partitioned table with column list to the publication, I think we
need to check the RI of all its leaf partition. Because the RI on the partition
is the one actually takes effect.

2)
ALTER TABLE ADD PRIMARY KEY;
ALTER TABLE DROP CONSTRAINT "PRIMAEY KEY";

If the replica identity is default, it will use the primary key. we might also
need to prevent user from adding or removing primary key in this case.


Based on the above cases, the RI check seems could bring considerable amount of
code. So, how about we follow what we already did in CheckCmdReplicaIdentity(),
we can put the check for RI in that function, so that we can cover all the
cases and reduce the code change. And if we are worried about the cost of do
the check for UPDATE and DELETE every time, we can also save the result in the
relcache. It's safe because every operation change the RI will invalidate the
relcache. We are using this approach in row filter patch to make sure all
columns in row filter expression are part of RI.

Best regards,
Hou zj

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