MIME-Version: 1.0
References: <OS0PR01MB5716352552DFADB8E9AD1D8994C92@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <OS0PR01MB57161006B8F2779F2C97318194D42@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <CABdArM6wYc6SAhxRK26f=24D170DeLxirdPk6mC8dvWbD7mkeA@mail.gmail.com>
 <OS0PR01MB5716094C114589064B1B21E594D62@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <OS0PR01MB571686E464A325F26CEFCCEF94DD2@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <CAJpy0uBtGwj+_8QTQZDi8ZW=eYTbRbK3Pt2MCMX2hzCxf==4Vw@mail.gmail.com>
 <OS0PR01MB57168F2C0DA21A8DAAA06AED94A52@OS0PR01MB5716.jpnprd01.prod.outlook.com>
 <CAJpy0uCBNfcjm+4AA7SKPPNN5MzW1V7sy=CHsyGNXz2D3v1Wvw@mail.gmail.com>
 <CAA4eK1+aK4MLxbfLtp=EV5bpvJozKhxGDRS6T9q8sz_s+LK3vw@mail.gmail.com> <OS0PR01MB5716837A0DFA3B4FA903DA9C94BD2@OS0PR01MB5716.jpnprd01.prod.outlook.com>
In-Reply-To: <OS0PR01MB5716837A0DFA3B4FA903DA9C94BD2@OS0PR01MB5716.jpnprd01.prod.outlook.com>
From: Amit Kapila <amit.kapila16@gmail.com>
Date: Mon, 5 Aug 2024 09:50:38 +0530
Message-ID: <CAA4eK1L5vh8YGZ2Ec9M5HbGcN6uf0OZtUV583LmFXZHCUXO3Jw@mail.gmail.com>
Subject: Re: Conflict detection and logging in logical replication
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Cc: shveta malik <shveta.malik@gmail.com>, Nisha Moond <nisha.moond412@gmail.com>, 
	pgsql-hackers <pgsql-hackers@postgresql.org>, Dilip Kumar <dilipbalaut@gmail.com>, 
	Jan Wieck <jan@wi3ck.info>, Tomas Vondra <tomas.vondra@enterprisedb.com>, 
	Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAA4eK1L5vh8YGZ2Ec9M5HbGcN6uf0OZtUV583LmFXZHCUXO3Jw%40mail.gmail.com>
Precedence: bulk

On Sun, Aug 4, 2024 at 1:04=E2=80=AFPM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> On Friday, July 26, 2024 2:26 PM Amit Kapila <amit.kapila16@gmail.com> wr=
ote:
> >
> >
> > I agree that displaying pk where applicable should be okay as we displa=
y it at
> > other places but the same won't be possible when we do sequence scan to
> > fetch the required tuple. So, the message will be different in that cas=
e, right?
>
> After some research, I think we can report the key values in DETAIL if th=
e
> apply worker uses any unique indexes to find the tuple to update/delete.
> Otherwise, we can try to output all column values in DETAIL if the curren=
t user
> of apply worker has SELECT access to these columns.
>

I don't see any problem with displaying the column values in the LOG
message when the user can access it. Also, we do the same in other
places to further strengthen this idea.

> This is consistent with what we do when reporting table constraint violat=
ion
> (e.g. when violating a check constraint, it could output all the column v=
alue
> if the current has access to all the column):
>
> - First, use super user to create a table.
> CREATE TABLE t1 (c1 int, c2 int, c3 int check (c3 < 5));
>
> - 1) using super user to insert a row that violates the constraint. We sh=
ould
> see all the column value.
>
> INSERT INTO t1(c3) VALUES (6);
>         ERROR:  new row for relation "t1" violates check constraint "t1_c=
3_check"
>         DETAIL:  Failing row contains (null, null, 6).
>
> - 2) use a user without access to all the columns. We can only see the in=
serted column and
> CREATE USER regress_priv_user2;
> GRANT INSERT (c1, c2, c3) ON t1 TO regress_priv_user2;
>
> SET SESSION AUTHORIZATION regress_priv_user2;
> INSERT INTO t1 (c3) VALUES (6);
>
>         ERROR:  new row for relation "t1" violates check constraint "t1_c=
3_check"
>         DETAIL:  Failing row contains (c3) =3D (6).
>
> To achieve this, I think we can expose the ExecBuildSlotValueDescription
> function and use it in conflict reporting. What do you think ?
>

Agreed. We should also consider displaying both the local and remote
rows in case of update/delete_differ conflicts. Do, we have any case
during conflict reporting where we won't have access to any of the
columns? If so, we need to see what to display in such a case.

--=20
With Regards,
Amit Kapila.