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.
Thanks and Regards,
Shubham Khanna.
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 |