On Tue, Oct 22, 2024 at 9:42 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Tue, Oct 22, 2024 at 3:50 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > Considering this, I feel if find this behavior buggy then we should > > fix this separately rather than part of this patch. What do you think? > > Agreed. It's better to fix it separately. >
Thanks. One more thing that I didn't like about the patch is that it used column_list to address the "publish_generated_columns = false" case such that we build column_list without generated columns for the same. The first problem is that it will add overhead to always probe column_list during proto.c calls (for example during logicalrep_write_attrs()), then it makes the column_list code complex especially the handling in pgoutput_column_list_init(), and finally this appears to be a misuse of column_list. So, I suggest remembering this information in RelationSyncEntry and then using it at the required places. We discussed above that contradictory values of "publish_generated_columns" across publications for the same relations are not accepted, so we can detect that during get_rel_sync_entry() and give an ERROR for the same. Additional comment on the 0003 patch +# ============================================================================= +# Misc test. +# +# A "normal -> generated" replication. +# +# In this test case we use DROP EXPRESSION to change the subscriber generated +# column into a normal column, then verify replication works ok. +# ============================================================================= In patch 0003, why do we have the above test? This doesn't seem to be directly related to this patch. -- With Regards, Amit Kapila.
