Обсуждение: 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
On Mon, Jan 4, 2021 at 12:55 PM Peter Geoghegan <pg@bowt.ie> 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/
At 2021-01-03 19:54:38 -0800, pg@bowt.ie wrote: > > It just seems worth removing gratuitous inconsistencies, > such as this one. Agreed. -- Abhijit
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
On Mon, Jan 4, 2021 at 1:12 PM Andres Freund <andres@anarazel.de> 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
On Sun, Jan 3, 2021 at 8:58 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> On Mon, Jan 4, 2021 at 12:55 PM Peter Geoghegan <pg@bowt.ie> 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
On Mon, Jan 4, 2021 at 2:06 PM Peter Geoghegan <pg@bowt.ie> 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