Re: logical changeset generation v6.7

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