Thread

Commits

  1. Standardize one aspect of rmgr desc output.

  1. pg_waldump/heapdesc.c and struct field names

    Peter Geoghegan <pg@bowt.ie> — 2021-01-04T03:54:38Z

    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
    
    
    
    
  2. Re: pg_waldump/heapdesc.c and struct field names

    Masahiko Sawada <sawada.mshk@gmail.com> — 2021-01-04T04:57:36Z

    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/
    
    
    
    
  3. Re: pg_waldump/heapdesc.c and struct field names

    Abhijit Menon-Sen <ams@toroid.org> — 2021-01-04T05:21:36Z

    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
    
    
    
    
  4. Re: pg_waldump/heapdesc.c and struct field names

    Andres Freund <andres@anarazel.de> — 2021-01-04T21:12:03Z

    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
    
    
    
    
  5. Re: pg_waldump/heapdesc.c and struct field names

    Peter Geoghegan <pg@bowt.ie> — 2021-01-04T21:56:23Z

    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
    
    
    
    
  6. Re: pg_waldump/heapdesc.c and struct field names

    Peter Geoghegan <pg@bowt.ie> — 2021-01-04T22:06:01Z

    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
    
    
    
    
  7. Re: pg_waldump/heapdesc.c and struct field names

    Peter Geoghegan <pg@bowt.ie> — 2021-01-05T03:46:41Z

    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