Le jeu. 15 nov. 2018 à 19:44, Robert Haas <robertmh...@gmail.com> a écrit :
> On Tue, Nov 13, 2018 at 3:40 PM Tomas Vondra > <tomas.von...@2ndquadrant.com> wrote: > > 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). > > I agree. > First, thank you all for your reviews. I also agree that the A/B/C format is right (and it may be a good thing to document it, maybe by adding some changes in the doc/src/sgml/ref/pg_waldump.sgml file to this patch). To reply to Andres, I agree we should not change things for a target format that would not fit clearly defined syntax. In that way, I agree with Tomas on the fact that people reading pg_waldump output are quickly familiar with the A/B/C notation. My first use case was to decode the ids with a processing script to identify each id in A/B/C or pg_waldump output with a "human readable" item. For this, my processing script connects the cluster and tries resolve the ids with simple queries (and building a local cache for this). Then it replaces each looked up id item with its corresponding text. In some cases, this could be useful for DBA to find more easily when a specific relation was modified (searching for DELETE BTW). But that's only my use case and my little script. Going back to the code : As I can figure by crawling the source tree (and discovering it) there are messages with : * A/B/C notation which seems to be the one we should adopt ( meaning ts/db/refilenode ) some are only * A/B for the COPY message we discussed later On the other hand, and I don't know if it's relevant, I've pointed some examples such as XLOG_RELMAP_UPDATE in relmapdesc.c which could benefit of that "notation" : appendStringInfo(buf, "database %u tablespace %u size %u", xlrec->dbid, xlrec->tsid, xlrec->nbytes); could be written like this : appendStringInfo(buf, "%u/%u size %u", xlrec->tsid, xlrec->dbid, xlrec->nbytes); In that case ts and db should also be switched. In that case the message would only by B/C which is confusing, but we have other place where "base/" is put in prefix. The same transform may be also applied to standbydesc.c in standby_desc() function. appendStringInfo(buf, "xid %u db %u rel %u ", xlrec->locks[i].xid, xlrec->locks[i].dbOid, xlrec->locks[i].relOid); may be changed to appendStringInfo(buf, "xid %u (db/rel) %u/%u ", xlrec->locks[i].xid, xlrec->locks[i].dbOid, xlrec->locks[i].relOid); As I said, I don't know whether it's relevant to perform these changes or not. If the A/B/C notation is to be generalized, it would be worth document it in the SGML file. If not, the first patch provided should be enough. Regards -- Jean-Christophe Arnu