Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1waYKJ-001svF-1h for pgsql-hackers@arkaria.postgresql.org; Fri, 19 Jun 2026 12:28:03 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.96) (envelope-from ) id 1waYKI-00G0Xg-0e for pgsql-hackers@arkaria.postgresql.org; Fri, 19 Jun 2026 12:28:02 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1waYKH-00G0XX-2l for pgsql-hackers@lists.postgresql.org; Fri, 19 Jun 2026 12:28:01 +0000 Received: from mail-ed1-x531.google.com ([2a00:1450:4864:20::531]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.98.2) (envelope-from ) id 1waYKF-00000001GYy-1Oxg for pgsql-hackers@lists.postgresql.org; Fri, 19 Jun 2026 12:28:01 +0000 Received: by mail-ed1-x531.google.com with SMTP id 4fb4d7f45d1cf-693c69b97e7so3292743a12.2 for ; Fri, 19 Jun 2026 05:27:58 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1781872078; cv=none; d=google.com; s=arc-20240605; b=MMGFPf/cnL2Rius9cuuPJP+z+TmgYR6VmW8wBdrM56TdmlP7ETRVaYcd5lGTAbLkXf yYM/VilHIm84BwMEbpVKd8dRnAirKVWfbntwS5UhvovqmKor5wbA96GpeLYsMWlNbefc g1Pkg4aAaGR6bdBCvFYn7NzV7VqpPiqujMOCyh5WZyLMk81GWLFCEnQPRk6mwrTzDuTt 5eGnK4Jd/SIhricGr4iuUwsk2R1JbNDH9dljz0cCYl02ZQ2shw7wBveQbsPxVkE+mi0t 1U+mhij0qbzL9DdCVZ+7TkzepqxZ1onVgOqS/0JakvywtZyf4Na04xYFuq6MsPSOTYk7 IzAw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20240605; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:dkim-signature; bh=UqJot1osPsGAziyRDgfDcqYTK+jfk9xt2YiZzz2GlNY=; fh=n4b4lv1o12xFWgDI6agbHczRvGca0YOKtrg3zrctZZ0=; b=IYLDcnQb0ZfAY6EFfonJymgIVy/GQy/pU4nUurWpHn3vKjVFoHjZBeZvZoVhqMS9Tz CrcBOaMxZuUnm57Y1zcPqeYelBZIRw5FNYvgQ+Tdlr9F/heb3zPiNpeh9JQZR3TOhOzF izskds3Uup5ox5iQOlcEozZdtnNsfxeOHZJh0Yx0/uDLajOgDF313ULlXJwB80PggdwB RUOHQgrahU5dHxGXNSTHkf+3gb/WU2EtAADcRw7w5FE9zzXtZqTHyMiA+Tax5Ns5okzZ 88gCR7nsd6vUZf1mGud6VrBMCyoY1rfYDrRB712uFrOjuNPC2WUIND7mKbbZA5WwK6yC y8EQ==; darn=lists.postgresql.org ARC-Authentication-Results: i=1; mx.google.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1781872078; x=1782476878; darn=lists.postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=UqJot1osPsGAziyRDgfDcqYTK+jfk9xt2YiZzz2GlNY=; b=SELJHisxO5nKyocQ+TfSlxnD0AOHr3u7JqA3QLIaAWpvF9ilMGy2ProWG1tzYgjKqK hg3I3aHzLt7ICPhxthSaBTD1zoVSanBTslRWbWuVzfGe0mR4D/2BVL+HRiWCz+j7hoh9 jEoesa8tJEpDmvAY9izEM+mnZR/IDxME4XLLCmQ9sjP3TLSaIRGBedXSLRvOO3zu1e1m R55r3hp3R2g+YEJb5h0RYfdUJJfhX8F5ulxKTngZFwegUTx3BA+AggAgc4zhYX/H+Gz5 AJk1FkKalaqW6MdXUUquonmMe05UJyn8PCtBeycLQpiq318ZiefuFs0ImPn8cGnmOWU3 VhpA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781872078; x=1782476878; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=UqJot1osPsGAziyRDgfDcqYTK+jfk9xt2YiZzz2GlNY=; b=cyVnGiPAcTlnuA5w5xfM1XrmtYU2G+QOmPJFsZnTV+KeRM4KhCErDNirVgJgYYs8gz pezd3FVYXs9J/TfwQb106B01wg7iCQcSoIiMr5q9d1UXecSejYxPg6lDTtXD/Nrhhdyj 6n8XIkdU6paxd6yzTR1lBq2u/WSlqM8bMy5eEPtim/Fio47ZszpzjCu5QnrTuE0Lyyfg ChtkRQfI15MOCtav6cfFBLss3tGWvkKy++Yar/9ulTisxVhQz8mlXxg+OcGADOeJopK+ g6iuku3CjOhSKdB72TF+yhV5iJWQoPvR+nPnWDI3HkYksV4RHpLkatD16smZwJXq6yWb fFZw== X-Forwarded-Encrypted: i=1; AFNElJ8VnRgZJZvtCWVyX9p6JIw7asLBsfb+J4Mnq+JUXfBc9rMk5ZJvQbPCXOJcrZ+lU/EaQ2mS2h8qUIxdbX6k@lists.postgresql.org X-Gm-Message-State: AOJu0YzHyBXSQiVBqcBQbxr7WpjtJXWISoMUzS4yyK/CA+IDq6Fnq8PL /96KH4pSHC0AuI4i5InkeSOid4UaVsSCqn84rwtg3figmmgIDrYyC6No5/S3mz0pXa+vOSjjspw O9l3Fq/isEQ+FBJAU3sWI7FNZeqTvBf4= X-Gm-Gg: AfdE7cmZYObmBRb2vhATxBgyUKb0/IzRmGYTGFdSOqaSB1gDmozPdlKr+g+LbfK7uTo Ogzd6EusxIi/zCYVd/Sa7EAgDe9YWbhnqU7i8cBREhUGVi4BOVGTY4+K0nb7CCsqCpxIslaM9k1 6z+sjzYkt/VbRamZGNfVg8bkq0OkTNDop4nz/tlQSakxOD7GHk3g/fTIrCVgVexCXj0r/bYkZf3 PZmFP9AIk33mfSL2Scv274RkMYannXxfoqdhTnZH2BmAnIQUNCMM5K+71WW/KrWXmhv7ihZ0g== X-Received: by 2002:a17:906:9fca:b0:c08:49a4:c737 with SMTP id a640c23a62f3a-c097ae2ddd2mr232397366b.2.1781872076411; Fri, 19 Jun 2026 05:27:56 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Nikhil Shetty Date: Fri, 19 Jun 2026 17:57:42 +0530 X-Gm-Features: AVVi8CcwR5McGRNQsaSBi7ZSw7vRUItW9dCj1auKqpiaJz5WBF7X09jWuhbxDK0 Message-ID: Subject: Re: Proposal: Conflict log history table for Logical Replication To: Dilip Kumar Cc: Shlok Kyal , vignesh C , Peter Smith , Nisha Moond , Amit Kapila , shveta malik , Masahiko Sawada , Bharath Rupireddy , PostgreSQL Hackers , shveta malik Content-Type: multipart/alternative; boundary="00000000000026574806549a6c6c" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --00000000000026574806549a6c6c Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Dilip, I was just testing this patch.I was thinking if we can have another column which stores the timestamp when the conflict happened. This will be useful for debugging purposes. Thanks, Nikhil On Fri, Jun 19, 2026 at 3:53=E2=80=AFPM Dilip Kumar = wrote: > On Thu, Jun 18, 2026 at 11:29=E2=80=AFAM Shlok Kyal > wrote: > > > > On Wed, 17 Jun 2026 at 18:31, vignesh C wrote: > > > > > > On Tue, 16 Jun 2026 at 05:19, Peter Smith > wrote: > > > > > > > > On Mon, Jun 8, 2026 at 9:39=E2=80=AFPM vignesh C > wrote: > > > > > > > > > > On Fri, 5 Jun 2026 at 07:59, Peter Smith > wrote: > > > > > > > > > > > > Hi Vignesh. > > > > > > > > > > > > Some review comments for the patch v45-0004. > > > > > > > > > > > > 4c. > > > > > > IMO there should be a separate function for handling the > subscription > > > > > > footer/s, same as there is already a function > > > > > > addFooterToPublicationDesc. > > > > > > > > > > It is not required in this case as we don't have multiple footers > from > > > > > different places to be added here. > > > > > > > > > > > > > Sure, it's not "required", but I think: > > > > A) Separating the footer code from the non-footer code makes it > easier to read > > > > B) The 'describeSubscriptions' function is too long. This would mak= e > > > > it 20 lines shorter. > > > > C) Consistent footer handling for pub/sub describes. > > > > > > Ok, Let's keep it consistent > > > > > > > ////// > > > > > > > > More review comments for v50-0005 > > > > > > > > =3D=3D=3D=3D=3D=3D > > > > src/bin/psql/describe.c > > > > > > > > 1. > > > > + /* Conflict log destination is supported in v19 and higher */ > > > > + if (pset.sversion >=3D 190000) > > > > > > > > The CLT is targeting PG20, right? So, that comment ought to say "is > > > > supported in v20 and higher". > > > > > > > > Ideally, there should be some "TODO" reminder comments here to ensu= re > > > > the appropriate 190000's get replaced by 200000 as soon as the > version > > > > number is bumped. Better to flag/comment all those places now, so > that > > > > nothing gets missed later. > > > > > > > > (A similar review comment probably applies also to the pg_dump > changes > > > > in the previous v50-0004 patch). > > > > > > I felt it is better to mention it in the commit message of the patch > > > instead of mentioning it in the code. > > > > > > The attached v51 version patch has the changes for the same. > > > The patch also addresses the comment from [1]. > > > [1] - > https://www.postgresql.org/message-id/CANhcyEUjc9TCcW1YAQVMTs6-huWBZoy%2B= sVkz5C8b72os5p-f%2Bg%40mail.gmail.com > > > > > Hi Dilip/ Vignesh, > > > > I reviewed the patch and here are some comments: > > > > 1. I further tested for the object which can be created in 'pg_conflict= ' > schemas > > and found that operations such CREATE EXTENSION, CREATE OPERATOR, > > CREATE COLLATION, CREATE TEXT SEARCH DICTIONARY on the schema > > pg_conflict. > > Is this expected? > > > > postgres=3D# CREATE EXTENSION hstore WITH SCHEMA pg_conflict; > > CREATE EXTENSION > > postgres=3D# CREATE EXTENSION pg_walinspect WITH SCHEMA pg_conflict; > > CREATE EXTENSION > > postgres=3D# CREATE COLLATION pg_conflict.mycollation (provider =3D > > libc,locale =3D 'C'); > > CREATE COLLATION > > postgres=3D# CREATE TEXT SEARCH DICTIONARY pg_conflict.simple_dict > > (TEMPLATE =3D pg_catalog.simple); > > CREATE TEXT SEARCH DICTIONARY > > postgres=3D# CREATE OPERATOR pg_conflict.=3D=3D=3D (LEFTARG =3D int, RI= GHTARG =3D > > int, PROCEDURE =3D int4eq); > > CREATE OPERATOR > > > > Also, when we create extension several objects are created in the schem= a: > > eg: > > extname | object > > > ---------+---------------------------------------------------------------= --------------------------------------------- > > hstore | type pg_conflict.hstore > > hstore | function pg_conflict.hstore_in(cstring) > > hstore | function pg_conflict.hstore_out(pg_conflict.hstore) > > hstore | function pg_conflict.hstore_recv(internal) > > hstore | function pg_conflict.hstore_send(pg_conflict.hstore) > > hstore | type pg_conflict.hstore[] > > hstore | function pg_conflict.hstore_version_diag(pg_conflict.hstore) > > . > > . > > > > So, should it be allowed? > > > > 2. When we try to DROP conflict log table we get an ERROR: > > postgres=3D# DROP TABLE pg_conflict.pg_conflict_log_16395; > > ERROR: permission denied: "pg_conflict_log_16395" is a system catalog > > > > But for other restricted operation such as UPDATE/INSERT we get the > error as: > > postgres=3D# INSERT INTO pg_conflict.pg_conflict_log_16395 VALUES (1); > > ERROR: cannot modify or insert data into conflict log table > > "pg_conflict_log_16395" > > DETAIL: Conflict log tables are system-managed and only support > > cleanup via DELETE or TRUNCATE. > > > > I think for the DROP case we should throw a similar error. Thoughts? > > Please find the updated patch version (rebased on current HEAD), which > fixes all the open comments in 0001 and 0002. We are still working on > 0003 regarding handling conflict log table insertion errors. > > > -- > Regards, > Dilip Kumar > Google > --00000000000026574806549a6c6c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Dilip,

