[PATCH] Clean up property graph error messages

New Activity: Ashutosh Bapat Weighs In — Argues Against the ReleaseSysCache Fix

The only new message is from Ashutosh Bapat, who provides a surprising counter-argument to the one remaining technically defensible change in the patch: the ReleaseSysCache-before-ereport(ERROR) reorder.

His argument has three prongs:

1. Code size trade-off: Moving ReleaseSysCache() before ereport() requires extracting values from the syscache tuple into local variables before releasing the pin, since those values are used in the ereport() format string. This increases lines of code and adds variable declarations. The current (dead) code is more compact because it accesses tuple attributes directly inside the ereport() call, relying on the pin still being held.

2. The dead code serves as documentation: Bapat argues the unreachable ReleaseSysCache() after ereport(ERROR) "helps justify fetching the attributes in the ereport call" — i.e., it signals to the reader that the tuple was pinned and would need releasing, making the code's intent clearer even though the call never executes.

3. Future-proofing: He raises the hypothetical that someone might someday make ereport conditional (e.g., a warning level instead of ERROR), in which case the trailing ReleaseSysCache() would become reachable and necessary. This is a speculative argument — ereport(ERROR) performing a longjmp is a foundational PostgreSQL invariant — but it reflects a defensive-coding philosophy.

Assessment

This is a minority position within PostgreSQL conventions. The established idiom throughout the backend is to release syscache pins before raising errors, precisely to avoid misleading readers into thinking control returns past ereport(ERROR). Bapat's "documentation value of dead code" argument is creative but cuts against the project's general aversion to dead code (which typically triggers compiler warnings or static-analysis findings). His future-proofing argument is extremely unlikely to materialize — changing ereport(ERROR) to be non-fatal would break thousands of call sites across the backend.

Impact on the Patch

Combined with Peter Eisentraut's rejection of the wording changes, this new pushback on the syscache reorder leaves the patch with no remaining changes that have reviewer/committer support. Unless Ayush can rally a different committer or produce a more comprehensive revision, the patch appears headed for abandonment. The thread is effectively a case study in how even small, seemingly obvious cleanup patches can be rejected when each individual change faces distinct objections.