Re: pg_waldump/heapdesc.c and struct field names

2021-01-04 Thread Peter Geoghegan
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

2021-01-04 Thread Peter Geoghegan
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

2021-01-04 Thread Peter Geoghegan
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

2021-01-04 Thread Andres Freund
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

2021-01-03 Thread Abhijit Menon-Sen
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

2021-01-03 Thread Masahiko Sawada
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

2021-01-03 Thread Peter Geoghegan
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