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 structure. - 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.. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
