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 <[email protected]> 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 <[email protected]> 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
