Re: Pgoutput not capturing the generated columns

From: Shubham Khanna <khannashubham1197(at)gmail(dot)com>
To: Peter Smith <smithpb2250(at)gmail(dot)com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Rajendra Kumar Dangwal <dangwalrajendra888(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "euler(at)eulerto(dot)com" <euler(at)eulerto(dot)com>
Subject: Re: Pgoutput not capturing the generated columns
Date: 2024-07-16 11:23:45
Message-ID: CAHv8RjKQJTmapc=6CPsGsJPTEom2LxF+EctWDNh9XxtvL2vJ9g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jul 15, 2024 at 11:09 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi, I had a quick look at the patch v17-0004 which is the split-off
> new BMS logic.
>
> IIUC this 0004 is currently undergoing some refactoring and
> cleaning-up, so I won't comment much about it except to give the
> following observation below.
>
> ======
> src/backend/replication/logical/proto.c.
>
> I did not expect to see any code fragments that are still checking
> generated columns like below:
>
> logicalrep_write_tuple:
>
> if (att->attgenerated)
> {
> - if (!include_generated_columns)
> - continue;
>
> if (att->attgenerated != ATTRIBUTE_GENERATED_STORED)
> continue;
> ~
>
> if (att->attgenerated)
> {
> - if (!include_generated_columns)
> - continue;
>
> if (att->attgenerated != ATTRIBUTE_GENERATED_STORED)
> continue;
>
> ~~~
>
> logicalrep_write_attrs:
>
> if (att->attgenerated)
> {
> - if (!include_generated_columns)
> - continue;
>
> if (att->attgenerated != ATTRIBUTE_GENERATED_STORED)
> continue;
>
> ~
> if (att->attgenerated)
> {
> - if (!include_generated_columns)
> - continue;
>
> if (att->attgenerated != ATTRIBUTE_GENERATED_STORED)
> continue;
> ~~~
>
>
> AFAIK, now checking support of generated columns will be done when the
> BMS 'columns' is assigned, so the continuation code will be handled
> like this:
>
> if (!column_in_column_list(att->attnum, columns))
> continue;
>
> ======
>
> BTW there is a subtle but significant difference in this 0004 patch.
> IOW, we are introducing a difference between the list of published
> columns VERSUS a publication column list. So please make sure that all
> code comments are adjusted appropriately so they are not misleading by
> calling these "column lists" still.
>
> BEFORE: BMS 'columns' means "columns of the column list" or NULL if
> there was no publication column list
> AFTER: BMS 'columns' means "columns to be replicated" or NULL if all
> columns are to be replicated

I have addressed all the comments in v19-0004 patch.
Please refer to the updated v19-0004 Patch here in [1]. See [1] for
the changes added.

[1] https://www.postgresql.org/message-id/CAHv8Rj%2BR0cj%3Dz1bTMAgQKQWx1EKvkMEnV9QsHGvOqTdnLUQi1A%40mail.gmail.com

Thanks and Regards,
Shubham Khanna.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Yasir 2024-07-16 11:46:46 ActiveState Perl is not valid anymore to build PG17 on the Windows 10/11 platforms, So Documentation still suggesting it should be updated
Previous Message Shubham Khanna 2024-07-16 11:19:42 Re: Pgoutput not capturing the generated columns