patch: full table vocabulary + patch_view / patch_dictionary#156
Merged
Conversation
After #153, 70 objects in posthog-cloud-infra remained copied per env, each blocked on a field no patch could carry — ~30 on engine alone (Distributed targets that move with the env's topology), 6 on views and dictionaries with no patch form at all, the rest on index / order_by / ttl / column modification. patch_table now carries the full delta vocabulary. Columns: add (must not exist), modify_column (in-place replace, position kept, must exist), drop_columns (must exist) — applied modify, drop, add, so an add sees the post-drop state. Indexes: add and drop_indexes, drops first, so a drop+add pair redefines an index in one patch. order_by / partition_by / sample_by / ttl replace the target's value when set. The engine block replaces wholesale — merging engine sub-arguments is not meaningful. Settings keep their merge-patch-wins semantics. primary_key, comment, constraints, and projections stay non-patchable: a table differing there is genuinely different, which is override's job. patch_view (query, comment) and patch_dictionary (source / layout / lifetime replace wholesale, settings merge) extend the same idea to the object kinds that had no patch form. Both error on unknown targets. Patched values canonicalize exactly like declared ones — patch column and index expressions through the same normalization, patch_view queries through normalizeQuery, patch engine and dictionary source/layout decoded at parse — so a patched object converges with its introspected form. The locate scanner records patch_view / patch_dictionary as patch sites, keeping the -duplicates audit correct. Closes #154
orian
enabled auto-merge (squash)
July 18, 2026 09:36
This was referenced Jul 18, 2026
orian
added a commit
that referenced
this pull request
Jul 18, 2026
## Summary Implements #158: a `patch_table` column add can now be positioned, so an env whose extra columns interleave mid-table no longer forces a full redeclaration — the last large blocker (12 objects, the ~380-column tables among them) for the once-only rule in posthog-cloud-infra. ```hcl patch_table "person" { column "pmat_email" { type = "String" after = "is_deleted" # or first = true } column "pmat_name" { type = "String" after = "pmat_email" # adds chain in patch order } } ``` **Semantics** (both asks from the issue): - `after = "<name>"` inserts immediately after the named column; `first = true` inserts at the front; neither keeps today's append. Both together error. - Adds apply in patch order, each resolving against the post-previous-op state (the modify → drop → add discipline from #156): `after` may name a column added earlier in the same patch; a dropped or unknown target errors. - **Canonicalization**: placement is transient (cleared on application, `diff:"-"` like `renamed_from`) — the composed table renders **byte-identical** to the same table declared plainly in that order. The acceptance test asserts `Write(base+patch) == Write(flat)`, and the binary smoke test confirms it with `cmp`. - **Patch-only, by design**: `after`/`first` on a declared table column (including inherited from an abstract base), on an MV column, or on `modify_column` is a resolve error — repositioning *base* columns is genuine drift and stays a full redeclaration, exactly as the issue scoped it. Related: #88 (emitting `ADD COLUMN … FIRST/AFTER` in generated DDL) stays open — this is compose-side only, and the transient placement fields on `ColumnSpec` are the shared model plumbing #88 can build on. Plan doc: `docs/plans/2026-07-18-patch-column-position.md`; docs updated in the `patch_table` reference (column bullet + example), FAQ (positioned form added to the env-column recipe), README, CLAUDE.md. ## Test plan - [x] `patch_position_test.go`: the issue's person scenario with byte-parity (`Write(base+patch)` equals `Write(flat)`); chained `after`; `first` + append mix with cleared placement; `after` against post-modify/drop state; every error path (both set, unknown/dropped target, on `modify_column`, on a declared column, inherited from an abstract base, on an MV column) - [x] `go test -race ./internal/... ./cmd/... ./test` — all green; `gofmt -s` clean - [x] Binary smoke test: composed interleaved order, `cmp` byte-identical to the flat declaration, declared-column misuse rejected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements #154 — all five asks. After #153, 70 objects in posthog-cloud-infra were still copied per env, each blocked on a field no patch could carry (~30 on
enginealone). The patch vocabulary is now complete:patch_tablecarries the full delta:modify_columnkeeps position).enginereplaces wholesale — per the issue, merging engine sub-arguments is not meaningful. The Distributed-target-moves-with-topology case is now one engine block per env instead of a 20-line copy.primary_key,comment, constraints, projections — a table differing there is genuinely different, which isoverride = true's job.patch_view(query,comment) andpatch_dictionary(source/layout/lifetimereplace wholesale,settingsmerge) give the last two object kinds a patch form. Unknown targets error, likepatch_table.Correctness details:
patch_viewqueries throughnormalizeQuery, engine + dictionary source/layout decoded at parse) — a patched object converges with its introspected form (introspect is not round-trip-consistent with compose (sha-0409212): spurious diffs on unchanged view queries, MATERIALIZED columns, minmax indexes, and Distributed-proxy columns #136 discipline).locatescanner recordspatch_view/patch_dictionaryas patch sites, so-duplicatesstays exempt-correct.extend.Plan doc:
docs/plans/2026-07-17-patch-vocabulary.md. Docs: reference (patch_tablerewritten, newpatch_view/patch_dictionarysection, pipeline + decision table), FAQ (engine question now "yes", two new worked entries: Distributed-per-env and view/dictionary patching), README, CLAUDE.md.Test plan
patch_vocabulary_test.go: the issue'sraw_sessionsengine scenario through LoadLayers + a cross-env diff that is exactly the engine change; scalar replace-when-set; column modify/drop/add ordering + all error paths; index drop+add redefine + errors;patch_viewheredoc/one-liner convergence, comment replace, unknown target;patch_dictionarysource/lifetime replace + settings merge + unknown target; locate exemptionTestParseFile_PatchDisallowedAttribute(engine is legal now;primary_keystands in for the still-rejected fields)go test -race ./internal/... ./cmd/... ./test— 985 passed;gofmt -scleandiff -left cloud -right cloud,devreports exactly the engine change and query change