Thread: deferred primary key and logical replication
Hi,
--
While looking at an old wal2json issue, I stumbled on a scenario that a table
with a deferred primary key is not updatable in logical replication. AFAICS it
has been like that since the beginning of logical decoding and seems to be an
oversight while designing logical decoding. I don't envision a problem with a
deferred primary key in an after commit scenario. Am I missing something?
Just in case, I'm attaching a patch to fix it and also add a test to cover this
scenario.
with a deferred primary key is not updatable in logical replication. AFAICS it
has been like that since the beginning of logical decoding and seems to be an
oversight while designing logical decoding. I don't envision a problem with a
deferred primary key in an after commit scenario. Am I missing something?
Just in case, I'm attaching a patch to fix it and also add a test to cover this
scenario.
Euler Taveira http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
The patch no longer applies, because of additions in the test source. Otherwise, I have tested the patch and confirmed thatupdates and deletes on tables with deferred primary keys work with logical replication. The new status of this patch is: Waiting on Author
On Fri, 24 Jul 2020 at 05:16, Ajin Cherian <itsajin@gmail.com> wrote:
The patch no longer applies, because of additions in the test source. Otherwise, I have tested the patch and confirmed that updates and deletes on tables with deferred primary keys work with logical replication.
The new status of this patch is: Waiting on Author
Euler Taveira http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachment
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: tested, passed Spec compliant: not tested Documentation: not tested Patch applies cleanly. Tested that update/delete of tables with deferred primary keys now work with logical replication.Code/comments look fine. The new status of this patch is: Ready for Committer
On Mon, May 11, 2020 at 2:41 AM Euler Taveira <euler.taveira@2ndquadrant.com> wrote: > > Hi, > > While looking at an old wal2json issue, I stumbled on a scenario that a table > with a deferred primary key is not updatable in logical replication. AFAICS it > has been like that since the beginning of logical decoding and seems to be an > oversight while designing logical decoding. > I am not sure if it is an oversight because we document that the index must be non-deferrable, see "USING INDEX records the old values of the columns covered by the named index, which must be unique, not partial, not deferrable, and include only columns marked NOT NULL." in docs [1]. Now sure this constraint is when we use USING INDEX for REPLICA IDENTITY but why it has to be different for PRIMARY KEY especially when UNIQUE constraint will have similar behavior and the same is documented? [1] - https://www.postgresql.org/docs/devel/sql-altertable.html -- With Regards, Amit Kapila.
On Mon, 5 Oct 2020 at 08:34, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Mon, May 11, 2020 at 2:41 AM Euler Taveira
<euler.taveira@2ndquadrant.com> wrote:
>
> Hi,
>
> While looking at an old wal2json issue, I stumbled on a scenario that a table
> with a deferred primary key is not updatable in logical replication. AFAICS it
> has been like that since the beginning of logical decoding and seems to be an
> oversight while designing logical decoding.
>
I am not sure if it is an oversight because we document that the index
must be non-deferrable, see "USING INDEX records the old values of the
columns covered by the named index, which must be unique, not partial,
not deferrable, and include only columns marked NOT NULL." in docs
[1].
Inspecting this patch again, I forgot to consider that RelationGetIndexList()
is called by other backend modules. Since logical decoding deals with finished
transactions, it is ok to use a deferrable primary key. However, this patch is
probably wrong because it does not consider the other modules.
is called by other backend modules. Since logical decoding deals with finished
transactions, it is ok to use a deferrable primary key. However, this patch is
probably wrong because it does not consider the other modules.
Euler Taveira http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Sun, Oct 25, 2020 at 9:39 PM Euler Taveira <euler.taveira@2ndquadrant.com> wrote: > > On Mon, 5 Oct 2020 at 08:34, Amit Kapila <amit.kapila16@gmail.com> wrote: >> >> On Mon, May 11, 2020 at 2:41 AM Euler Taveira >> <euler.taveira@2ndquadrant.com> wrote: >> > >> > Hi, >> > >> > While looking at an old wal2json issue, I stumbled on a scenario that a table >> > with a deferred primary key is not updatable in logical replication. AFAICS it >> > has been like that since the beginning of logical decoding and seems to be an >> > oversight while designing logical decoding. >> > >> >> I am not sure if it is an oversight because we document that the index >> must be non-deferrable, see "USING INDEX records the old values of the >> columns covered by the named index, which must be unique, not partial, >> not deferrable, and include only columns marked NOT NULL." in docs >> [1]. >> > > Inspecting this patch again, I forgot to consider that RelationGetIndexList() > is called by other backend modules. Since logical decoding deals with finished > transactions, it is ok to use a deferrable primary key. > But starting PG-14, we do support logical decoding of in-progress transactions as well. -- With Regards, Amit Kapila.
On 27.10.2020 13:46, Amit Kapila wrote: > On Sun, Oct 25, 2020 at 9:39 PM Euler Taveira > <euler.taveira@2ndquadrant.com> wrote: >> On Mon, 5 Oct 2020 at 08:34, Amit Kapila <amit.kapila16@gmail.com> wrote: >>> On Mon, May 11, 2020 at 2:41 AM Euler Taveira >>> <euler.taveira@2ndquadrant.com> wrote: >>>> Hi, >>>> >>>> While looking at an old wal2json issue, I stumbled on a scenario that a table >>>> with a deferred primary key is not updatable in logical replication. AFAICS it >>>> has been like that since the beginning of logical decoding and seems to be an >>>> oversight while designing logical decoding. >>>> >>> I am not sure if it is an oversight because we document that the index >>> must be non-deferrable, see "USING INDEX records the old values of the >>> columns covered by the named index, which must be unique, not partial, >>> not deferrable, and include only columns marked NOT NULL." in docs >>> [1]. >>> >> Inspecting this patch again, I forgot to consider that RelationGetIndexList() >> is called by other backend modules. Since logical decoding deals with finished >> transactions, it is ok to use a deferrable primary key. >> > But starting PG-14, we do support logical decoding of in-progress > transactions as well. > > Commitfest entry status update. As far as I see, this patch needs some further work, so I move it to "Waiting on author". Euler, are you going to continue working on it? -- Anastasia Lubennikova Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
On Tue, Nov 24, 2020 at 3:04 AM Anastasia Lubennikova <a.lubennikova@postgrespro.ru> wrote: > > On 27.10.2020 13:46, Amit Kapila wrote: > > On Sun, Oct 25, 2020 at 9:39 PM Euler Taveira > > <euler.taveira@2ndquadrant.com> wrote: > >> On Mon, 5 Oct 2020 at 08:34, Amit Kapila <amit.kapila16@gmail.com> wrote: > >>> On Mon, May 11, 2020 at 2:41 AM Euler Taveira > >>> <euler.taveira@2ndquadrant.com> wrote: > >>>> Hi, > >>>> > >>>> While looking at an old wal2json issue, I stumbled on a scenario that a table > >>>> with a deferred primary key is not updatable in logical replication. AFAICS it > >>>> has been like that since the beginning of logical decoding and seems to be an > >>>> oversight while designing logical decoding. > >>>> > >>> I am not sure if it is an oversight because we document that the index > >>> must be non-deferrable, see "USING INDEX records the old values of the > >>> columns covered by the named index, which must be unique, not partial, > >>> not deferrable, and include only columns marked NOT NULL." in docs > >>> [1]. > >>> > >> Inspecting this patch again, I forgot to consider that RelationGetIndexList() > >> is called by other backend modules. Since logical decoding deals with finished > >> transactions, it is ok to use a deferrable primary key. > >> > > But starting PG-14, we do support logical decoding of in-progress > > transactions as well. > > > > > Commitfest entry status update. > As far as I see, this patch needs some further work, so I move it to > "Waiting on author". > I think this should be marked as "Returned with Feedback" as there is no response to the feedback for a long time and also it is not very clear if this possible. -- With Regards, Amit Kapila.