Re: Question about behavior of deletes with REPLICA IDENTITY NOTHING

On Wed, Feb 7, 2024 at 6:04 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> On Thu, Feb 8, 2024 at 9:04 AM James Coleman <jtc331(at)gmail(dot)com> wrote:
> >
> > On Wed, Feb 7, 2024 at 3:22 PM Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> wrote:
> > >
> > > On Wed, 2024-02-07 at 15:12 -0500, James Coleman wrote:
> > > > We recently noticed some behavior that seems reasonable but also
> > > > surprised our engineers based on the docs.
> > > >
> > > > If we have this setup:
> > > > create table items(i int);
> > > > insert into items(i) values (1);
> > > > create publication test_pub for all tables;
> > > >
> > > > Then when we:
> > > > delete from items where i = 1;
> > > >
> > > > we get:
> > > > ERROR: cannot delete from table "items" because it does not have a
> > > > replica identity and publishes deletes
> > > > HINT: To enable deleting from the table, set REPLICA IDENTITY using
> > > > ALTER TABLE.
> > > >
> > > > Fair enough. But if we do this:
> > > > alter table items replica identity nothing;
> > > >
> > > > because the docs [1] say that NOTHING means "Records no information
> > > > about the old row." We still get the same error when we try the DELETE
> > > > again.
> > >
> > > Well, "REPLICA IDENTITY NOTHING" is the same as "has no replica identity".
> > > So is "REPLICA IDENTITY DEFAULT" if there is no primary key, or
> > > "REPLICA IDENTITY USING INDEX ..." if the index is dropped.
> > >
> > > See "pg_class": the column "relreplident" is not nullable.
> >
> > Right, I think the confusing point for us is that the docs for NOTHING
> > ("Records no information about the old row") imply you can decide you
> > don't have to record anything if you don't want to do so, but the
> > publication feature is effectively overriding that and asserting that
> > you can't make that choice.
> >
>
> Hi, I can see how the current docs could be interpreted in a way that
> was not intended.
>
> ~~~
>
> To emphasise the DEFAULT behaviour that Laurenze described, I felt
> there could be another sentence about DEFAULT, the same as there is
> already for the USING INDEX case.
>
> BEFORE [1]
> Records the old values of the columns of the primary key, if any. This
> is the default for non-system tables.
>
> SUGGESTION
> Records the old values of the columns of the primary key, if any. This
> is the default for non-system tables. If there is no primary key, the
> behavior is the same as NOTHING.
>
> ~~~
>
> If that is done, then would a publication docs tweak like the one
> below clarify things sufficiently?
>
> BEFORE [2]
> If a table without a replica identity is added to a publication that
> replicates UPDATE or DELETE operations then subsequent UPDATE or
> DELETE operations will cause an error on the publisher.
>
> SUGGESTION
> If a table without a replica identity (or with replica identity
> behavior equivalent to NOTHING) is added to a publication that
> replicates UPDATE or DELETE operations then subsequent UPDATE or
> DELETE operations will cause an error on the publisher.
>
> ======
> [1] https://www.postgresql.org/docs/current/sql-altertable.html#SQL-ALTERTABLE-REPLICA-IDENTITY
> [2] https://www.postgresql.org/docs/current/logical-replication-publication.html
>
> Kind Regards,
> Peter Smith.
> Fujitsu Australia

Thanks for looking at this!

Yes, both of those changes together would make this unambiguous (and,
I think, easier to mentally parse).

Thanks,
James Coleman