Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <file>` — 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.
Expand Down Expand Up @@ -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
Expand Down
83 changes: 83 additions & 0 deletions cmd/hclexp/diff_column_order_e2e_test.go
Original file line number Diff line number Diff line change
@@ -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()
}
35 changes: 35 additions & 0 deletions cmd/hclexp/hclexp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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)
Expand All @@ -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 + ";")
Expand All @@ -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
Expand Down
24 changes: 24 additions & 0 deletions cmd/hclexp/hclexp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
23 changes: 22 additions & 1 deletion cmd/hclexp/plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 == "" {
Expand Down Expand Up @@ -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, "", " ")
Expand All @@ -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(
Expand Down
99 changes: 99 additions & 0 deletions cmd/hclexp/plan_column_order_e2e_test.go
Original file line number Diff line number Diff line change
@@ -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()
}
12 changes: 12 additions & 0 deletions cmd/hclexp/plan_render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading