Hi, here are some review comments for patch v19-0003 ====== src/backend/catalog/pg_publication.c
1. /* * Translate a list of column names to an array of attribute numbers * and a Bitmapset with them; verify that each attribute is appropriate * to have in a publication column list (no system or generated attributes, * no duplicates). Additional checks with replica identity are done later; * see pub_collist_contains_invalid_column. * * Note that the attribute numbers are *not* offset by * FirstLowInvalidHeapAttributeNumber; system columns are forbidden so this * is okay. */ static void publication_translate_columns(Relation targetrel, List *columns, int *natts, AttrNumber **attrs) ~ I though the above comment ought to change: /or generated attributes/or virtual generated attributes/ IIRC this was already addressed back in v16, but somehow that fix has been lost (???). ====== src/backend/replication/logical/tablesync.c fetch_remote_table_info: nitpick - missing end space in this comment /* TODO: use ATTRIBUTE_GENERATED_VIRTUAL*/ ====== 2. (in patch v19-0001) +# tab3: +# publisher-side tab3 has generated col 'b'. +# subscriber-side tab3 has generated col 'b', using a different computation. (here, in patch v19-0003) # tab3: -# publisher-side tab3 has generated col 'b'. -# subscriber-side tab3 has generated col 'b', using a different computation. +# publisher-side tab3 has stored generated col 'b' but +# subscriber-side tab3 has DIFFERENT COMPUTATION stored generated col 'b'. It has become difficult to review these TAP tests, particularly when different patches are modifying the same comment. e.g. I post suggestions to modify comments for patch 0001. Those get addressed OK, only to vanish in subsequent patches like has happened in the above example. Really this patch 0003 was only supposed to add the word "stored", not revert the entire comment to something from an earlier version. Please take care that all comment changes are carried forward correctly from one patch to the next. ====== Kind Regards, Peter Smith. Fujitsu Australia.
