diff --git a/README.md b/README.md index 901cedd..9811288 100644 --- a/README.md +++ b/README.md @@ -289,6 +289,13 @@ hclexp diff -left ./schema/posthog.hcl \ and definition while ignoring declaration order. By default physical column order is significant because it affects `SELECT *`, positional inserts, and dump convergence. + +When materialized-view column names and definitions match but their declaration +order differs, the diff reports `column_order` with both ordered name lists and +states that recreating the view is required. It does not silently treat the +reorder as a content change. Interactive output from `diff` and +`plan -format text` also suggests `-ignore-column-order` as the explicit +opt-out; redirected and JSON output stay machine-clean. - `-exclude ` — apply the same HCL exclude config to both sides. For a live side, matching objects are skipped before their `CREATE` DDL is parsed, so transient or unsupported objects cannot abort the diff. @@ -748,8 +755,11 @@ hclexp plan \ `-from-manifest` and `-dump` are mutually exclusive. A proposed-only role plans as all-CREATE; a previous-only role is rejected because decommissioning a role must be explicit. `-format text` prints the same ordered list -human-readably. Require the reference-scoped live drift check to be empty -immediately before applying this reference-to-proposed plan. +human-readably. Pass `-ignore-column-order` to compare table and materialized- +view columns by name and definition instead of declaration position, matching +`diff`; the default remains order-sensitive. Require the reference-scoped live +drift check to be empty immediately before applying this reference-to-proposed +plan. See **[Cross-role planning](docs/README.hcl.md#cross-role-planning--hclexp-plan)** in the reference for the manifest format, and diff --git a/cmd/hclexp/diff_column_order_e2e_test.go b/cmd/hclexp/diff_column_order_e2e_test.go new file mode 100644 index 0000000..12cc844 --- /dev/null +++ b/cmd/hclexp/diff_column_order_e2e_test.go @@ -0,0 +1,83 @@ +package main + +import ( + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "testing" + + hclload "github.com/posthog/chschema/internal/loader/hcl" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDiffColumnOrderCLIProcess(t *testing.T) { + if os.Getenv("HCLEXP_DIFF_COLUMN_ORDER_HELPER") != "1" { + return + } + for i, arg := range os.Args { + if arg == "--" { + runDiff(os.Args[i+1:]) + os.Exit(0) + return + } + } + t.Fatal("missing diff CLI arguments") +} + +func TestDiffMaterializedViewColumnOrderEndToEnd(t *testing.T) { + root := t.TempDir() + left := filepath.Join(root, "left.hcl") + right := filepath.Join(root, "right.hcl") + require.NoError(t, os.WriteFile(left, []byte(materializedViewOrderHCL("a", "b")), 0o600)) + require.NoError(t, os.WriteFile(right, []byte(materializedViewOrderHCL("b", "a")), 0o600)) + + text, err := runDiffColumnOrderCLI(left, right) + require.NoError(t, err, string(text)) + assert.Contains(t, string(text), "UNSAFE: materialized view column order changed; recreating the view is required") + assert.Contains(t, string(text), "~ column_order: a, b -> b, a") + assert.NotContains(t, string(text), "~ columns changed") + + structured, err := runDiffColumnOrderCLI(left, right, "-format", "json") + require.NoError(t, err, string(structured)) + var doc hclload.DiffJSON + require.NoError(t, json.Unmarshal(structured, &doc), string(structured)) + require.Len(t, doc.Objects, 1) + assert.Equal(t, []hclload.FieldChange{{ + Field: "column_order", Change: "modify", Old: "a, b", New: "b, a", + }}, doc.Objects[0].Changes) + assert.True(t, doc.Objects[0].Unsafe) + assert.Equal(t, "materialized view column order changed; recreating the view is required", doc.Objects[0].UnsafeReason) + assert.Empty(t, doc.Operations) + + ignored, err := runDiffColumnOrderCLI(left, right, "-ignore-column-order") + require.NoError(t, err, string(ignored)) + assert.Equal(t, "no differences\n", string(ignored)) +} + +func materializedViewOrderHCL(first, second string) string { + return fmt.Sprintf(`database "posthog" { + table "destination" { + column "a" { type = "UInt8" } + column "b" { type = "UInt8" } + engine "log" {} + } + materialized_view "events_mv" { + to_table = "destination" + query = "SELECT a, b FROM source" + column %q { type = "UInt8" } + column %q { type = "UInt8" } + } +} +`, first, second) +} + +func runDiffColumnOrderCLI(left, right string, extra ...string) ([]byte, error) { + args := []string{"-test.run=^TestDiffColumnOrderCLIProcess$", "--", "-left", left, "-right", right} + args = append(args, extra...) + cmd := exec.Command(os.Args[0], args...) + cmd.Env = append(os.Environ(), "HCLEXP_DIFF_COLUMN_ORDER_HELPER=1") + return cmd.Output() +} diff --git a/cmd/hclexp/hclexp.go b/cmd/hclexp/hclexp.go index 58b785c..d0f3534 100644 --- a/cmd/hclexp/hclexp.go +++ b/cmd/hclexp/hclexp.go @@ -16,6 +16,7 @@ import ( "github.com/ClickHouse/clickhouse-go/v2/lib/driver" "github.com/posthog/chschema/config" hclload "github.com/posthog/chschema/internal/loader/hcl" + "golang.org/x/term" ) func main() { @@ -946,6 +947,7 @@ func runDiff(args []string) { cs := hclload.DiffWithOptions(left, right, hclload.DiffOptions{IgnoreColumnOrder: *ignoreColumnOrder}) gen := hclload.GenerateSQL(cs) + showColumnOrderHint := shouldShowColumnOrderHint(cs, *ignoreColumnOrder, term.IsTerminal(int(os.Stdout.Fd()))) if *formatFlag == "json" { out, err := hclload.RenderDiffJSON(cs, gen, left, right) @@ -961,6 +963,9 @@ func runDiff(args []string) { for _, u := range gen.Unsafe { fmt.Printf("-- UNSAFE: %s: %s\n", qualifiedName(u.Database, u.Table), u.Reason) } + if showColumnOrderHint { + renderColumnOrderHint(os.Stdout, true) + } for i, stmt := range gen.Statements { if gen.Ops[i].Manual { fmt.Println("-- MANUAL: " + stmt + ";") @@ -980,6 +985,36 @@ func runDiff(args []string) { } hclload.RenderObjectComparisons(os.Stdout, hclload.BuildObjectComparisons(cs, gen, left, right)) + if showColumnOrderHint { + renderColumnOrderHint(os.Stdout, false) + } +} + +func shouldShowColumnOrderHint(cs hclload.ChangeSet, ignoreColumnOrder, terminal bool) bool { + if ignoreColumnOrder || !terminal { + return false + } + for _, dc := range cs.Databases { + for _, td := range dc.AlterTables { + if td.ColumnOrderChange != nil { + return true + } + } + for _, mvd := range dc.AlterMaterializedViews { + if mvd.ColumnOrderChange != nil { + return true + } + } + } + return false +} + +func renderColumnOrderHint(w io.Writer, sql bool) { + prefix := "\nhint:" + if sql { + prefix = "-- hint:" + } + fmt.Fprintf(w, "%s if column order is not significant for this comparison, rerun with -ignore-column-order\n", prefix) } // applyDiffScope narrows one side of a two-way comparison to the logical diff --git a/cmd/hclexp/hclexp_test.go b/cmd/hclexp/hclexp_test.go index 0fcb917..cb6832a 100644 --- a/cmd/hclexp/hclexp_test.go +++ b/cmd/hclexp/hclexp_test.go @@ -364,6 +364,30 @@ func TestRenderChangeSet_MaterializedViews(t *testing.T) { require.Equal(t, want, buf.String()) } +func TestColumnOrderHintIsTerminalOnly(t *testing.T) { + cs := hclload.ChangeSet{Databases: []hclload.DatabaseChange{{ + Database: "posthog", + AlterMaterializedViews: []hclload.MaterializedViewDiff{{ + Name: "mv", Recreate: true, ColumnsChanged: true, + ColumnOrderChange: &hclload.OrderByChange{Old: []string{"a", "b"}, New: []string{"b", "a"}}, + }}, + }}} + require.True(t, shouldShowColumnOrderHint(cs, false, true)) + require.False(t, shouldShowColumnOrderHint(cs, false, false), "redirected output stays machine-clean") + require.False(t, shouldShowColumnOrderHint(cs, true, true), "the user already selected the opt-out") + + genericColumns := cs + genericColumns.Databases[0].AlterMaterializedViews[0].ColumnOrderChange = nil + require.False(t, shouldShowColumnOrderHint(genericColumns, false, true), "content changes do not suggest ignoring order") + + var text bytes.Buffer + renderColumnOrderHint(&text, false) + require.Equal(t, "\nhint: if column order is not significant for this comparison, rerun with -ignore-column-order\n", text.String()) + var sql bytes.Buffer + renderColumnOrderHint(&sql, true) + require.Equal(t, "-- hint: if column order is not significant for this comparison, rerun with -ignore-column-order\n", sql.String()) +} + func TestRenderChangeSet_Views(t *testing.T) { cs := hclload.ChangeSet{ Databases: []hclload.DatabaseChange{ diff --git a/cmd/hclexp/plan.go b/cmd/hclexp/plan.go index 2fda723..6fc3889 100644 --- a/cmd/hclexp/plan.go +++ b/cmd/hclexp/plan.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/hcl/v2/gohcl" "github.com/hashicorp/hcl/v2/hclparse" hclload "github.com/posthog/chschema/internal/loader/hcl" + "golang.org/x/term" ) // planManifest is the HCL manifest: role blocks, each with one env block per @@ -76,6 +77,7 @@ func runPlan(args []string) { scopeFlag := fs.String("scope", "all", "dump object scope: all (exact) or desired (ignore live-only objects)") formatFlag := fs.String("format", "json", "output format: json (default) or text") excludeFlag := fs.String("exclude", "", "HCL exclude config: objects matching its patterns/object_types are dropped from both sides before diffing") + ignoreColumnOrder := fs.Bool("ignore-column-order", false, "ignore table and materialized-view column declaration order") _ = fs.Parse(args) if *manifestFlag == "" || *envFlag == "" { @@ -139,7 +141,7 @@ func runPlan(args []string) { } } - plan := hclload.BuildPlan(roleDiffs) + plan := hclload.BuildPlanWithOptions(roleDiffs, hclload.DiffOptions{IgnoreColumnOrder: *ignoreColumnOrder}) if *formatFlag == "json" { out, err := json.MarshalIndent(plan, "", " ") @@ -151,6 +153,25 @@ func runPlan(args []string) { return } renderPlanText(os.Stdout, plan) + if shouldShowPlanColumnOrderHint(plan, *ignoreColumnOrder, term.IsTerminal(int(os.Stdout.Fd()))) { + renderColumnOrderHint(os.Stdout, false) + } +} + +func shouldShowPlanColumnOrderHint(plan hclload.PlanResult, ignoreColumnOrder, terminal bool) bool { + if ignoreColumnOrder || !terminal { + return false + } + for _, role := range plan.Roles { + for _, object := range role.Objects { + for _, change := range object.Changes { + if change.Field == "column_order" { + return true + } + } + } + } + return false } func roleDiffsFromDump( diff --git a/cmd/hclexp/plan_column_order_e2e_test.go b/cmd/hclexp/plan_column_order_e2e_test.go new file mode 100644 index 0000000..53966b6 --- /dev/null +++ b/cmd/hclexp/plan_column_order_e2e_test.go @@ -0,0 +1,99 @@ +package main + +import ( + "encoding/json" + "os" + "os/exec" + "path/filepath" + "testing" + + hclload "github.com/posthog/chschema/internal/loader/hcl" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestPlanColumnOrderCLIProcess(t *testing.T) { + if os.Getenv("HCLEXP_PLAN_COLUMN_ORDER_HELPER") != "1" { + return + } + for i, arg := range os.Args { + if arg == "--" { + runPlan(os.Args[i+1:]) + os.Exit(0) + return + } + } + t.Fatal("missing plan CLI arguments") +} + +func TestPlanIgnoreColumnOrderEndToEnd(t *testing.T) { + root := t.TempDir() + desiredRoot := filepath.Join(root, "desired") + dumpRoot := filepath.Join(root, "dump") + manifest := `role "ops" { + env "prod" { layers = ["schema.hcl"] } +} +` + writeFileT(t, filepath.Join(desiredRoot, "manifest.hcl"), manifest) + writeFileT(t, filepath.Join(desiredRoot, "schema.hcl"), planColumnOrderHCL("b", "a")) + writeFileT(t, filepath.Join(dumpRoot, "prod-us-iad-ch-1a-ops.hcl"), `node "prod-us-iad-ch-1a-ops" { + macros = { cluster = "ops", hostClusterRole = "ops", shard = "1", replica = "a" } +} +`+planColumnOrderHCL("a", "b")) + + args := []string{ + "-manifest", filepath.Join(desiredRoot, "manifest.hcl"), + "-layer-root", desiredRoot, + "-env", "prod", + "-dump", dumpRoot, + } + output, err := runPlanColumnOrderCLI(args...) + require.NoError(t, err, string(output)) + var orderSensitive hclload.PlanResult + require.NoError(t, json.Unmarshal(output, &orderSensitive), string(output)) + require.Len(t, orderSensitive.Unsafe, 2) + require.Len(t, orderSensitive.Roles, 1) + require.Len(t, orderSensitive.Roles[0].Objects, 2) + for _, object := range orderSensitive.Roles[0].Objects { + assert.Equal(t, []hclload.FieldChange{{ + Field: "column_order", Change: "modify", Old: "a, b", New: "b, a", + }}, object.Changes) + } + + ignored, err := runPlanColumnOrderCLI(append(args, "-ignore-column-order")...) + require.NoError(t, err, string(ignored)) + var clean hclload.PlanResult + require.NoError(t, json.Unmarshal(ignored, &clean), string(ignored)) + assert.Empty(t, clean.Operations) + assert.Empty(t, clean.Unsafe) + require.Len(t, clean.Roles, 1) + assert.Empty(t, clean.Roles[0].Objects) + + text, err := runPlanColumnOrderCLI(append(args, "-ignore-column-order", "-format", "text")...) + require.NoError(t, err, string(text)) + assert.Equal(t, "no changes\n", string(text)) +} + +func planColumnOrderHCL(first, second string) string { + return `database "posthog" { + table "events" { + column "` + first + `" { type = "UInt64" } + column "` + second + `" { type = "UInt64" } + engine "log" {} + } + materialized_view "events_mv" { + to_table = "events" + query = "SELECT a, b FROM source" + column "` + first + `" { type = "UInt64" } + column "` + second + `" { type = "UInt64" } + } +} +` +} + +func runPlanColumnOrderCLI(args ...string) ([]byte, error) { + commandArgs := append([]string{"-test.run=^TestPlanColumnOrderCLIProcess$", "--"}, args...) + cmd := exec.Command(os.Args[0], commandArgs...) + cmd.Env = append(os.Environ(), "HCLEXP_PLAN_COLUMN_ORDER_HELPER=1") + return cmd.Output() +} diff --git a/cmd/hclexp/plan_render_test.go b/cmd/hclexp/plan_render_test.go index 2546c06..4adfb49 100644 --- a/cmd/hclexp/plan_render_test.go +++ b/cmd/hclexp/plan_render_test.go @@ -25,6 +25,18 @@ func TestPlanRenderEmpty(t *testing.T) { assert.Equal(t, "no changes\n", renderPlanToString(t, hclload.PlanResult{})) } +func TestPlanColumnOrderHintIsTerminalOnly(t *testing.T) { + plan := hclload.PlanResult{Roles: []hclload.RoleComparison{{ + Role: "ops", Objects: []hclload.ObjectComparison{{ + Object: "events", Changes: []hclload.FieldChange{{Field: "column_order", Change: "modify"}}, + }}, + }}} + assert.True(t, shouldShowPlanColumnOrderHint(plan, false, true)) + assert.False(t, shouldShowPlanColumnOrderHint(plan, false, false), "redirected output stays machine-clean") + assert.False(t, shouldShowPlanColumnOrderHint(plan, true, true), "the user already selected the opt-out") + assert.False(t, shouldShowPlanColumnOrderHint(hclload.PlanResult{}, false, true)) +} + // TestPlanRenderUnsafeOnly: unsafe changes that produce no statement (engine or // ORDER BY swap needing a full recreate) still print their warnings before the // "no changes" summary. diff --git a/docs/README.hcl.md b/docs/README.hcl.md index 0f2e408..fcd4d21 100644 --- a/docs/README.hcl.md +++ b/docs/README.hcl.md @@ -1010,6 +1010,9 @@ role "data" { `operations`, the JSON carries a `roles` list: each role's own [object comparisons](#structured-comparison-output) with derived counts, deliberately **not** deduped (triage is per role, execution is global). +- `-ignore-column-order` compares table and materialized-view columns by name + and definition rather than declaration position, with the same semantics as + `diff`. The default remains order-sensitive. - `-exclude` drops matching objects from both sides of every role's diff. Desired-scoped dump planning is the safe live-convergence form for a managed @@ -1206,10 +1209,11 @@ so its DROP+CREATE is the destructive one. | `column:` | table | | `index:`, `projection:`, `constraint:` | table | | `setting:` | table | -| `engine`, `column_order`, `order_by`, `primary_key`, `partition_by`, `sample_by`, `ttl` | table | +| `engine`, `order_by`, `primary_key`, `partition_by`, `sample_by`, `ttl` | table | +| `column_order` | table, materialized view | | `comment` | table, view, named collection | | `query` | view, materialized view | -| `to_table`, `columns` | materialized view (either forces a recreate) | +| `to_table`, `columns` | materialized view (either forces a recreate; `columns` means names or definitions differ) | | `column_aliases`, `sql_security`, `definer`, `cluster` | view (each forces a recreate) | | `param:`, `on_cluster` | named collection | | `sql` | raw block | @@ -1218,7 +1222,10 @@ so its DROP+CREATE is the destructive one. How values render: a column as a compact descriptor (`Nullable(String) MATERIALIZED upper(s) CODEC(LZ4)`), an engine as its SQL clause, and `column_order`/ -`order_by`/`primary_key` comma-joined. A rename is reported on the **new** name (`column:`, with `old` +`order_by`/`primary_key` comma-joined. For an order-only difference, interactive +output from `diff` and `plan -format text` also suggests +`-ignore-column-order`; redirected and JSON output contain only the structured +comparison. A rename is reported on the **new** name (`column:`, with `old` = the previous name). Two cases carry no per-field values, because the diff holds none: a dictionary reconciles via `CREATE OR REPLACE`, so it emits one `modify` per changed config path; and a named-collection `param:` set is always `modify` diff --git a/internal/loader/hcl/compare.go b/internal/loader/hcl/compare.go index 92ab45a..78b1768 100644 --- a/internal/loader/hcl/compare.go +++ b/internal/loader/hcl/compare.go @@ -310,7 +310,12 @@ func fieldChangesForMaterializedView(mvd MaterializedViewDiff) []FieldChange { if c := mvd.ToTableChange; c != nil { out = append(out, stringChangeField("to_table", c)) } - if mvd.ColumnsChanged { + if c := mvd.ColumnOrderChange; c != nil { + out = append(out, FieldChange{ + Field: "column_order", Change: "modify", + Old: strings.Join(c.Old, ", "), New: strings.Join(c.New, ", "), + }) + } else if mvd.ColumnsChanged { out = append(out, FieldChange{Field: "columns", Change: "modify"}) } if c := mvd.QueryChange; c != nil { diff --git a/internal/loader/hcl/compare_test.go b/internal/loader/hcl/compare_test.go index dc35d1f..ea6c432 100644 --- a/internal/loader/hcl/compare_test.go +++ b/internal/loader/hcl/compare_test.go @@ -217,6 +217,16 @@ func TestFieldChangesForMaterializedView(t *testing.T) { assert.Equal(t, []FieldChange{ {Field: "query", Change: "modify", Old: "SELECT 1", New: "SELECT 2"}, }, fieldChangesForMaterializedView(mvd)) + + // An order-only column difference is explicit and replaces the opaque + // generic "columns changed" field. + mvd = MaterializedViewDiff{ + Name: "mv", Recreate: true, ColumnsChanged: true, + ColumnOrderChange: &OrderByChange{Old: []string{"a", "b"}, New: []string{"b", "a"}}, + } + assert.Equal(t, []FieldChange{{ + Field: "column_order", Change: "modify", Old: "a, b", New: "b, a", + }}, fieldChangesForMaterializedView(mvd)) } func TestFieldChangesForView(t *testing.T) { diff --git a/internal/loader/hcl/diff.go b/internal/loader/hcl/diff.go index ee42ac0..ec3492b 100644 --- a/internal/loader/hcl/diff.go +++ b/internal/loader/hcl/diff.go @@ -100,8 +100,13 @@ type MaterializedViewDiff struct { // Detail fields used by renderers. ColumnsChanged can accompany a safe // QueryChange when the desired output is a compatible additive projection. - ToTableChange *StringChange - ColumnsChanged bool + ToTableChange *StringChange + + // ColumnOrderChange is populated when the column names and definitions are + // identical and declaration order is the only column difference. The view + // still requires recreation; this field makes that reason diagnosable. + ColumnOrderChange *OrderByChange + ColumnsChanged bool } func (mvd MaterializedViewDiff) IsEmpty() bool { @@ -740,6 +745,12 @@ func diffMaterializedView(database string, from, to *MaterializedViewSpec, toR T } if !materializedViewColumnsEqual(from.Columns, to.Columns, options.IgnoreColumnOrder) { mvd.ColumnsChanged = true + if !options.IgnoreColumnOrder && materializedViewColumnsEqual(from.Columns, to.Columns, true) { + mvd.ColumnOrderChange = &OrderByChange{ + Old: orderedColumnNames(from.Columns), + New: orderedColumnNames(to.Columns), + } + } } queryChanged := from.Query != to.Query if mvd.ToTableChange != nil { @@ -845,6 +856,14 @@ func materializedViewColumnsEqual(from, to []ColumnSpec, ignoreOrder bool) bool return true } +func orderedColumnNames(columns []ColumnSpec) []string { + names := make([]string, len(columns)) + for i := range columns { + names[i] = columns[i].Name + } + return names +} + func indexTables(tables []TableSpec) map[string]*TableSpec { out := make(map[string]*TableSpec, len(tables)) for i := range tables { diff --git a/internal/loader/hcl/diff_test.go b/internal/loader/hcl/diff_test.go index 18aa7ec..744fbab 100644 --- a/internal/loader/hcl/diff_test.go +++ b/internal/loader/hcl/diff_test.go @@ -217,7 +217,25 @@ func TestDiff_MaterializedViewColumnOrderPolicy(t *testing.T) { Columns: []ColumnSpec{{Name: "b", Type: "UInt8"}, {Name: "a", Type: "UInt8"}}, }}}}} - assert.False(t, Diff(from, to).IsEmpty()) + cs := Diff(from, to) + assert.False(t, cs.IsEmpty()) + require.Len(t, cs.Databases, 1) + require.Len(t, cs.Databases[0].AlterMaterializedViews, 1) + mvd := cs.Databases[0].AlterMaterializedViews[0] + assert.True(t, mvd.ColumnsChanged) + assert.True(t, mvd.Recreate) + assert.Equal(t, &OrderByChange{ + Old: []string{"a", "b"}, + New: []string{"b", "a"}, + }, mvd.ColumnOrderChange) + generated := GenerateSQL(cs) + require.Len(t, generated.Unsafe, 1) + assert.Equal(t, "materialized view column order changed; recreating the view is required", generated.Unsafe[0].Reason) + comparisons := BuildObjectComparisons(cs, generated, from, to) + require.Len(t, comparisons, 1) + assert.Equal(t, []FieldChange{{ + Field: "column_order", Change: "modify", Old: "a, b", New: "b, a", + }}, comparisons[0].Changes) assert.True(t, DiffWithOptions(from, to, DiffOptions{IgnoreColumnOrder: true}).IsEmpty()) } diff --git a/internal/loader/hcl/plan.go b/internal/loader/hcl/plan.go index 309bed1..a617691 100644 --- a/internal/loader/hcl/plan.go +++ b/internal/loader/hcl/plan.go @@ -65,6 +65,12 @@ type PlanResult struct { // the MV); DROP runs in reverse. Identical statements across roles dedupe to a // single operation carrying the union of contributing roles. func BuildPlan(roles []RoleDiff) PlanResult { + return BuildPlanWithOptions(roles, DiffOptions{}) +} + +// BuildPlanWithOptions is BuildPlan with the same comparison policy controls +// used by the other schema-comparison entry points. +func BuildPlanWithOptions(roles []RoleDiff, options DiffOptions) PlanResult { type opKey struct{ kind, db, object, sql string } var firstSeen []opKey @@ -75,7 +81,7 @@ func BuildPlan(roles []RoleDiff) PlanResult { // as DiffJSON.Objects). roleComparisons := make([]RoleComparison, 0, len(roles)) for _, rd := range roles { - cs := Diff(rd.Current, rd.Desired) + cs := DiffWithOptions(rd.Current, rd.Desired, options) gen := GenerateSQL(cs) objs := BuildObjectComparisons(cs, gen, rd.Current, rd.Desired) roleComparisons = append(roleComparisons, RoleComparison{ diff --git a/internal/loader/hcl/plan_test.go b/internal/loader/hcl/plan_test.go index 2e38ec5..725e3ee 100644 --- a/internal/loader/hcl/plan_test.go +++ b/internal/loader/hcl/plan_test.go @@ -134,6 +134,42 @@ func TestBuildPlan_CrossRoleAlterOrdering(t *testing.T) { assert.True(t, sharded.Replicated) } +func TestBuildPlanWithOptions_IgnoreColumnOrder(t *testing.T) { + column := func(name string) ColumnSpec { return ColumnSpec{Name: name, Type: "UInt64"} } + table := func(columns ...ColumnSpec) TableSpec { + return TableSpec{Name: "events", Columns: columns, Engine: &EngineSpec{Kind: "log", Decoded: EngineLog{}}} + } + view := func(columns ...ColumnSpec) MaterializedViewSpec { + return MaterializedViewSpec{ + Name: "events_mv", ToTable: "events", Query: "SELECT a, b FROM source", Columns: columns, + } + } + schema := func(columns ...ColumnSpec) *Schema { + return &Schema{Databases: []DatabaseSpec{{ + Name: "posthog", Tables: []TableSpec{table(columns...)}, MaterializedViews: []MaterializedViewSpec{view(columns...)}, + }}} + } + + roles := []RoleDiff{{ + Role: "ops", Current: schema(column("a"), column("b")), Desired: schema(column("b"), column("a")), + }} + orderSensitive := BuildPlan(roles) + require.Len(t, orderSensitive.Unsafe, 2) + require.Len(t, orderSensitive.Roles, 1) + require.Len(t, orderSensitive.Roles[0].Objects, 2) + for _, object := range orderSensitive.Roles[0].Objects { + assert.Equal(t, []FieldChange{{ + Field: "column_order", Change: "modify", Old: "a, b", New: "b, a", + }}, object.Changes) + } + + ignored := BuildPlanWithOptions(roles, DiffOptions{IgnoreColumnOrder: true}) + assert.Empty(t, ignored.Operations) + assert.Empty(t, ignored.Unsafe) + require.Len(t, ignored.Roles, 1) + assert.Empty(t, ignored.Roles[0].Objects) +} + // Removed objects exist only in current, so the current-side union must drive // DROP ordering and metadata. This also exercises the cross-role case where // the storage table and its dependants are discovered in different role diffs. diff --git a/internal/loader/hcl/sqlgen.go b/internal/loader/hcl/sqlgen.go index f434100..f224c64 100644 --- a/internal/loader/hcl/sqlgen.go +++ b/internal/loader/hcl/sqlgen.go @@ -190,9 +190,13 @@ func GenerateSQL(cs ChangeSet) GeneratedSQL { for _, dc := range cs.Databases { for _, mvd := range dc.AlterMaterializedViews { if mvd.Recreate { + reason := "materialized view to_table or incompatible column list change requires recreating the view" + if mvd.ToTableChange == nil && mvd.ColumnOrderChange != nil { + reason = "materialized view column order changed; recreating the view is required" + } out.Unsafe = append(out.Unsafe, UnsafeChange{ Database: dc.Database, Table: mvd.Name, - Reason: "materialized view to_table or incompatible column list change requires recreating the view", + Reason: reason, }) continue }