Thread

[PATCH] Allow complex data for GUC extra.

Bryan Green <dbryan.green@gmail.com> — 2025-11-17T21:17:19Z

Following up on Tom Lane's suggestion to use MemoryContexts for GUC
extra data [1], I've implemented a working solution that addresses the
design issues Robert Haas identified with my initial approach.

The original problem: GUC check hooks can only return a single chunk for
extra data, making it awkward to use complex structures like Lists or
hash tables. Tom suggested allowing the extra field to point to a
MemoryContext instead, which would enable arbitrary nested structures
with automatic cleanup via MemoryContextDelete().

My first implementation stored the context pointer directly as the extra
data. Robert pointed out the fatal flaw: during transaction rollback,
the assign hook needs to receive a data pointer (to update its global
variable), but if extra contains a context pointer, there's no way to
retrieve the actual data. A global mapping table would work but seemed
unnecessarily complex.

The solution uses a wrapper struct (GucContextExtra) containing both the
MemoryContext and data pointers. Check hooks:
  1. Create a context under CurrentMemoryContext (for error safety)
  2. Allocate their data structures within it
  3. Allocate the wrapper itself within the same context
  4. On success, re-parent the context to TopMemoryContext
  5. Return the wrapper as extra

The GUC machinery manages wrapper pointers in its stack. On rollback,
the assign hook receives the old wrapper and extracts the correct old
data pointer, maintaining proper transaction semantics. When freeing
extra data, we simply delete the context - since the wrapper lives
inside it, everything is freed with one call.

Error handling is automatic: if the check hook errors during parsing,
the context is still under CurrentMemoryContext and gets cleaned up
normally, preventing leaks.

The attached patch adds:
  - GUC_EXTRA_IS_CONTEXT flag
  - GucContextExtra struct definition
  - Modified free_extra_value() to handle both paths
  - Test module (src/test/modules/test_guc) with simple counter
(traditional path, no context) and server pool (context-based path with
List)

Regression tests validate that Lists survive transaction rollback and
savepoint operations correctly.

Thoughts?

Patch attached.

[1] https://discord.com/channels/1258108670710124574/1402360503036285130

-- 
Bryan Green
EDB: https://www.enterprisedb.com