Hello, this is cont'd comments.

> 0008 and after to come later..

I had nothing to comment for patch 0008.

===== 0009: 

 - In repl_scanner.l, you omitted double-doublequote handling for
   replication but it should be implemented. Zero-length
   identifier check might be needed depending on the upper-layer.

 - In walsender.c, the log messages "Initiating logical rep.."
   and "Starting logical replication.." should be INFO or LOG in
   loglevel, not WARNING. And 'rep' in the former message would
   be better not abbreviated since not done so in the latter.

 - In walsender.c, StartLogicalReplication seems trying to abort
   itself for timeline change. But timeline changes in 9.3+ don't
   need such an aid. You'd better consult StartReplication in
   current master for detail. There might be other defferences.

 - In walsender.c, the typedef name WalSndSendData doesn't seem
   to be a function pointer. I suppose passing bare function
   pointer to WanSndLoop and WalSndDone is not a good deed. It'd
   be better to wrap it in any struct for callback, say,
   LogicalDecodingContext. It'd be even better if it could be a
   common struct with 'physycal' replication.

 - In walsender.c, I wonder if the differences are necessary
   between logical and physical replication in fetching latest
   WALs, construction of WAL sending loop and so on .. Logical
   walsender seems to be implimentated in somewhat ad-hoc way on
   the whole. I belive it could be more commonize in the base

 - In procarray.c, the added two includes which is not
   accompanied by any other modification are needless. make emits
   no error or warning without them.

...Time's up. It'll be continued for later from 0010..


Kyotaro Horiguchi
NTT Open Source Software Center

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to