RE: START_REPLICATION silently truncates an overlong LSN component

Hi Jan,

> Patch 0001
> ==========
>
> I have just a small comment on the moved MAXPG_LSNLEN. It's currently a
> hard-coded 17. It could make sense to export MAXPG_LSNCOMPONENT as well
> and define MAXPG_LSNLEN as (2 * MAXPG_LSNCOMPONENT + 1), which
> would
> document where the value comes from.

I feel the notation cannot be used. CppAsString2() converts the argument to the
string, but it does not evaluate arithmetic at that time. So your suggestion
would be "%(2 * 8 + 1)s" and it won't work well. You can find that the compiler
raises warnings.

I feel we can add a static assert to ensure they remain consistent. I put in
pg_parse_lsn.c and it will be checked at compilation time. See new 0002 patch.
I separated them because it may be the independent improvement.

> Patch 0002
> ==========
>
> Kuroda-san wrote in the v5 mail:
>
> > I also attached the test code to check invalid strings can be
> rejected, see 0002.
> > I'm not intended to be pushed.
>
> I think this test is worth committing. As far as I can see, nothing else
> covers the "this is an incremental backup, not a data directory" error,
> and the new LSN checks would otherwise remain untested. However, we test
> only an overlong high component. It could make sense to add this test
> for the low component as well (e.g., using 0/123456789).

Hmm. But 5ffb8c74 also did not have any tests and the error "syntax error in history file:"
is nor verified, right? I still feel we can follow the manner.

Best regards,
Hayato Kuroda
FUJITSU LIMITED