Technical Analysis: Ternary Reloption Type Patch Review

Core Problem

PostgreSQL's relation options (reloptions) system historically supported boolean, integer, real, enum, and string types. Several options—notably vacuum_index_cleanup, vacuum_truncate, and GiST's buffering—have three-valued semantics: true, false, and an "unset/auto" state where the system decides behavior. Previously, these were implemented as enum reloptions with explicit value lists (e.g., on/off/auto), which was awkward because:

1. Redundant with boolean parsing: PostgreSQL already has parse_bool() which accepts many synonyms (true/false, yes/no, on/off, 1/0), but enum-based ternary options only accepted a subset of these spellings.
2. Boilerplate: Each ternary option required defining an enum type and its string mappings, when the real semantic is simply "bool + unset."
3. Inconsistency: Users expect boolean-like options to accept all boolean synonyms uniformly.

A prior patch (v2) introduced a dedicated RELOPT_TYPE_TERNARY type with a pg_ternary enum (PG_TERNARY_TRUE, PG_TERNARY_FALSE, PG_TERNARY_UNSET) and a configurable unset_alias string (e.g., "auto") that maps to the unset state. This review addresses remaining issues in that v2 patch.

Proposed Changes (Review Patch)

This review identifies six distinct fixes/improvements to the v2 ternary reloption implementation:

1. Type Correctness: default_val field typing

The relopt_ternary struct's default_val was declared as int despite holding pg_ternary values. This is a type-safety issue—while ABI-compatible on most platforms, it obscures intent and could cause issues with strict compilers or static analysis tools. The fix changes the field to pg_ternary.

2. fillRelOptions() default value handling

This is the most architecturally significant fix. When a reloption is not explicitly set in pg_class.reloptions, parseRelOptions() marks it isset=false. fillRelOptions() then fills the options struct with a default. For bool, int, and enum types, this default comes from the registered default_val. However, the ternary path was hardcoded to PG_TERNARY_UNSET:

// Before (broken):
*(pg_ternary *) itempos = options[i].isset ?
    options[i].ternary_val : PG_TERNARY_UNSET;

// After (correct):
*(pg_ternary *) itempos = options[i].isset ?
    options[i].ternary_val :
    ((relopt_ternary *) options[i].gen)->default_val;

This was not a visible bug for built-in options (all default to PG_TERNARY_UNSET), but it broke the contract for extension AMs using add_ternary_reloption() with a non-UNSET default. The dummy_index_am test module's option_ternary_2 would have been affected.

3. Error message completeness

When ternary parsing fails, the error previously listed only "on", "off", and the unset alias. Since parsing goes through parse_bool() first, all boolean synonyms are valid. The updated DETAIL message enumerates the full set: "on", "off", "true", "false", "yes", "no", "1", "0", and "<unset_alias>". This restores the level of user guidance that the old enum-based implementation provided.

4. GiST relopt_parse_elt table correction

After converting buffering from enum to ternary, the GiST relopt_parse_elt table in gistutil.c still declared it as RELOPT_TYPE_ENUM. This was not a runtime bug because fillRelOptions() uses the type from the global registry (options[i].gen->type), not from the local parse element table. However, it's misleading for code readers and could become a real bug if the fill logic ever changes to use the local table's type field.

5. Formatting cleanup

Multiple declarations used const char* (C++ style) instead of PostgreSQL's standard const char *. Prototype alignment was also inconsistent with pgindent conventions. These are cosmetic but important for code consistency and automated formatting tools.

6. Test coverage

New regression tests verify:

• vacuum_index_cleanup=true and vacuum_index_cleanup=false are accepted and stored as-is in pg_class.reloptions
• GiST buffering=true is accepted as a synonym for on
• Error messages for invalid values show the complete set of valid inputs

Key Design Decisions and Tradeoffs

Value normalization (or lack thereof)

The tests show that vacuum_index_cleanup=true is stored as literal "true" in pg_class.reloptions, not normalized to "on". This is consistent with how PostgreSQL stores other reloptions (as raw text), but means that semantically equivalent settings may have different textual representations. This is a deliberate tradeoff favoring simplicity over canonical forms.

Unset alias flexibility

The unset_alias field can be NULL, meaning some ternary options may not expose the unset state to users at all (they'd only get true/false, with UNSET being the internal default). This provides flexibility for options that want tri-state internal behavior but binary user-facing semantics.

Backward compatibility

All existing options (vacuum_index_cleanup, vacuum_truncate, buffering) defaulting to PG_TERNARY_UNSET means the fillRelOptions() fix is backward-compatible. The expanded set of accepted input values is also backward-compatible (previously valid inputs remain valid).

Architectural Implications

The ternary reloption type is particularly useful for the extension ecosystem. Access methods frequently need "user preference or system decides" semantics. Without a first-class ternary type, each AM had to either abuse enums or use integers with magic values. The add_ternary_reloption() / add_local_ternary_reloption() API makes this pattern easy and consistent for extension authors.

The default_val fix is especially important for extensions: an AM might want buffering-like behavior where the default is "auto/unset" (system decides), but another AM might want a ternary option that defaults to TRUE unless explicitly overridden.