Re: Pgoutput not capturing the generated columns

On Sat, Aug 10, 2024 at 7:53 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Thu, 8 Aug 2024 at 10:53, Shubham Khanna <khannashubham1197(at)gmail(dot)com> wrote:
> >
> > On Wed, Aug 7, 2024 at 1:31 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > >
> > > Hi Shubham,
> > >
> > > Here are my review comments for patch v24-0001
> > >
> > > I think the TAP tests have incorrect expected results for the nogen-to-gen case.
> > >
> > > Whereas the HEAD code will cause "ERROR" for this test scenario, patch
> > > 0001 does not. IMO the behaviour should be unchanged for this scenario
> > > which has no generated column on the publisher side. So it seems this
> > > is a bug in patch 0001.
> > >
> > > FYI, I have included "FIXME" comments in the attached top-up diff
> > > patch to show which test cases I think are expecting wrong results.
> > >
> >
> > Fixed all the comments. The attached Patch(v25-0001) contains all the changes.
>
> Few comments:
> 1) Can we add one test with replica identity full to show that
> generated column is included in case of update operation with
> test_decoding.
>
> 2) At the end of the file generated_columns.sql a newline is missing:
> +-- when 'include-generated-columns' = '0' the generated column 'b'
> values will not be replicated
> +INSERT INTO gencoltable (a) VALUES (7), (8), (9);
> +SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL,
> NULL, 'include-xids', '0', 'skip-empty-xacts', '1',
> 'include-generated-columns', '0');
> +
> +DROP TABLE gencoltable;
> +
> +SELECT 'stop' FROM pg_drop_replication_slot('regression_slot');
> \ No newline at end of file
>
> 3)
> 3.a)This can be changed:
> +-- when 'include-generated-columns' is not set the generated column
> 'b' values will be replicated
> +INSERT INTO gencoltable (a) VALUES (1), (2), (3);
>
> to:
> -- By default, 'include-generated-columns' is enabled, so the values
> for the generated column 'b' will be replicated even if it is not
> explicitly specified.
>
> 3.b) This can be changed:
> -- when 'include-generated-columns' = '1' the generated column 'b'
> values will be replicated
> to:
> -- when 'include-generated-columns' is enabled, the values of the
> generated column 'b' will be replicated.
>
> 3.c) This can be changed:
> -- when 'include-generated-columns' = '0' the generated column 'b'
> values will not be replicated
> to:
> -- when 'include-generated-columns' is disabled, the values of the
> generated column 'b' will not be replicated.
>
> 4) I did not see any test for dump, can we add one test for this.

Fixed all the given comments. The attached Patch v27-0001 contains all
the required changes. Also, I have created a separate Patch (v27-0002)
for TAP Tests related to 'include-generated-columns'

Thanks and Regards,
Shubham Khanna.