I was just = testing this patch.I w= as thinking if we can have another column which stores the timestamp when t= he conflict happened. This will be useful for debugging=C2=A0purposes.

Thanks,
Nikhil

On Fri, Jun 19, = 2026 at 3:53=E2=80=AFPM Dilip Kumar <dilipbalaut@gmail.com> wrote:
On Thu, Jun 18, 2026 at 11:29= =E2=80=AFAM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
>
> On Wed, 17 Jun 2026 at 18:31, vignesh C <vignesh21@gmail.com> wrote:
> >
> > On Tue, 16 Jun 2026 at 05:19, Peter Smith <smithpb2250@gmail.com> wrote= :
> > >
> > > On Mon, Jun 8, 2026 at 9:39=E2=80=AFPM vignesh C <vignesh21@gmail.com&g= t; wrote:
> > > >
> > > > On Fri, 5 Jun 2026 at 07:59, Peter Smith <smithpb2250@gmail.com&= gt; wrote:
> > > > >
> > > > > Hi Vignesh.
> > > > >
> > > > > Some review comments for the patch v45-0004.
> > > > >
> > > > > 4c.
> > > > > IMO there should be a separate function for handli= ng the subscription
> > > > > footer/s, same as there is already a function
> > > > > addFooterToPublicationDesc.
> > > >
> > > > It is not required in this case as we don't have mu= ltiple footers from
> > > > different places to be added here.
> > > >
> > >
> > > Sure, it's not "required", but I think:
> > > A) Separating the footer code from the non-footer code makes= it easier to read
> > > B) The 'describeSubscriptions' function is too long.= This would make
> > > it 20 lines shorter.
> > > C) Consistent footer handling for pub/sub describes.
> >
> > Ok, Let's keep it consistent
> >
> > > //////
> > >
> > > More review comments for v50-0005
> > >
> > > =3D=3D=3D=3D=3D=3D
> > > src/bin/psql/describe.c
> > >
> > > 1.
> > > + /* Conflict log destination is supported in v19 and higher= */
> > > + if (pset.sversion >=3D 190000)
> > >
> > > The CLT is targeting PG20, right? So, that comment ought to = say "is
> > > supported in v20 and higher".
> > >
> > > Ideally, there should be some "TODO" reminder comm= ents here to ensure
> > > the appropriate 190000's get replaced by 200000 as soon = as the version
> > > number is bumped. Better to flag/comment all those places no= w, so that
> > > nothing gets missed later.
> > >
> > > (A similar review comment probably applies also to the pg_du= mp changes
> > > in the previous v50-0004 patch).
> >
> > I felt it is better to mention it in the commit message of the pa= tch
> > instead of mentioning it in the code.
> >
> > The attached v51 version patch has the changes for the same.
> > The patch also addresses the comment from [1].
> > [1] - https://www.postgresql.org/message-id/CANhcyEUjc9= TCcW1YAQVMTs6-huWBZoy%2BsVkz5C8b72os5p-f%2Bg%40mail.gmail.com
> >
> Hi Dilip/ Vignesh,
>
> I reviewed the patch and here are some comments:
>
> 1. I further tested for the object which can be created in 'pg_con= flict' schemas
> and found that operations such CREATE EXTENSION, CREATE OPERATOR,
> CREATE COLLATION, CREATE TEXT SEARCH DICTIONARY on the schema
> pg_conflict.
> Is this expected?
>
> postgres=3D# CREATE EXTENSION hstore WITH SCHEMA pg_conflict;
> CREATE EXTENSION
> postgres=3D# CREATE EXTENSION pg_walinspect WITH SCHEMA pg_conflict; > CREATE EXTENSION
> postgres=3D# CREATE COLLATION pg_conflict.mycollation (provider =3D > libc,locale =3D 'C');
> CREATE COLLATION
> postgres=3D# CREATE TEXT SEARCH DICTIONARY pg_conflict.simple_dict
> (TEMPLATE =3D pg_catalog.simple);
> CREATE TEXT SEARCH DICTIONARY
> postgres=3D# CREATE OPERATOR pg_conflict.=3D=3D=3D (LEFTARG =3D int, R= IGHTARG =3D
> int, PROCEDURE =3D int4eq);
> CREATE OPERATOR
>
> Also, when we create extension several objects are created in the sche= ma:
> eg:
>=C2=A0 extname |=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0object
> ---------+------------------------------------------------------------= ------------------------------------------------
>=C2=A0 hstore=C2=A0 | type pg_conflict.hstore
>=C2=A0 hstore=C2=A0 | function pg_conflict.hstore_in(cstring)
>=C2=A0 hstore=C2=A0 | function pg_conflict.hstore_out(pg_conflict.hstor= e)
>=C2=A0 hstore=C2=A0 | function pg_conflict.hstore_recv(internal)
>=C2=A0 hstore=C2=A0 | function pg_conflict.hstore_send(pg_conflict.hsto= re)
>=C2=A0 hstore=C2=A0 | type pg_conflict.hstore[]
>=C2=A0 hstore=C2=A0 | function pg_conflict.hstore_version_diag(pg_confl= ict.hstore)
>=C2=A0 .
>=C2=A0 .
>
> So, should it be allowed?
>
> 2. When we try to DROP conflict log table we get an ERROR:
> postgres=3D# DROP TABLE pg_conflict.pg_conflict_log_16395;
> ERROR:=C2=A0 permission denied: "pg_conflict_log_16395" is a= system catalog
>
> But for other restricted operation such as UPDATE/INSERT we get the er= ror as:
> postgres=3D# INSERT INTO pg_conflict.pg_conflict_log_16395 VALUES (1);=
> ERROR:=C2=A0 cannot modify or insert data into conflict log table
> "pg_conflict_log_16395"
> DETAIL:=C2=A0 Conflict log tables are system-managed and only support<= br> > cleanup via DELETE or TRUNCATE.
>
> I think for the DROP case we should throw a similar error. Thoughts?
Please find the updated patch version (rebased on current HEAD), which
fixes all the open comments in 0001 and 0002. We are still working on
0003 regarding handling conflict log table insertion errors.


--
Regards,
Dilip Kumar
Google
--00000000000026574806549a6c6c--