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
LOGin 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-hocway on the whole. I belive it could be more commonize in the base structure.
- In procarray.c, the added two includes which is not accompanied by any other modification are needless. make emits
noerror or warning without them.
...Time's up. It'll be continued for later from 0010..
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
