Re: pgoutput incorrectly replaces missing values with NULL since PostgreSQL 15

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Nikhil Benesch <nikhil(dot)benesch(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "sean(at)materialize(dot)com" <sean(at)materialize(dot)com>, "petrosagg(at)materialize(dot)com" <petrosagg(at)materialize(dot)com>
Subject: Re: pgoutput incorrectly replaces missing values with NULL since PostgreSQL 15
Date: 2023-11-24 11:47:05
Message-ID: CAA4eK1JgXuigg41vS8jaQ=9k0t4-K89r6K=Mt3DetDnuVhns4w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Nov 23, 2023 at 2:33 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Nov 23, 2023 at 1:10 PM Nikhil Benesch <nikhil(dot)benesch(at)gmail(dot)com> wrote:
> >
> > While working on Materialize's streaming logical replication from Postgres [0],
> > my colleagues Sean Loiselle and Petros Angelatos (CC'd) discovered today what
> > appears to be a correctness bug in pgoutput, introduced in v15.
> >
> > The problem goes like this. A table with REPLICA IDENTITY FULL and some
> > data in it...
> >
> > CREATE TABLE t (a int);
> > ALTER TABLE t REPLICA IDENTITY FULL;
> > INSERT INTO t VALUES (1), (2), (3), ...;
> >
> > ...undergoes a schema change to add a new column with a default:
> >
> > ALTER TABLE t ADD COLUMN b bool DEFAULT false NOT NULL;
> >
> > PostgreSQL is smart and does not rewrite the entire table during the schema
> > change. Instead it updates the tuple description to indicate to future readers
> > of the table that if `b` is missing, it should be filled in with the default
> > value, `false`.
> >
> > Unfortunately, since v15, pgoutput mishandles missing attributes. If a
> > downstream server is subscribed to changes from t via the pgoutput plugin, when
> > a row with a missing attribute is updated, e.g.:
> >
> > UPDATE t SET a = 2 WHERE a = 1
> >
> > pgoutput will incorrectly report b's value as NULL in the old tuple, rather than
> > false.
> >
>
> Thanks, I could reproduce this behavior. I'll look into your patch.
>

I verified your fix is good and made minor modifications in the
comment. Note, that the test doesn't work for PG15, needs minor
modifications.

--
With Regards,
Amit Kapila.

Attachment Content-Type Size
v2-0001-Avoid-unconditionally-filling-in-missing-values-w.patch application/octet-stream 4.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Banck 2023-11-24 12:10:01 Re: [HACKERS] pg_upgrade vs vacuum_cost_delay
Previous Message Nikita Malakhov 2023-11-24 11:40:14 Re: Patch: Global Unique Index