Subject: Re[2]: [PATCH] Add pg_current_vxact_id() function to expose virtual transaction IDs

From: "Pavlo Golub" <[email-masked]>
Date: Tue, 06 Jan 2026 12:47:08 +0000

Hello.

Attached is v2 of the pg_current_vxact_id() patch.
Changes in v2:
- Rebased on current master
- Changed OID from 5101 to 9538 (following unused_oids best practice
   recommendation to use the 8000-9999 range for patch development)

Best regards,
Pavlo Golub

---

From: Henson Choi <[email-masked]>
Date: Tue, 6 Jan 2026 22:59:18 +0900

Hi Pavlo,

Thank you for the v2 patch. I've reviewed it and here are my comments:

== Summary ==

The patch applies cleanly and all regression tests pass.
The implementation is straightforward and follows existing patterns.

== Detailed Review ==

1. Functionality: OK
   - The function correctly returns the VXID in the expected format.

2. Tests: OK
   - Regression tests are included and pass.
   - gcov/valgrind testing is unnecessary due to the simplicity of the code.

3. Code Safety: OK
   - Buffer size (32 bytes) is sufficient for maximum output (23 bytes),
     consistent with VXIDGetDatum() in lockfuncs.c.
   - Memory allocated by cstring_to_text() via palloc is in
     ecxt_per_tuple_memory and automatically managed.

4. Typos: None found.

== Suggestions for Improvement ==

1. Format String Duplication

   The format string "%d/%u" is now duplicated in three places:
   - src/backend/utils/adt/lockfuncs.c (VXIDGetDatum)
   - src/backend/utils/error/elog.c (%v placeholder)
   - src/backend/utils/adt/xid8funcs.c (pg_current_vxact_id)

   Consider defining a macro in lock.h for consistency:

     #define VXID_FMT "%d/%u"

   All three files already include lock.h indirectly:
   - lockfuncs.c -> predicate_internals.h -> lock.h
   - elog.c -> proc.h -> lock.h
   - xid8funcs.c -> proc.h -> lock.h

2. Documentation Terminology

   The terms "localTransactionId" and "localXID" are used inconsistently:
   - localTransactionId: 30+ in C code (actual field name), 1 in sgml
(monitoring.sgml)
   - localXID: 3 in sgml only (xact.sgml, config.sgml)

   The new func-info.sgml uses "localTransactionId" which matches the
   actual C struct field name. However, existing documentation prefers
   "localXID" for user-facing text. Consider using "localXID" in
   func-info.sgml for consistency with xact.sgml and config.sgml.

== Comparison with pg_current_xact_id ==

The implementation follows a similar pattern to pg_current_xact_id(),
which was introduced in commit 4c04be9b05a. The placement in
xid8funcs.c is appropriate.

--

>    recommendation to use the 8000-9999 range for patch development)

Best regards,
Henson

---

From: Henson Choi <[email-masked]>
Date: Fri, 9 Jan 2026 09:07:05 +0900

Hi Pavlo,

I've moved this patch to "Waiting on author" status in the commitfest.

I'm interested in your thoughts on the two suggestions from my review:

1. VXID_FMT macro to eliminate format string duplication
2. Using "localXID" terminology in documentation for consistency

Would you like to incorporate these in v3, or do you have concerns
about either suggestion?

Looking forward to your feedback.

Best regards,
Henson Choi