Re: pg_waldump/heapdesc.c and struct field names
On Mon, Jan 4, 2021 at 2:06 PM Peter Geoghegan wrote: > Right. Self-consistency matters, as does consistency with the source > code itself. Pushed a commit that standardizes on the name latestRemovedXid within rmgr desc output routines just now. Thanks -- Peter Geoghegan
Re: pg_waldump/heapdesc.c and struct field names
On Sun, Jan 3, 2021 at 8:58 PM Masahiko Sawada wrote: > On Mon, Jan 4, 2021 at 12:55 PM Peter Geoghegan wrote: > +1 for changing heapdesc.c on master. It's not only readable but also > consistent with other *desc showing the field named latestRemovedXid. > For instance, nbtdesc.c has: > > case XLOG_BTREE_REUSE_PAGE: > { > xl_btree_reuse_page *xlrec = (xl_btree_reuse_page *) rec; > > appendStringInfo(buf, "rel %u/%u/%u; latestRemovedXid %u", > xlrec->node.spcNode, xlrec->node.dbNode, > xlrec->node.relNode, > xlrec->latestRemovedXid); > break; > } Right. Self-consistency matters, as does consistency with the source code itself. -- Peter Geoghegan
Re: pg_waldump/heapdesc.c and struct field names
On Mon, Jan 4, 2021 at 1:12 PM Andres Freund wrote: > I personally mildly prefer remxid - anything that is understandable but > shortens the line length is good for my uses of waldump. I want to use latestRemovedXid here because it is quite recognizable to me as a symbol name. It appears as a symbol name 84 times, across many files in several distinct areas. Whereas remxid means nothing to me unless I go look it up, which is not ideal. -- Peter Geoghegan
Re: pg_waldump/heapdesc.c and struct field names
Hi, On 2021-01-03 19:54:38 -0800, Peter Geoghegan wrote: > I notice that heapdesc.c outputs the field latestRemovedXid as > "remxid". But why? What sense is there in changing the name for output > by tools like pg_waldump, which are intrinsically internals focussed? I personally mildly prefer remxid - anything that is understandable but shortens the line length is good for my uses of waldump. Greetings, Andres Freund
Re: pg_waldump/heapdesc.c and struct field names
At 2021-01-03 19:54:38 -0800, p...@bowt.ie wrote: > > It just seems worth removing gratuitous inconsistencies, > such as this one. Agreed. -- Abhijit
Re: pg_waldump/heapdesc.c and struct field names
On Mon, Jan 4, 2021 at 12:55 PM Peter Geoghegan wrote: > > I notice that heapdesc.c outputs the field latestRemovedXid as > "remxid". But why? What sense is there in changing the name for output > by tools like pg_waldump, which are intrinsically internals focussed? Not sure but it has been "remxid" from the beginning. See efc16ea5206. > Does anyone have any objections to my changing the details within > heapdesc.c on master, so that pg_waldump will use struct field names? > It doesn't seem necessary to change the output that has spaces instead > of an underscore, or something like that. It just seems worth removing > gratuitous inconsistencies, such as this one. +1 for changing heapdesc.c on master. It's not only readable but also consistent with other *desc showing the field named latestRemovedXid. For instance, nbtdesc.c has: case XLOG_BTREE_REUSE_PAGE: { xl_btree_reuse_page *xlrec = (xl_btree_reuse_page *) rec; appendStringInfo(buf, "rel %u/%u/%u; latestRemovedXid %u", xlrec->node.spcNode, xlrec->node.dbNode, xlrec->node.relNode, xlrec->latestRemovedXid); break; } Regards, -- Masahiko Sawada EnterpriseDB: https://www.enterprisedb.com/
pg_waldump/heapdesc.c and struct field names
I notice that heapdesc.c outputs the field latestRemovedXid as "remxid". But why? What sense is there in changing the name for output by tools like pg_waldump, which are intrinsically internals focussed? Does anyone have any objections to my changing the details within heapdesc.c on master, so that pg_waldump will use struct field names? It doesn't seem necessary to change the output that has spaces instead of an underscore, or something like that. It just seems worth removing gratuitous inconsistencies, such as this one. -- Peter Geoghegan