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

On Friday, November 24, 2023 7:47 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> 
> On Thu, Nov 23, 2023 at 2:33 PM Amit Kapila <amit.kapila16@gmail.com>
> wrote:
> >
> > On Thu, Nov 23, 2023 at 1:10 PM Nikhil Benesch <nikhil.benesch@gmail.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 willz 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.

Thank you for fixing and reviewing the fix!

The fix also looks good to me. I verified that it can fix the problem in
HEAD ~ PG15 and the added tap test can detect the problem without the fix. I
tried to rebase the patch on PG15, and combines some queries into one safe_sql
block to simplify the code. Here are the patches for all branches.

Best Regards,
Hou zj