Thread

  1. Re: [PATCH] Add pg_current_vxact_id() function to expose virtual transaction IDs

    Henson Choi <assam258@gmail.com> — 2026-05-14T00:08:04Z

    Hi Pavlo, Robert,
    
    Responding to both of you in one mail since the points are related.
    
    == To Pavlo, on v3 ==
    
    On Thu, Feb 5, 2026 at 10:13 AM Pavlo Golub <pavlo.golub@cybertec.at> wrote:
    > I've prepared v3 of the patch addrressing Henson's code review:
    > - Added #define VXID_FMT "%d/%u" to lock.h
    > - Updated lockfuncs.c, elog.c, and xid8funcs.c to use it
    > - Use "localXID" (not "localTransactionId") in user docs
    
    Thanks.  All three items from my v2 review are addressed in v3:
    
      1. VXID_FMT macro                 -- OK
      2. VXID_FMT applied to 3 files    -- OK
      3. "localXID" in func-info.sgml   -- OK
    
    The patch applies cleanly on master and "make check-world" passes on
    my machine (macOS/arm64).
    
    > My main concern though is about semantic clarity. I see a huge problem
    > that one needs to query pg_locks to get VXID. Why would I want to
    > query the lock subsystem to get transaction ID? That's very confusing.
    
    Agreed.  The asymmetry with pg_backend_pid() / pg_current_xact_id() is
    the part I find most persuasive too -- regardless of performance,
    having to enter the lock subsystem to ask "what is my transaction's
    identity?" is an odd shape for the API.
    
    == To Robert ==
    
    On Sat, Feb 14, 2026 at 2:16 AM Robert Haas <robertmhaas@gmail.com> wrote:
    > I agree with this and would be inclined to accept the patch. I have
    > reviewed the v3 patch and I didn't see anything wrong with it.
    
    Thanks for taking a look.  I'll leave the empirical side of the
    performance argument to Pavlo if he wants to follow up on it; my own
    endorsement rests mainly on the API-shape point above.
    
    == One nit on v3 ==
    
    Running src/tools/pgindent/pgindent over the touched C/H files
    produces one small rewrap in xid8funcs.c (no semantic change, just a
    comment reflow):
    
        - * Check if we have a valid vxid.  The vxid format matches what's used
        - * in elog.c for the %v placeholder and in pg_locks.virtualtransaction.
        + * Check if we have a valid vxid.  The vxid format matches what's used
    in
        + * elog.c for the %v placeholder and in pg_locks.virtualtransaction.
    
    Pavlo, could you fold that into a v4?  Other than that I have nothing
    more to add, and with v4 in place I would mark the patch Ready for
    Committer.
    
    Best regards,
    Henson Choi