Feedback-ID: id4a34324:Fastmail
Date: Mon, 9 Jan 2023 12:03:47 -0800
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: =?utf-8?B?w5ZuZGVyIEthbGFjxLE=?= <onderkalaci@gmail.com>,
	"shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>,
	"wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>,
	Amit Kapila <amit.kapila16@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is
 full on the publisher
Message-ID: <20230109200347.ps43ri7llrxusgu5@awork3.anarazel.de>
References: <OSZPR01MB63105BD4B03B3C1FBFCC7E6CFD249@OSZPR01MB6310.jpnprd01.prod.outlook.com>
 <CACawEhWdoiaXwKjn5ZfAKF_Toz=t1aqT1ewHmJLRnXmKOQKwyg@mail.gmail.com>
 <OS3PR01MB62754E945BD95A9B2A1164789E289@OS3PR01MB6275.jpnprd01.prod.outlook.com>
 <CACawEhXBtt9aMoU0j6funj-s+CW+e8HMFCGz30gyEwLazXB_1w@mail.gmail.com>
 <OSZPR01MB63109FF52120649EECACC7AEFD2A9@OSZPR01MB6310.jpnprd01.prod.outlook.com>
 <CACawEhUUHcSVhp-bODSAnYNUQHY=4gvY29DXM=yuphgrkcqqmQ@mail.gmail.com>
 <CACawEhX6TvX+j8EpcpCKvnMGao8Gcp8W43Sgc87pg9o6-Xbf2Q@mail.gmail.com>
 <20221206184756.2fdpd4epvyftutoe@awork3.anarazel.de>
 <CACawEhV68Ry7BGJV150xsUskJPa88rpOf3zo_OcA_MpSWGQobQ@mail.gmail.com>
 <3466340.1673117404@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <3466340.1673117404@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/20230109200347.ps43ri7llrxusgu5%40awork3.anarazel.de>
Precedence: bulk

Hi,

On 2023-01-07 13:50:04 -0500, Tom Lane wrote:
> I think we should either live within the constraints set by this
> overarching design, or else nuke execReplication.c from orbit and
> start using the real planner and executor.  Perhaps the foreign
> key enforcement mechanisms could be a model --- although if you
> don't want to buy into using SPI as well, you probably should look
> at Amit L's work at [1].

I don't think using the full executor for every change is feasible from an
overhead perspective. But it might make sense to bail out to using the full
executor in a bunch of non-fastpath paths.

I think this is basically similar to COPY not using the full executor.

But that doesn't mean that all of this has to be open coded in
execReplication.c. Abstracting pieces so that COPY, logical rep and perhaps
even nodeModifyTable.c can share code makes sense.


> Also ... maybe I am missing something, but is REPLICA IDENTITY FULL
> sanely defined in the first place?  It looks to me that
> RelationFindReplTupleSeq assumes without proof that there is a unique
> full-tuple match, but that is only reasonable to assume if there is at
> least one unique index (and maybe not even then, if nulls are involved).

If the table definition match between publisher and standby, it doesn't matter
which tuple is updated, if all columns are used to match. Since there's
nothing distinguishing two rows with all columns being equal, it doesn't
matter which we update.

Greetings,

Andres Freund