System-variable schema metadata model (varDef): declarative defs feeding registry, flags, guards, RESET ALL, SET LOCAL, and docs

[apstndb]

Design: System-Variable Schema Metadata Model (varDef)

Status: design (no code changed yet). Citations are against main at b5057e5; paths are repo-relative under internal/mycli/ unless noted.

0. Current state, verified

The registry today is a runtime map built imperatively:

• VarRegistry.registerAll() (var_registry.go:162-516) makes ~87 r.Register(name, handler) calls. Metadata per variable is smeared across the handler: description and read-only flag live inside each handler (VarHandler[T].description/readOnly, var_handler.go:40-48), guards are hand-rolled CustomVar setters (READONLY's in-transaction check at var_registry.go:166-180; CLI_ENABLE_ADC_PLUS's session-init-only check at var_registry.go:486-499, with the acknowledged TODO at var_handler.go:33-39).
• Variable is {Get, Set, Description, IsReadOnly} (var_handler.go:12-17); 10 handler types each re-implement Description()/IsReadOnly() boilerplate (var_custom_handlers.go, var_enum_handlers.go).
• COMMIT_RESPONSE and CLI_DIRECT_READ are special-cased at 6 sites: SHOW VARIABLES (statements_system_variable.go:51-66), SET LOCAL (statements_system_variable.go:116-118), HELP VARIABLES (statements_system_variable.go:202-215), setFromGoogleSQL/setFromSimple (system_variables_registry.go:30-38, 65-73), and get (system_variables_registry.go:137-165), plus the tombstone comment at var_registry.go:515.
• Flag wiring is a dual write path: createSystemVariablesFromOptions (config.go:342-456) direct-assigns ~25 fields, bypassing setters. Concrete divergences: --prompt2="" is accepted (config.go:369-371) though the registry setter rejects empty (var_registry.go:244-253); --log-level is parsed by SetLogLevel (config.go:348) duplicating LogLevelVar.Set (var_custom_handlers.go:257-273); STATEMENT_TIMEOUT's default lives only in a kong tag (default:"10m", config.go:159) while RPC_PRIORITY's default lives in both newSystemVariablesWithDefaults (system_variables.go:341) and the mapping row cmp.Or(opts.Priority, "MEDIUM") (config.go:591).
• The output-template loader exists twice with divergent semantics: the registry setter maps "" -> OutputTemplate = nil (var_registry.go:436-441 via var_custom_handlers.go:184-196), while startup maps "" -> defaultOutputFormat (config.go:479-488 via system_variables.go:472-491). So SET CLI_OUTPUT_TEMPLATE_FILE = '' and the flag default produce different states — this breaks any Get/Set round-trip that RESET ALL relies on and must be fixed first.
• SET LOCAL's undo log lives on TransactionManager.localVarUndo (transaction_manager.go:131-144), pushed after a pre-flight Registry.Set(name, oldValue) round-trip (statements_system_variable.go:129-135), replayed in reverse through Registry.Set by restoreLocalVarsIfIdle (transaction_manager.go:283-309).
• Docs generation (Generate system variables reference in docs/system_variables.md from the variable registry #710): helpVariableRows (statements_system_variable.go:175-222) feeds both HELP VARIABLES and renderSystemVariablesHelp (app.go:426-460, hidden --sysvars-help); make docs-update splices between markers; readme-sync.yml is the drift check. Columns today: Name, Operations, Description.
• The copy-and-repoint registry lifecycle is gone: exactly one live systemVariables per process (system_variables.go:192-203); USE/DETACH mutate in place. This design depends on that invariant.
• Issue-body nit: Rename CLI_DIRECT_READ to DIRECTED_READ and make read/write #486 says "CLI_DIRECTED_READ", but the actual variable is CLI_DIRECT_READ.

1. Go representation: a declarative varDef table, runtime (no codegen)

Decision: a package-level []varDef table that registerAll iterates, with per-variable bind closures constructing today's handlers. No code generation. The VarHandler[T]/EnumVar[T]/ProtoEnumVar[T] generics stay as the value substrate; everything about the variable moves out of handlers into the def. Codegen is rejected: 87 variables is nothing at runtime, the table is type-checked Go in the same package (bind closures can reach unexported fields like sv.typeStyles), and codegen would add a build step plus a generated-file drift surface for zero benefit.

type varScope int

const (
	// scopeSession: the SET-able surface. Participates in RESET ALL and SET LOCAL.
	scopeSession varScope = iota
	// scopeStartup: StartupConfig-backed. Read-only via SET; written only by
	// config.go/app.go before session creation.
	scopeStartup
	// scopeConnection: connection identity. Read-only via SET; mutated only by USE/DETACH.
	scopeConnection
	// scopeResult: last-statement outputs (LastResult).
	scopeResult
)

type varDef struct {
	name string
	desc string
	typ  string // docs only: "BOOL", "INT64", "STRING", "DURATION", "ENUM", "TIMESTAMP"

	scope    varScope
	readOnly bool // for scopeSession exceptions only; other scopes implicitly read-only
	initOnly bool // settable only before session creation
	txnGuard bool // SET rejected while a transaction is active
	noLocal  bool // opt-out of SET LOCAL for otherwise-eligible vars
	noReset  bool // opt-out of RESET ALL for otherwise-eligible vars

	aliases []string // deprecated names accepted by SET/SHOW VARIABLE; not listed/completed

	flag           string   // primary CLI flag; docs + drift test
	defaultDisplay string   // docs override when the live default is env-dependent
	examples       []string // for generated docs (#292)
	related        []string // related variable names; validated by a registry test

	bind     func(sv *systemVariables) Variable
	bindAdd  func(sv *systemVariables) func(string) error
	fromOpts func(opts *spannerOptions) (value string, ok bool)
}

func (d *varDef) settable() bool     { return d.scope == scopeSession && !d.readOnly }
func (d *varDef) localAllowed() bool { return d.settable() && !d.initOnly && !d.txnGuard && !d.noLocal }
func (d *varDef) resettable() bool   { return d.settable() && !d.initOnly && !d.noReset }

The registry stores the def next to the handler so policy is enforced in one place:

type registeredVar struct {
	def *varDef
	v   Variable
	add func(string) error
}

func (r *VarRegistry) Set(name, value string, isGoogleSQL bool) error {
	rv, ok := r.vars[strings.ToUpper(name)]
	if !ok {
		return &ErrUnknownVariable{Name: name}
	}
	def := rv.def
	switch {
	case !def.settable():
		return errSetterReadOnly
	case def.initOnly && r.sv.inTransaction != nil: // "session exists" proxy, as today
		return &errSetterInitOnly{Name: def.name}
	case def.txnGuard && r.sv.inTransaction != nil && r.sv.inTransaction():
		return errSetterInTransaction
	}
	if isGoogleSQL {
		value = parseGoogleSQLValue(value)
	}
	return rv.v.Set(value)
}

Consequences (all code-reducing):

• Variable shrinks to {Get() (string, error); Set(string) error}. Deleted: AsReadOnly, the description/readOnly fields and both interface methods on all 10 handler types (~70 lines).
• CustomVar survives only where get/set are genuinely custom (CLI_QUERY_MODE, CLI_TYPE_STYLES). Guard-only wrappers (READONLY, CLI_ENABLE_ADC_PLUS) are deleted, executing the TODO at var_handler.go:33-39.
• Valid values stay on handlers (ValidValuesEnumerator), NOT in the def — enum handlers already derive them from enumer/proto maps; duplicating them would create drift.
• New registry invariant tests: def names uppercase and unique including aliases; related entries resolve; every flag matches a kong tag; every localAllowed() var's Get->Set round-trips on defaults.

2. Flag wiring and config.go: one write path, one defaults source

Do not generate the kong struct. spannerOptions carries hidden aliases with precedence warnings, mutual exclusions, pointer-typed unset detection, and ~30 non-variable flags. Instead:

(a) Simple flags -> fromOpts on the def, executed generically in initializeSystemVariables:

for i := range varDefs {
	def := &varDefs[i]
	if def.fromOpts == nil {
		continue
	}
	if value, ok := def.fromOpts(opts); ok {
		if err := sysVars.SetFromSimple(def.name, value); err != nil {
			return nil, fmt.Errorf("invalid value of %s: %v: %w", def.flag, value, err)
		}
	}
}

This closes the --prompt2="" validation bypass, deletes the duplicate SetLogLevel parse, and replaces applyOptionMappings. Startup honors the same validation and side effects as SET.

(b) Startup-scope fields stay direct assignments — by design (StartupConfig's contract). CLI_ENDPOINT/HOST/PORT resolution, insecure/role precedence, --strong/--read-timestamp composition (via one SetFromSimple("READ_ONLY_STALENESS", ...)), format-flag precedence all stay explicit startup code.

(c) Defaults: single source = newSystemVariablesWithDefaults, verified by a Get/Set round-trip drift test; move STATEMENT_TIMEOUT's 10m there, drop the cmp.Or(opts.Priority, "MEDIUM") duplicate.

(d) Drift test instead of generation for kong: reflect over kong tags, assert every varDef.flag names a real flag.

3. Registry-native COMMIT_RESPONSE and DIRECTED_READ

CLI_DIRECT_READ stops being special the moment it becomes writable (#486). Register DIRECTED_READ as an ordinary def with aliases: ["CLI_DIRECT_READ"], a directedReadVar handler (Get reuses the formatting currently duplicated at 2 sites; Set calls parseDirectedReadOption; ""/NULL clears). Behavior change to flag: DIRECTED_READ then always appears in SHOW VARIABLES (empty when unset).

Alias mechanics (serves #487, available to #485): aliases are extra keys in r.vars pointing at the same registeredVar. Lookups resolve them; listings iterate varDefs, so aliases never appear in SHOW VARIABLES/HELP/docs/completion. Optionally one slog.Warn per use. Recommendation: alias for one release for pure renames (CLI_DIRECT_READ->DIRECTED_READ, CLI_PROTO_DESCRIPTOR_FILE->PROTO_DESCRIPTORS_FILE_PATH); no alias for CLI_ASYNC_DDL->DDL_EXECUTION_MODE (#485, type changes bool->enum).

COMMIT_RESPONSE is the one genuinely multi-valued variable. One optional capability interface:

// MultiValueVar is a variable whose SHOW VARIABLE result has multiple columns.
type MultiValueVar interface {
	GetMulti() (map[string]string, error)
}

Register COMMIT_RESPONSE with scope: scopeResult and a commitResponseVar{sv}. Then sv.get loses both special cases; setFrom*/addFrom* lose theirs (enabling the duplicated-pair collapse and deleting the dead errors.As conversions); ShowVariablesStatement merges GetMulti results generically; helpVariableRows drops its two hand-appended rows; SetLocalStatement drops its name check (metadata handles it).

4. RESET ALL (#484) and SET LOCAL integration

RESET semantics: restore to session-startup values (post-flag), not compiled-in defaults (PostgreSQL-compatible; --format=csv then RESET ALL stays CSV; sidesteps "default not reachable via Set").

Mechanism: after initializeSystemVariables, snapshot every resettable def's current display string into sv.startupValues; ResetAll() replays through Registry.Set with an equality skip (avoids re-running side-effectful setters when already at the startup value), reporting per-variable errors via errors.Join. RESET ALL is rejected while a transaction is active (same shape as the USE/DETACH guard), which makes the SET LOCAL undo-log interaction a non-issue by construction. RESET <name> falls out for one extra statement def. initOnly vars are excluded.

SET LOCAL (localAllowed). SetLocalStatement first checks def.localAllowed() with targeted errors; the pre-flight Set(old) round-trip is deleted, replaced by a table-driven round-trip unit test over all localAllowed defs. File-backed setters get noLocal: true (CLI_PROTO_DESCRIPTOR_FILE, CLI_OUTPUT_TEMPLATE_FILE). The undo log stays on TransactionManager.localVarUndo unchanged (mechanism sound and #293-proof).

New variables map cleanly: TRANSACTION_TIMEOUT (#482) = NullableDurationVar def; AUTO_BATCH_DML_UPDATE_COUNT(+_VERIFICATION) (#401) = plain defs; CLI_DDL_IN_TRANSACTION_MODE (#402) and CLI_INACTIVE_TRANSACTION_ACTION (#403) = initOnly enum defs; AUTOCOMMIT (#83) and RETRY_ABORTS_INTERNALLY (#293) replace their UnimplementedVar placeholders with txnGuard Bool defs when their runtime behavior lands.

5. Docs generation (#710/#292) on the richer metadata

renderSystemVariablesHelp iterates varDefs; HELP VARIABLES keeps its 3-column shape. Generated reference-table columns become: Name | Type | Default | Operations | Description.

• Default = Get(name) on a fresh defaults instance (live default, cannot drift), with defaultDisplay overrides for env-dependent values (CLI_HISTORY_FILE) and the column rendered only for settable defs (CLI_VERSION would break the drift check).
• Operations extends read,write,add with local (from localAllowed()), documenting SET LOCAL eligibility for free.
• Valid values stay folded into descriptions for the table; a later optional per-variable detail generator can use ValidValuesEnumerator + examples + related for Enhance system variables documentation with comprehensive user guide #292's detailed reference. Do NOT auto-replace the hand-written #### NAME sections; add a drift test that every manual heading is a registered def name.
• Every def change regenerates docs: each migration PR must include its make docs-update output. The Generate system variables reference in docs/system_variables.md from the variable registry #710+Add TSV output format with lossless escaping #711->docs: regenerate system variables reference after TSV registry change #716 merge-combination incident applies with more force as the table gets richer; required status checks with strict up-to-date branches (the ci: switch Dependabot to daily schedule with grouped updates and auto-merge #706 ruleset work) is prerequisite hygiene.

6. Migration plan (each PR independently green)

• PR0 — bug fix, independent (small): unify the output-template loaders; SET CLI_OUTPUT_TEMPLATE_FILE = '' restores defaultOutputFormat (today it nils the template). Delete setOutputTemplateFile/setDefaultOutputTemplate in favor of the registry setter. Precondition for RESET/round-trip semantics. ~-25 lines.
• PR1 — the mechanical core: introduce varDef{name, desc, typ, scope, readOnly, bind, bindAdd}; convert all ~87 Register calls into table entries 1:1; registry enforces read-only from the def; Variable shrinks to Get/Set; delete AsReadOnly and all Description()/IsReadOnly() boilerplate. Docs regen must be byte-identical — that IS the review check. Large diff, zero decisions.
• PR2 — guards: initOnly/txnGuard enforcement in Registry.Set; delete the READONLY and CLI_ENABLE_ADC_PLUS CustomVar wrappers; SetLocalStatement switches to localAllowed() and drops the pre-flight round-trip; add the round-trip unit test; mark noLocal on the two file-backed vars.
• PR3 — registry-native specials + aliases: MultiValueVar, alias resolution; register COMMIT_RESPONSE; delete the 6 special-case sites; collapse setFrom*/addFrom* pairs and their dead error conversions. PR3b implements Rename CLI_DIRECT_READ to DIRECTED_READ and make read/write #486 (DIRECTED_READ def + alias), the first consumer proving the rename machinery.
• PR4 — flag wiring: fromOpts on defs; generic loop replaces applyOptionMappings + the ~15 settable direct assignments; single defaults source; kong-tag<->def.flag drift test; default round-trip test.
• PR5 — RESET ALL (Implement RESET ALL with startup snapshots and atomic validation #484): snapshot + ResetAll + statement defs (RESET ALL, optionally RESET ), in-transaction guard, noReset classification.
• PR6 — docs enrichment (Generate system variables reference in docs/system_variables.md from the variable registry #710/Enhance system variables documentation with comprehensive user guide #292 partial): typ/defaultDisplay/examples/related population, new columns, manual-heading drift test, regen.
• Afterwards, on top of the model: Replace CLI_ASYNC_DDL with SYNC, ASYNC and ASYNC_WAIT execution modes #485, Rename CLI_PROTO_DESCRIPTOR_FILE to PROTO_DESCRIPTORS_FILE_PATH #487, Apply TRANSACTION_TIMEOUT to the logical transaction across replay #482, Verify expected update counts when automatic DML batches flush #401, Define DDL behavior inside logical transactions #402, feat: Support Inactive Transaction Action #403, and eventually Implement AUTOCOMMIT using the existing logical transaction lifecycle #83/Implement bounded ABORTED retries using the SAVEPOINT replay machinery #293. Each becomes "add a def + the runtime behavior". Update dev-docs/patterns/system-variables.md as part of "done" for PR1-PR4.

Rough LOC budget: table overhead ~+2 lines/var; deletions: guard CustomVars (-40), special-case sites (-90), setFrom*/addFrom* collapse (-80), interface boilerplate (-70), config direct assignments/mappings (-40), template-loader dup (-25). Net negative.

7. What NOT to do

• No codegen, no external schema file (YAML/JSON/go:generate).
• Do not generate spannerOptions/kong. Drift tests over generation.
• Do not put valid values, or parse/format logic, in the def.
• No overlay lookup for SET LOCAL — the undo log stays.
• No permanent alias layer; aliases are per-rename, one-release transition tools.
• No new package; no Variable interface growth beyond the one optional MultiValueVar; no mutexes; no speculative fields (no Since, no Category).
• Do not registry-fy Params, StreamManager, or the rest of LastResult.
• Watch the file-count failure indicator: if the table moves to var_defs.go, compensate by merging/deleting system_variables_registry.go content (largely disappears in PR3).
• Do not let any registry-touching PR merge without regenerated docs.

[apstndb]

Progress checklist against the migration plan (section 6):

Done:

• PR0: output-template loader unification (fix: unify output-template loaders so empty value restores default #727)
• PR1: declarative varDef table + registerAll iteration; Variable shrunk to Get/Set (var_defs.go, var_registry.go) (refactor: introduce declarative varDef table for system variables #731)
• PR2: central initOnly/txnGuard enforcement in Registry.Set; guard CustomVar wrappers removed; SET LOCAL pre-flight via the round-trip test (refactor: enforce initOnly/txnGuard guards centrally in the varDef registry #747)
• PR3: registry-native COMMIT_RESPONSE via MultiValueVar/GetMulti + the alias mechanism (refactor: make COMMIT_RESPONSE registry-native and add the alias mechanism #750)

Remaining:

• PR3b: Rename CLI_DIRECT_READ to DIRECTED_READ and make read/write #486 DIRECTED_READ def + CLI_DIRECT_READ alias; delete the remaining CLI_DIRECT_READ special-case sites (statements_system_variable.go, system_variables_registry.go)
• PR4: fromOpts on defs + generic startup loop replacing applyOptionMappings/direct assignments; single defaults source; kong-tag/flag drift test
• PR5: Implement RESET ALL with startup snapshots and atomic validation #484 RESET ALL (startup-value snapshot + ResetAll + statement defs + in-transaction guard); the noReset/resettable() scaffolding is already in place
• PR6: docs enrichment: populate typ/defaultDisplay/examples/related, add Type/Default columns to the generated table, manual-heading drift test (feeds Enhance system variables documentation with comprehensive user guide #292)

[apstndb]

Current-source reconciliation at 224d3053290c4d2d28ee1516e41f8571a243a239 (2026-09-15 JST): much of the migration is already on main.

• PR0's single output-template loader is present (fix: unify output-template loaders so empty value restores default #727).
• The declarative varDef registry and centralized guard policies are present (refactor: introduce declarative varDef table for system variables #731/refactor: enforce initOnly/txnGuard guards centrally in the varDef registry #747).
• COMMIT_RESPONSE is registry-native and aliases are present (refactor: make COMMIT_RESPONSE registry-native and add the alias mechanism #750); DIRECTED_READ is implemented (Match named query parameters case-insensitively #969).
• Startup snapshot RESET/RESET ALL is implemented (Implement RESET ALL with startup snapshot prepare/commit #976). Feature flags already use the feature-owned ApplyFlags seam; do not recreate it.

Two concrete parts of the old plan remain incomplete:

1. Core startup wiring (PR4). initializeSystemVariables still has its explicit optionMapping list and createSystemVariablesFromOptions still assigns some settable values directly. There is no varDef.flag/fromOpts metadata or corresponding flag-drift contract. Before implementation, select the simple registry-settable mappings and preserve intentional startup-only assignments, unset detection, precedence and special validation. The existing feature seam must not acquire core spannerOptions coupling.
2. User-facing metadata/docs (PR6 and Enhance system variables documentation with comprehensive user guide #292). varDef does not yet carry type/default/example/related metadata; the generated table is still Name/Operations/Description. Operations currently show read/write/add, with no explicit local/reset eligibility. Existing prose documents many variables, so Enhance system variables documentation with comprehensive user guide #292's original claim that the file only contains TODOs is obsolete. Enrich the remaining reference and usage gaps without replacing correct manual sections or duplicating live defaults/valid-value logic.

Keep these as separate implementation-sized changes. The old sketch is a starting point, not authorization for a broad registry rewrite. Preserve the current prepare/commit RESET semantics and ownership-specific guards added since this design was written.

[apstndb]

Current-source review of the remaining PR4 sketch: defer the mechanical startup-wiring migration. This is a benefit decision, not a claim that every optional metadata field in the old sketch was implemented.

The eight-row optionMapping table already routes through SetFromSimple. The current registry, guard/alias handling, feature-owned ApplyFlags, corrected prompt2 validation, and startup-snapshot RESET are present. Moving that table into varDef.fromOpts/flag would add spannerOptions coupling without removing a demonstrated behavioral defect. Remaining explicit assignments include intentional startup-only and presence/precedence policies. The inspected setters already share the relevant parsing helpers.

Do not change omitted STATEMENT_TIMEOUT from NULL to 10m as part of this refactor: execution chooses its contextual default later. A redundant MEDIUM priority assignment is insufficient benefit for a standalone PR. Leave these small sites explicit unless a concrete future change demonstrates drift.

The actionable remainder is #292 / PR6: richer generated user-facing metadata, examples, relationships and operation eligibility, plus the actual remaining prose/reference gaps. That should be specified independently against current source rather than reviving an old claim that the entire file is only TODOs. The #725 umbrella remains open; no manual closure is requested.

Reviewed baseline: current main 0ffacc862fe304e8db78c41a2f2912a31bc36793, with the unchanged startup implementation also inspected in 224d3053290c4d2d28ee1516e41f8571a243a239. Focused startup inventory and existing config/timeout/log/directed-read/prompt tests passed in the design investigation. No implementation PR was created for this deferral.