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
-
Add the testcases for 89e46da5e5.
- 805b821e77a3 16.0 landed
-
Allow the use of indexes other than PK and REPLICA IDENTITY on the subscriber.
- 89e46da5e511 16.0 landed
-
Rework query relation permission checking
- a61b1f74823c 16.0 cited
-
Generalize ri_RootToPartitionMap to use for non-partition children
- fb958b5da86d 16.0 cited
-
Add wait_for_subscription_sync for TAP tests.
- 0c20dd33db16 16.0 cited
-
Logical replication
- 665d1fad99e7 10.0 cited