On Mon, Jul 15, 2024 at 11:09 AM Peter Smith <smithpb2...@gmail.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.
