Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY

From: Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>
To: Aleksander Alekseev <aleksander(at)timescale(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY
Date: 2024-11-06 10:19:48
Message-ID: CANhcyEWcftppqdFN7kBbUq11DmEhewb15fcu8Jc-S0SOQqw53A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Aleksander,

>
> > Here the generated column 'b' is set as REPLICA IDENTITY for table
> > 'testpub_gencol'. When we create publication 'pub_gencol' we do not
> > specify any column list, so column 'b' will not be published.
> > So, the update message generated by the last UPDATE would have NULL
> > for column 'b'.
> >
> > To avoid the issue, we can disallow UPDATE/DELETE on table with
> > unpublished generated column as REPLICA IDENTITY. I have attached a
> > patch for the same.
>
> I don't think this would be a correct fix. Let's say I *don't* have
> any publications:
>
> ```
> =# CREATE TABLE testpub_gencol (a INT, b INT GENERATED ALWAYS AS (a + 1)
> STORED NOT NULL);
> CREATE TABLE
>
> =# CREATE UNIQUE INDEX testpub_gencol_idx ON testpub_gencol (b);
> CREATE INDEX
>
> =# INSERT INTO testpub_gencol (a) VALUES (1);
> INSERT 0 1
>
> =# UPDATE testpub_gencol SET a = 100 WHERE a = 1;
> UPDATE 1
> eax=# SELECT * FROM testpub_gencol ;
> a | b
> -----+-----
> 100 | 101
> (1 row)
> ```
>
> So far everything works fine. You are saying that when one creates a
> publication UPDATEs should stop working. That would be rather
> surprising behavior for a typical user not to mention that it will
> break the current behavior.
>
> I believe one would expect that both UPDATEs and the publication
> should continue to work. Perhaps we should forbid the creation of a
> publication like this instead. Or alternatively include a generated
> column to the publication list if it's used as a replica identity. Or
> maybe even keep everything as is.
>
> Thoughts?
>

While testing I found that similar behaviors already exist in some
cases. Where once we create a publication UPDATES might stop working.
For example:
Case1:
postgres=# create table t1(c1 int);
CREATE TABLE
postgres=# insert into t1 values(1);
INSERT 0 1
postgres=# update t1 set c1 = 100 where c1 = 1;
UPDATE 1
postgres=# create publication pub for table t1;
CREATE PUBLICATION
postgres=# update t1 set c1 = 100 where c1 = 1;
ERROR: cannot update table "t1" because it does not have a replica
identity and publishes updates
HINT: To enable updating the table, set REPLICA IDENTITY using ALTER TABLE.

Case2:
postgres=# create table t2(c1 int, c2 int not null);
CREATE TABLE
postgres=# create unique index t2_idx on t2 (c2);
CREATE INDEX
postgres=# alter table t2 replica identity using index t2_idx;
ALTER TABLE
postgres=# insert into t2 values(1,1);
INSERT 0 1
postgres=# update t2 set c1 = 100 where c1 = 1;
UPDATE 1
postgres=# create publication pub2 for table t2 where (c1 > 10);
CREATE PUBLICATION
postgres=# update t2 set c1 = 100 where c1 = 1;
ERROR: cannot update table "t2"
DETAIL: Column used in the publication WHERE expression is not part
of the replica identity.

Behaviour with the patch provided in [1] to resolve the issue:
postgres=# create table t3(c1 int, c2 INT GENERATED ALWAYS AS (c1 + 1)
STORED NOT NULL);
CREATE TABLE
postgres=# create unique index t3_idx on t3 (c2);
CREATE INDEX
postgres=# alter table t3 replica identity using index t3_idx;
ALTER TABLE
postgres=# insert into t3 values(1);
INSERT 0 1
postgres=# update t3 set c1 = 100 where c1 = 1;
UPDATE 1
postgres=# create publication pub3 for table t3;
CREATE PUBLICATION
postgres=# update t3 set c1 = 100 where c1 = 1;
ERROR: cannot update table "t3"
DETAIL: Column list used by the publication does not cover the
replica identity.

So, I think this behavior would be acceptable. Thoughts?

[1]: https://www.postgresql.org/message-id/CANhcyEVw4V2Awe2AB6i0E5AJLNdASShGfdBLbUd1XtWDboymCA%40mail.gmail.com

Thanks and Regards,
Shlok Kyal

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2024-11-06 10:36:42 Re: Time to add a Git .mailmap?
Previous Message Amit Kapila 2024-11-06 10:01:10 Re: DOCS - pg_replication_slot . Fix the 'inactive_since' description