Re: pg_xlogdump --stats

Hi,

On 2014-09-19 14:38:29 +0530, Abhijit Menon-Sen wrote:
> > b) I'm not against it, but I wonder if the best way to add the
> >    SizeOfXLogRecord to the record size. It's just as much part of the
> >    FPI. And this means that the record length will be > 0 even if all
> >    the record data has been removed due to the FPI.
> 
> I'm not sure I understand what you are proposing here.

I'm not really proposing anything. I'm just stating that it's
notationally not clear and might be improved. Doesn't need to be now.

> > What was the reason you moved away from --stats=record/rmgr? I think
> > we possibly will add further ones, so that seems more extensible?
> 
> It was because I wanted --stats to default to "=rmgr", so I tried to
> make the argument optional, but getopt in Windows didn't like that.
> Here's an excerpt from the earlier discussion:
> 
>     > 3. Some compilation error in windows
>     > .\contrib\pg_xlogdump\pg_xlogdump.c(1002) : error C2065: 'optional_argument' : undeclared identifier
>     > .\contrib\pg_xlogdump\pg_xlogdump.c(1002) : error C2099: initializer is not a constant
>     >
>     > optional_argument should be added to getopt_long.h file for windows.
> 
>     Hmm. I have no idea what to do about this. I did notice when I wrote the
>     code that nothing else used optional_argument, but I didn't realise that
>     it wouldn't work on Windows.
> 
>     It may be that the best thing to do would be to avoid using
>     optional_argument altogether, and have separate --stats and
>     --stats-per-record options. Thoughts?

Oh, I've since implemented optional arguments for windows/replacement
getopt_long. It's used by psql since a week or so ago.

> I have no objection to doing it differently if someone tells me how to
> make Windows happy (preferably without making me unhappy).

[x] Done

> > Given that you've removed the UNKNOWNs from the rm_descs, this really
> > should add it here.
> 
> You would prefer to see HEAP/UNKNOWN rather than HEAP/32 for an
> unidentifiable xl_info?

UNKNOWN (32)? It should visually stand out more than just the
number. Somebody borked something if that happens.

Greetings,

Andres Freund