Re: Pgoutput not capturing the generated columns - Mailing list pgsql-hackers

On Mon, Sep 23, 2024 at 6:19 PM vignesh C <vignesh21@gmail.com> wrote:
>
> On Fri, 20 Sept 2024 at 17:15, Shubham Khanna
> <khannashubham1197@gmail.com> wrote:
> >
> > On Wed, Sep 11, 2024 at 8:55 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > I have fixed all the comments. The attached patches contain the desired changes.
> > Also the merging of 0001 and 0002 can be done once there are no
> > comments on the patch to help in reviewing.
>
> Few comments:
> 1) This commit  message seems wrong, currently irrespective of
> publish_generated_columns, the column specified in column list take
> preceedene:
> When 'publish_generated_columns' is false, generated columns are not
> replicated, even when present in a PUBLICATION col-list.
>
> 2) Since we have added pubgencols to pg_pubication.h we can specify
> "Bump catversion" in the commit message.
>
> 3) In create publication column list/publish_generated_columns
> documentation we should mention that if generated column is mentioned
> in column list, generated columns mentioned in column list will be
> replication irrespective of publish_generated_columns option.
>
> 4) This warning should be mentioned only if publish_generated_columns is false:
>                 if (TupleDescAttr(tupdesc, attnum - 1)->attgenerated)
> -                       ereport(ERROR,
> +                       ereport(WARNING,
>
> errcode(ERRCODE_INVALID_COLUMN_REFERENCE),
> -                                       errmsg("cannot use generated
> column \"%s\" in publication column list",
> +                                       errmsg("specified generated
> column \"%s\" in publication column list for publication with
> publish_generated_columns as false",
>                                                    colname));
>
> 5) These tests are not required for this feature:
> +       'ALTER PUBLICATION pub5 ADD TABLE test_table WHERE (col1 > 0);' => {
> +               create_order => 51,
> +               create_sql =>
> +                 'ALTER PUBLICATION pub5 ADD TABLE
> dump_test.test_table WHERE (col1 > 0);',
> +               regexp => qr/^
> +                       \QALTER PUBLICATION pub5 ADD TABLE ONLY
> dump_test.test_table WHERE ((col1 > 0));\E
> +                       /xm,
> +               like => { %full_runs, section_post_data => 1, },
> +               unlike => {
> +                       exclude_dump_test_schema => 1,
> +                       exclude_test_table => 1,
> +               },
> +       },
> +
> +       'ALTER PUBLICATION pub5 ADD TABLE test_second_table WHERE
> (col2 = \'test\');'
> +         => {
> +               create_order => 52,
> +               create_sql =>
> +                 'ALTER PUBLICATION pub5 ADD TABLE
> dump_test.test_second_table WHERE (col2 = \'test\');',
> +               regexp => qr/^
> +                       \QALTER PUBLICATION pub5 ADD TABLE ONLY
> dump_test.test_second_table WHERE ((col2 = 'test'::text));\E
> +                       /xm,
> +               like => { %full_runs, section_post_data => 1, },
> +               unlike => { exclude_dump_test_schema => 1, },
> +         },
>

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

[1] https://www.postgresql.org/message-id/CAHv8RjJkUdYCdK_bL3yvEV%3DzKrA2dsnZYa1VMT2H5v0%2BqbaGbA%40mail.gmail.com

Thanks and Regards,
Shubham Khanna.