Hi Kuroda-san,
Thanks for reviewing the patch. I have fixed some of the comments
> 2.
> Currently, the option is implemented as streaming option. Are there any reasons
> to choose the way? Another approach is to implement as slot option, like failover
> and temporary.
I think the current approach is appropriate. The options such as
failover and temporary seem like properties of a slot and I think
decoding of generated column should not be slot specific. Also adding
a new option for slot may create an overhead.
> 3.
> You said that subscription option is not supported for now. Not sure, is it mean
> that logical replication feature cannot be used for generated columns? If so,
> the restriction won't be acceptable. If the combination between this and initial
> sync is problematic, can't we exclude them in CreateSubscrition and AlterSubscription?
> E.g., create_slot option cannot be set if slot_name is NONE.
Added an option 'generated_column' for create subscription. Currently
it allow to set 'generated_column' option as true only if 'copy_data'
is set to false.
Also we don't allow user to alter the 'generated_column' option.
> 6. logicalrep_write_tuple()
>
> ```
> - if (!column_in_column_list(att->attnum, columns))
> + if (!column_in_column_list(att->attnum, columns) && !att->attgenerated)
> + continue;
> ```
>
> Hmm, does above mean that generated columns are decoded even if they are not in
> the column list? If so, why? I think such columns should not be sent.
Fixed
Thanks and Regards,
Shlok Kyal
