public inbox for [email protected]  
help / color / mirror / Atom feed
From: Hironobu SUZUKI <[email protected]>
To: pgsql-hackers <[email protected]>
Cc: Euler Taveira <[email protected]>
Subject: Re: row filtering for logical replication
Date: Wed, 21 Nov 2018 05:51:34 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAHE3wgiHOdFHcEpm_w5f8vAXiugDmTciTiegNGVktezMbFkkrg@mail.gmail.com>
References: <CAHE3wggb715X+mK_DitLXF25B=jE6xyNCH4YOwM860JR7HarGQ@mail.gmail.com>
	<CAHE3wgiHOdFHcEpm_w5f8vAXiugDmTciTiegNGVktezMbFkkrg@mail.gmail.com>

On 2018/11/01 0:29, Euler Taveira wrote:
> Em qua, 28 de fev de 2018 às 20:03, Euler Taveira
> <[email protected]> escreveu:
>> The attached patches add support for filtering rows in the publisher.
>>
> I rebased the patch. I added row filtering for initial
> synchronization, pg_dump support and psql support. 0001 removes unused
> code. 0002 reduces memory use. 0003 passes only structure member that
> is used in create_estate_for_relation. 0004 reuses a parser node for
> row filtering. 0005 is the feature. 0006 prints WHERE expression in
> psql. 0007 adds pg_dump support. 0008 is only for debug purposes (I'm
> not sure some of these messages will be part of the final patch).
> 0001, 0002, 0003 and 0008 are not mandatory for this feature.
> 
> Comments?
> 
> 

Hi,

I reviewed your patches and I found a bug when I tested ALTER 
PUBLICATION statement.

In short, ALTER PUBLICATION SET with a WHERE clause does not applied new 
WHERE clause.

I describe the outline of the test I did and my conclusion.

[TEST]
I show the test case I tried in below.

(1)Publisher and Subscriber

I executed each statement on the publisher and the subscriber.

```
testdb=# CREATE PUBLICATION pub_testdb_t FOR TABLE t WHERE (id > 10);
CREATE PUBLICATION
```

```
testdb=# CREATE SUBSCRIPTION sub_testdb_t CONNECTION 'dbname=testdb 
port=5432 user=postgres' PUBLICATION pub_testdb_t;
NOTICE:  created replication slot "sub_testdb_t" on publisher
CREATE SUBSCRIPTION
```

(2)Publisher

I executed these statements shown below.

testdb=# INSERT INTO t VALUES (1,1);
INSERT 0 1
testdb=# INSERT INTO t VALUES (11,11);
INSERT 0 1

(3)Subscriber

I confirmed that the CREATE PUBLICATION statement worked well.

```
testdb=# SELECT * FROM t;
  id | data
----+------
  11 |   11
(1 row)
```

(4)Publisher
After that, I executed ALTER PUBLICATION with a WHERE clause and 
inserted a new row.

```
testdb=# ALTER  PUBLICATION pub_testdb_t SET TABLE t WHERE (id > 5);
ALTER PUBLICATION

testdb=# INSERT INTO t VALUES (7,7);
INSERT 0 1

testdb=# SELECT * FROM t;
  id | data
----+------
   1 |    1
  11 |   11
   7 |    7
(3 rows)
```

(5)Subscriber
I confirmed that the change of WHERE clause set by ALTER PUBLICATION 
statement was ignored.

```
testdb=# SELECT * FROM t;
  id | data
----+------
  11 |   11
(1 row)
```

[Conclusion]
I think AlterPublicationTables()@publicationcmds.c has a bug.

In the foreach(oldlc, oldrelids) loop, oldrel must be appended to 
delrels if oldrel or newrel has a WHERE clause. However, the current 
implementation does not, therefore, old WHERE clause is not deleted and 
the new WHERE clause is ignored.

This is my speculation. It may not be correct, but , at least, it is a 
fact that ALTER PUBLICATION with a WHERE clause is not functioned in my 
environment and my operation described in above.


Best regards,




view thread (489+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: row filtering for logical replication
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox