Skip to content

Conversation

@kirillkuts
Copy link
Collaborator

When comparing schemas, we were getting false positives about cluster changes even though nothing actually changed.

The problem:

  • When we compile schema from files, we weren't adding ON CLUSTER clauses
  • When we extract schema from the database, we DO add ON CLUSTER clauses
  • So diff thinks the cluster name changed every time, even when it didn't

Reproduction steps:

mkdir my-clickhouse-project
cd my-clickhouse-project
housekeeper init
echo "CREATE DATABASE \`analytics\` ENGINE = Atomic COMMENT 'Analytics database';" > db/main.sql
housekeeper diff
housekeeper diff

Expected result:
Second housekeeper diff finds no differences

Actual result:
Second housekeeper diff fails with cannot change cluster from 'cluster' to '': cluster configuration changes not supported error

The fix:
Moved the InjectOnCluster function to pkg/parser so both paths can use it:

  • pkg/parser/utils.go - moved InjectOnCluster to a new file inside parser package
  • pkg/clickhouse/dump_schema.go - now calls parser.InjectOnCluster
  • pkg/cmd/util.go - compileProjectSchema now also calls parser.InjectOnCluster when compiling schema

Now both schema extraction and compilation add ON CLUSTER the same way, so we don't get fake diffs anymore

Additional impact:

  • Migrations will automatically include ON CLUSTER into statements

@kirillkuts kirillkuts requested a review from pseudomuto October 12, 2025 01:42
@kirillkuts kirillkuts self-assigned this Oct 12, 2025
  When comparing schemas, we were getting false positives about cluster changes
  even though nothing actually changed.

  The problem:
  - When we compile schema from files, we weren't adding ON CLUSTER clauses
  - When we extract schema from the database, we DO add ON CLUSTER clauses
  - So diff thinks the cluster name changed every time, even when it didn't

  The fix:
  Moved the InjectOnCluster function to pkg/parser so both paths can use it:
  - pkg/parser/utils.go - new file with InjectOnCluster logic
  - pkg/clickhouse/dump_schema.go - now calls parser.InjectOnCluster
  - pkg/cmd/util.go - now also calls parser.InjectOnCluster when compiling

  Now both schema extraction and compilation add ON CLUSTER the same way,
  so we don't get fake diffs anymore

  Impact:
  - Migrations will automatically include `ON CLUSTER` into statements
@kirillkuts kirillkuts force-pushed the refactor/cluster-injection branch from eedbddf to a378dc7 Compare October 12, 2025 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants