Re: Pgoutput not capturing the generated columns

On Tue, Oct 22, 2024 at 3:50 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Oct 9, 2024 at 10:19 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > Regarding the 0001 patch, it seems to me that UPDATE and DELETE are
> > allowed on the table even if its replica identity is set to generated
> > columns that are not published. For example, consider the following
> > scenario:
> >
> > create table t (a int not null, b int generated always as (a + 1)
> > stored not null);
> > create unique index t_idx on t (b);
> > alter table t replica identity using index t_idx;
> > create publication pub for table t with (publish_generated_columns = false);
> > insert into t values (1);
> > update t set a = 100 where a = 1;
> >
> > The publication pub doesn't include the generated column 'b' which is
> > the replica identity of the table 't'. Therefore, the update message
> > generated by the last UPDATE would have NULL for the column 'b'. I
> > think we should not allow UPDATE and DELETE on such a table.
> >
>
> I see the same behavior even without a patch on the HEAD. See the
> following example executed on HEAD:
>
> postgres=# create table t (a int not null, b int generated always as (a + 1)
> postgres(# stored not null);
> CREATE TABLE
> postgres=# create unique index t_idx on t (b);
> CREATE INDEX
> postgres=# alter table t replica identity using index t_idx;
> ALTER TABLE
> postgres=# create publication pub for table t;
> CREATE PUBLICATION
> postgres=# insert into t values (1);
> INSERT 0 1
> postgres=# update t set a = 100 where a = 1;
> UPDATE 1
>
> So, the update is allowed even when we don't publish generated
> columns, if so, why do we need to handle it in this patch when the
> user gave publish_generated_columns=false?
>
> Also, on the subscriber side, I see the ERROR: "publisher did not send
> replica identity column expected by the logical replication target
> relation "public.t"".

Good point.

> Considering this, I feel if find this behavior buggy then we should
> fix this separately rather than part of this patch. What do you think?

Agreed. It's better to fix it separately.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com