Re: Pgoutput not capturing the generated columns

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: Shubham Khanna <khannashubham1197(at)gmail(dot)com>, Rajendra Kumar Dangwal <dangwalrajendra888(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, euler(at)eulerto(dot)com
Subject: Re: Pgoutput not capturing the generated columns
Date: 2024-09-10 04:14:55
Message-ID: CAA4eK1+V8X8ax+W9B1prPV=geLL3vdnMASiZ00+BeWeOpDJm7A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Sep 10, 2024 at 2:51 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Mon, Sep 9, 2024 at 2:38 AM Shubham Khanna
> <khannashubham1197(at)gmail(dot)com> wrote:
> >
>
> Thank you for updating the patches. I have some comments:
>
> Do we really need to add this option to test_decoding?
>

I don't see any reason to have such an option in test_decoding,
otherwise, we need a separate option for each publication option. I
guess this is leftover of the previous subscriber-side approach.

> I think it
> would be good if this improves the test coverage. Otherwise, I'm not
> sure we need this part. If we want to add it, I think it would be
> better to have it in a separate patch.
>

Right.

> ---
> + <para>
> + If the publisher-side column is also a generated column
> then this option
> + has no effect; the publisher column will be filled as normal with the
> + publisher-side computed or default data.
> + </para>
>
> I don't understand this description. Why does this option have no
> effect if the publisher-side column is a generated column?
>

Shouldn't it be subscriber-side?

I have one additional comment:
/*
- * If the publication is FOR ALL TABLES then it is treated the same as
- * if there are no column lists (even if other publications have a
- * list).
+ * If the publication is FOR ALL TABLES and include generated columns
+ * then it is treated the same as if there are no column lists (even
+ * if other publications have a list).
*/
- if (!pub->alltables)
+ if (!pub->alltables || !pub->pubgencolumns)

Why do we treat pubgencolumns at the same level as the FOR ALL TABLES
case? I thought that if the user has provided a column list, we only
need to publish the specified columns even when the
publish_generated_columns option is set.

--
With Regards,
Amit Kapila.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Zhijie Hou (Fujitsu) 2024-09-10 04:25:24 RE: long-standing data loss bug in initial sync of logical replication
Previous Message Ashutosh Bapat 2024-09-10 04:06:21 Re: change "attnum <=0" to "attnum <0" for better reflect system attribute