Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher

Amit Kapila <amit.kapila16@gmail.com>

From: Amit Kapila <amit.kapila16@gmail.com>
To: Önder Kalacı <onderkalaci@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>, "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, "wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>, Marco Slot <marco.slot@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Andres Freund <andres@anarazel.de>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-03-08T03:15:32Z
Lists: pgsql-hackers
On Tue, Mar 7, 2023 at 7:17 PM Önder Kalacı <onderkalaci@gmail.com> wrote:
>
>>
>> > > Let me give an example to demonstrate why I thought something is fishy here:
>> > >
>> > > Imagine rel has a (non-default) REPLICA IDENTITY with Oid=1111.
>> > > Imagine the same rel has a PRIMARY KEY with Oid=2222.
>> > >
>
>
> Hmm, alright, this is syntactically possible, but not sure if any user
> would do this. Still thanks for catching this.
>
> And, you are right, if a user has created such a schema, IdxIsRelationIdentityOrPK()
> would return the wrong result and we'd use sequential scan instead of index scan.
> This would be a regression. I think we should change the function.
>
>
> Here is the example:
> DROP TABLE tab1;
> CREATE TABLE tab1 (a int NOT NULL);
> CREATE UNIQUE INDEX replica_unique ON tab1(a);
> ALTER TABLE tab1 REPLICA IDENTITY USING INDEX replica_unique;
> ALTER TABLE tab1 ADD CONSTRAINT pkey PRIMARY KEY (a);
>

You have not given complete steps to reproduce the problem where
instead of the index scan, a sequential scan would be picked. I have
tried to reproduce by extending your steps but didn't see the problem.
Let me know if I am missing something.

Publisher
----------------
postgres=# CREATE TABLE tab1 (a int NOT NULL);
CREATE TABLE
postgres=# Alter Table tab1 replica identity full;
ALTER TABLE
postgres=# create publication pub2 for table tab1;
CREATE PUBLICATION
postgres=# insert into tab1 values(1);
INSERT 0 1
postgres=# update tab1 set a=2;

Subscriber
-----------------
postgres=# CREATE TABLE tab1 (a int NOT NULL);
CREATE TABLE
postgres=# CREATE UNIQUE INDEX replica_unique ON tab1(a);
CREATE INDEX
postgres=# ALTER TABLE tab1 REPLICA IDENTITY USING INDEX replica_unique;
ALTER TABLE
postgres=# ALTER TABLE tab1 ADD CONSTRAINT pkey PRIMARY KEY (a);
ALTER TABLE
postgres=# create subscription sub2 connection 'dbname=postgres'
publication pub2;
NOTICE:  created replication slot "sub2" on publisher
CREATE SUBSCRIPTION
postgres=# select * from tab1;
 a
---
 2
(1 row)

I have debugged the above example and it uses an index scan during
apply without your latest change which is what I expected. AFAICS, the
use of IdxIsRelationIdentityOrPK() is to decide whether we will do
tuples_equal() or not during the index scan and I see it gives the
correct results with the example you provided.

-- 
With Regards,
Amit Kapila.



Commits

  1. Add the testcases for 89e46da5e5.

  2. Allow the use of indexes other than PK and REPLICA IDENTITY on the subscriber.

  3. Rework query relation permission checking

  4. Generalize ri_RootToPartitionMap to use for non-partition children

  5. Add wait_for_subscription_sync for TAP tests.

  6. Logical replication