Horton
Menu

Re: Pgoutput not capturing the generated columns

Shubham Khanna <khannashubham1197@gmail.com>

View thread
From: Shubham Khanna <khannashubham1197@gmail.com>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: Rajendra Kumar Dangwal <dangwalrajendra888@gmail.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, "euler@eulerto.com" <euler@eulerto.com>
Date: 2024-05-23T03:49:03Z
Lists: pgsql-hackers

Attachments

> Dear Shubham,
>
> Thanks for creating a patch! Here are high-level comments.

> 1.
> Please document the feature. If it is hard to describe, we should change the API.

I have added the feature in the document.

> 4.
> Regarding the test_decoding plugin, it has already been able to decode the
> generated columns. So... as the first place, is the proposed option really needed
> for the plugin? Why do you include it?
> If you anyway want to add the option, the default value should be on - which keeps
> current behavior.

I have made the generated column options as true for test_decoding
plugin so by default we will send generated column data.

> 5.
> Assuming that the feature become usable used for logical replicaiton. Not sure,
> should we change the protocol version at that time? Nodes prior than PG17 may
> not want receive values for generated columns. Can we control only by the option?

I verified the backward compatibility test by using the generated
column option and it worked fine. I think there is no need to make any
further changes.

> 7.
>
> Some functions refer data->publish_generated_column many times. Can we store
> the value to a variable?
>
> Below comments are for test_decoding part, but they may be not needed.
>
> =====
>
> a. pg_decode_startup()
>
> ```
> +        else if (strcmp(elem->defname, "include_generated_columns") == 0)
> ```
>
> Other options for test_decoding do not have underscore. It should be
> "include-generated-columns".
>
> b. pg_decode_change()
>
> data->include_generated_columns is referred four times in the function.
> Can you store the value to a varibable?
>
>
> c. pg_decode_change()
>
> ```
> -                                    true);
> +                                    true, data->include_generated_columns );
> ```
>
> Please remove the blank.

Fixed.
The attached v3 Patch has the changes for the same.

Thanks and Regards,
Shubham Khanna.

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Doc: Generated column replication.

  2. Rename pubgencols_type to pubgencols in pg_publication.

  3. Doc: Fix column name in pg_publication catalog.

  4. Fix buildfarm failure introduced by commit e65dbc9927.

  5. Change publication's publish_generated_columns option type to enum.

  6. Fix \dRp+ output when describing publications with a lower server version.

  7. Replicate generated columns when 'publish_generated_columns' is set.

  8. Doc: Update the behavior of generated columns in Logical Replication.

  9. Replicate generated columns when specified in the column list.

  10. Doc: Generated columns are skipped for logical replication.