On Thu, Dec 2, 2021 at 6:18 PM Peter Smith <smithpb2250@gmail.com> wrote:
>
> PSA a new v44* patch set.
>
Some initial comments:
0001
src/backend/replication/logical/tablesync.c
(1) In fetch_remote_table_info update, "list_free(*qual);" should be
"list_free_deep(*qual);"
doc/src/sgml/ref/create_subscription.sgml
(2) Refer to Notes
Perhaps a link to the Notes section should be used here, as follows:
- copied. Refer to the Notes section below.
+ copied. Refer to the <xref
linkend="sql-createsubscription-notes"/> section below.
- <refsect1>
+ <refsect1 id="sql-createsubscription-notes" xreflabel="Notes">
0002
1) Typo in patch comment
"Specifially"
src/backend/catalog/pg_publication.c
2) bms_replident comment
Member "Bitmapset *bms_replident;" in rf_context should have a
comment, maybe something like "set of replica identity col indexes".
3) errdetail message
In rowfilter_walker(), the "forbidden" errdetail message is loaded
using gettext() in one instance, but just a raw formatted string in
other cases. Shouldn't they all consistently be translated strings?
0003
src/backend/replication/logical/proto.c
1) logicalrep_write_tuple
(i)
if (slot == NULL || TTS_EMPTY(slot))
can be replaced with:
if (TupIsNull(slot))
(ii) In the above case (where values and nulls are palloc'd),
shouldn't the values and nulls be pfree()d at the end of the function?
0005
src/backend/utils/cache/relcache.c
(1) RelationGetInvalRowFilterCol
Shouldn't "rfnode" be pfree()d after use?
Regards,
Greg Nancarrow
Fujitsu Australia
