Re: wal_dump output on CREATE DATABASE

On Tue, 2018-11-13 at 18:53 +0100, Jean-Christophe Arnu wrote:
> Le lun. 5 nov. 2018 à 15:37, Jean-Christophe Arnu <jcarnu(at)gmail(dot)com>
> a
> écrit :
>
> >
> >
> > Le dim. 4 nov. 2018 à 18:01, Jean-Christophe Arnu <jcarnu(at)gmail(dot)com
> > > a
> > écrit :
> >
> > > Le ven. 2 nov. 2018 à 08:37, Peter Eisentraut <
> > > peter(dot)eisentraut(at)2ndquadrant(dot)com> a écrit :
> > >
> > > > On 26/10/2018 15:53, Jean-Christophe Arnu wrote:
> > > > > Exemple on CREATE DATABASE (without defining a template
> > > > > database) :
> > > > > rmgr: Database len (rec/tot): 42/ 42,
> > > > > tx: 568, lsn:
> > > > > 0/01865790, prev 0/01865720, desc: CREATE copy dir 1/1663 to
> > > > > 16384/1663
> > > > >
> > > > > It comes out (to me) it may be more consistent to use the
> > > > > same template
> > > > > than the other operations in pg_waldump.
> > > > > I propose to swap the parameters positions for the copy dir
> > > > > operation
> > > > > output.
> > > > >
> > > > > You'll find a patch file included which does the switching
> > > > > job :
> > > > > rmgr: Database len (rec/tot): 42/ 42,
> > > > > tx: 568, lsn:
> > > > > 0/01865790, prev 0/01865720, desc: CREATE copy dir 1663/1 to
> > > > > 1663/16384
> > > >
> > > > I see your point. I suspect this was mainly implemented that
> > > > way
> > > > because that's how the fields occur in the underlying
> > > > xl_dbase_create_rec structure. (But that structure also has
> > > > the target
> > > > location first, so it's not entirely consistent that way
> > > > either.) We
> > > > could switch the fields around in the output, but I wonder
> > > > whether we
> > > > couldn't make the whole thing a bit more readable like this:
> > > >
> > > > desc: CREATE copy dir ts=1663 db=1 to ts=1663 db=16384
> > > >
> > > > or maybe like this
> > > >
> > > > desc: CREATE copy dir (ts/db) 1663/1 to 1663/16384
> > > >
> > >

I don't know, but this feels like a step too far to me.

The patch started with the goal of making the CREATE DATABASE format
more consistent w.r.t. to other pg_waldump output, and this seems more
like redesigning the whole format with no clear use case.

People reading pg_waldump output quickly learn to read the A/B/C format
and what those fields mean. Breaking that into ts=A db=B relfilenode=C
does not make that particularly clearer or easier to read. I'd say it'd
also makes it harder to parse, and it increases the size of the output
(both in terms of line length and data size).

So -1 to this from me. I'd just swap the fields in the copy dir message
and call it a day.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services