> Hi Shubham, thanks for providing a patch. > I have some comments for v6-0001. > > 1. create_subscription.sgml > There is repetition of the same line. > > + <para> > + Specifies whether the generated columns present in the tables > + associated with the subscription should be replicated. If the > + subscriber-side column is also a generated column then this option > + has no effect; the replicated data will be ignored and the > subscriber > + column will be filled as normal with the subscriber-side computed > or > + default data. > + <literal>false</literal>. > + </para> > + > + <para> > + This parameter can only be set true if > <literal>copy_data</literal> is > + set to <literal>false</literal>. If the subscriber-side > column is also a > + generated column then this option has no effect; the > replicated data will > + be ignored and the subscriber column will be filled as > normal with the > + subscriber-side computed or default data. > + </para> > > ============================== > 2. subscriptioncmds.c > > 2a. The macro name should be in uppercase. We can use a short name > like 'SUBOPT_INCLUDE_GEN_COL'. Thought? > +#define SUBOPT_include_generated_columns 0x00010000 > > 2b.Update macro name accordingly > + if (IsSet(supported_opts, SUBOPT_include_generated_columns)) > + opts->include_generated_columns = false; > > 2c. Update macro name accordingly > + else if (IsSet(supported_opts, SUBOPT_include_generated_columns) && > + strcmp(defel->defname, "include_generated_columns") == 0) > + { > + if (IsSet(opts->specified_opts, SUBOPT_include_generated_columns)) > + errorConflictingDefElem(defel, pstate); > + > + opts->specified_opts |= SUBOPT_include_generated_columns; > + opts->include_generated_columns = defGetBoolean(defel); > + } > > 2d. Update macro name accordingly > + SUBOPT_RUN_AS_OWNER | SUBOPT_FAILOVER | SUBOPT_ORIGIN | > + SUBOPT_include_generated_columns); > > > ============================== > > 3. decoding_into_rel.out > > 3a. In comment, I think it should be "When 'include-generated-columns' > = '1' the generated column 'b' values will be replicated" > +-- When 'include-generated-columns' = '1' the generated column 'b' > values will not be replicated > +INSERT INTO gencoltable (a) VALUES (1), (2), (3); > +SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, > NULL, 'include-xids', '0', 'skip-empty-xacts', '1', > 'include-generated-columns', '1'); > + data > +------------------------------------------------------------- > + BEGIN > + table public.gencoltable: INSERT: a[integer]:1 b[integer]:2 > + table public.gencoltable: INSERT: a[integer]:2 b[integer]:4 > + table public.gencoltable: INSERT: a[integer]:3 b[integer]:6 > + COMMIT > > 3b. In comment, I think it should be "When 'include-generated-columns' > = '1' the generated column 'b' values will not be replicated" > +-- When 'include-generated-columns' = '0' the generated column 'b' > values will be replicated > +SELECT data FROM pg_logical_slot_get_changes('regression_slot', NULL, > NULL, 'include-xids', '0', 'skip-empty-xacts', '1', > 'include-generated-columns', '0'); > + data > +------------------------------------------------ > + BEGIN > + table public.gencoltable: INSERT: a[integer]:4 > + table public.gencoltable: INSERT: a[integer]:5 > + table public.gencoltable: INSERT: a[integer]:6 > + COMMIT > +(5 rows) > > ========================= > > 4. Here names for both the tests are the same. I think we should use > different names. > > +$result = $node_subscriber->safe_psql('postgres', "SELECT * FROM tab2"); > +is( $result, qq(4|8 > +5|10), 'generated columns replicated to non-generated column on subscriber'); > + > +$node_publisher->safe_psql('postgres', "INSERT INTO tab3 VALUES (4), (5)"); > + > +$node_publisher->wait_for_catchup('sub3'); > + > +$result = $node_subscriber->safe_psql('postgres', "SELECT * FROM tab3"); > +is( $result, qq(4|24 > +5|25), 'generated columns replicated to non-generated column on subscriber');
All the comments are handled. The attached Patch contains all the suggested changes. Thanks and Regards, Shubham Khanna.
v8-0001-Currently-generated-column-values-are-not-replica.patch
Description: Binary data