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

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.

--

2026년 1월 6일 (화) PM 9:47, Pavlo Golub <pavlo(dot)golub(at)cybertec(dot)at>님이 작성:

> 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

Best regards,
Henson