Hi, here are some review comments about patch v17-0003
======
1.
Missing a docs change?
Previously, (v16-0002) the patch included a change to
doc/src/sgml/protocol.sgml like below to say STORED generated instead
of just generated.
<para>
- Boolean option to enable generated columns. This option controls
- whether generated columns should be included in the string
- representation of tuples during logical decoding in PostgreSQL.
+ Boolean option to enable <literal>STORED</literal> generated columns.
+ This option controls whether <literal>STORED</literal>
generated columns
+ should be included in the string representation of tuples
during logical
+ decoding in PostgreSQL.
</para>
Why is that v16 change no longer present in patch v17-0003?
======
src/backend/catalog/pg_publication.c
2.
Previously, (v16-0003) this patch included a change to clarify the
kind of generated cols that are allowed in a column list.
* 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.
+ * to have in a publication column list (no system or virtual generated
+ * attributes, no duplicates). Additional checks with replica identity
+ * are done later; see pub_collist_contains_invalid_column.
Why is that v16 change no longer present in patch v17-0003?
======
src/backend/replication/logical/tablesync.c
3. make_copy_attnamelist
- if (!attr->attgenerated)
+ if (attr->attgenerated != ATTRIBUTE_GENERATED_STORED)
continue;
IIUC this logic is checking to make sure the subscriber-side table
column was not a generated column (because we don't replicate on top
of generated columns). So, does the distinction of STORED/VIRTUAL
really matter here?
~~~
fetch_remote_table_info:
nitpick - Should not change any spaces unrelated to the patch
======
send_relation_and_attrs:
- if (att->attgenerated && !include_generated_columns)
+ if (att->attgenerated && (att->attgenerated !=
ATTRIBUTE_GENERATED_STORED || !include_generated_columns))
continue;
nitpick - It seems over-complicated. Conditions can be split so the
code fragment looks the same as in other places in this patch.
======
99.
Please see the attached diffs patch that implements any nitpicks
mentioned above.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
