Fix ClickHouse 23.x/24.x compatibility in ci_finished_builds engine swap

Narendranrequested to merge
narendran-kannan/593129/fix-ch-23-24-compat-engine-swap into master

What does this MR do and why?

Follow-up to !240219 (merged), which fixed migration 20260108073937_alter_ci_finished_builds_engine_with_version to clone the live ci_finished_builds schema via CREATE TABLE ci_finished_builds_tmp AS ci_finished_builds ENGINE = ... SETTINGS ....

That form works on ClickHouse 25.x and 26.x (which is what master's CI matrix tests) but is rejected by ClickHouse 23.x and 24.x with:

Code: 36. DB::Exception: You must provide an ORDER BY or PRIMARY KEY expression
in the table definition. (BAD_ARGUMENTS)

Older ClickHouse versions don't inherit ORDER BY (or PARTITION BY) from the source table when using this CREATE form — they require both to be restated explicitly.

The fix is minimal: add explicit PARTITION BY toYear(finished_at) and ORDER BY (status, runner_type, project_id, finished_at, id) clauses to the CREATE TABLE ... AS statement. These keys are stable across all GitLab versions that have this table (they were set at table creation and have never changed), so restating them is safe.

Why this matters on master even though 19.x dropped CH 23/24 support

The docs state GitLab 19.0+ requires ClickHouse 25.x or 26.x, so on a fully-supported configuration this migration only ever runs against modern ClickHouse. However:

  1. Migration files live forever in db/click_house/post_migrate/main/. If a customer has the migration recorded in schema_migrations as failed/pending (e.g. they hit the original bug), and they upgrade through several GitLab versions before re-running migrations, the file from whatever GitLab version they're currently on runs, not the version that originally shipped the migration.
  2. Customers can run unsupported configurations during upgrade windows. A defensive fix that works on every ClickHouse version costs nothing.
  3. Maintenance hygiene: the same migration file ships in backports to 18.10 / 18.11 (which still support CH 23/24). Keeping master and the stable branches identical (a strict subset relationship) prevents future cherry-pick conflicts and avoids the confusion of "why is the explicit-keys form on the stable branch but not on master?".

This MR is being landed on master before the corresponding backports to 18.10 / 18.11 / 19.0, so the stable branches will be a strict subset of master.

Database changes

ClickHouse only. The post-deploy migration db/click_house/post_migrate/main/20260108073937_alter_ci_finished_builds_engine_with_version.rb is amended in place (it has already shipped in 18.9.0–19.1.x and !240219 (merged)'s first fix landed in 19.1).

Diff is a single addition to the CREATE TABLE statement:

CREATE TABLE IF NOT EXISTS ci_finished_builds_tmp AS ci_finished_builds
  ENGINE = #{engine}
  PARTITION BY toYear(finished_at)                                    -- NEW
  ORDER BY (status, runner_type, project_id, finished_at, id)         -- NEW
  SETTINGS #{settings};

No new migration files. No PostgreSQL changes. No behavioural change on modern ClickHouse.

Screenshots or screen recordings

N/A — backend ClickHouse migration, no UI.

Local verification matrix

Verified locally against three ClickHouse versions before pushing:

ClickHouse Old (broken) CREATE New (fixed) CREATE ATTACH PARTITION EXCHANGE Final engine
23.11.3 Code: 36 BAD_ARGUMENTS ✗ OK ✓ OK ✓ OK ✓ ReplacingMergeTree(version, deleted) ✓
24.9.1 Code: 36 BAD_ARGUMENTS ✗ OK ✓ OK ✓ OK ✓ ReplacingMergeTree(version, deleted) ✓
26.3.1 (local GDK) OK (still works) OK ✓ OK ✓ OK ✓ ReplacingMergeTree(version, deleted) ✓

The 23.11 and 24.9 verifications were done via dockerised ClickHouse (see snippet below); 26.3 was the local GDK.

References

How to set up and validate locally

The bug only reproduces against ClickHouse 23.x or 24.x. The local GDK uses ClickHouse 26.x, which is not affected. Reviewers can reproduce against an older ClickHouse in two ways:

Option A — Quick reviewer check (one ClickHouse 24.9 container)

The fastest way to confirm the fix: start a single dockerised ClickHouse 24.9 instance, run the OLD statement (expect a Code: 36 error), then run the NEW statement (expect success), then ATTACH PARTITION.

# Start ClickHouse 24.9
docker run -d --name ch-review -p 18249:8123 clickhouse/clickhouse-server:24.9.1.3278-alpine
sleep 5

# A small helper so we don't repeat the curl boilerplate
ch() { curl -s --data-binary "$1" "http://localhost:18249/"; }

# Set up a source table that mirrors the real ci_finished_builds shape
ch "DROP TABLE IF EXISTS ci_finished_builds_tmp SYNC"
ch "DROP TABLE IF EXISTS ci_finished_builds SYNC"
ch "CREATE TABLE ci_finished_builds (
      \`id\` UInt64 DEFAULT 0,
      \`project_id\` UInt64 DEFAULT 0,
      \`status\` LowCardinality(String) DEFAULT '',
      \`finished_at\` DateTime64(6, 'UTC') DEFAULT 0,
      \`runner_type\` UInt8 DEFAULT 0,
      \`version\` DateTime64(6, 'UTC') DEFAULT now(),
      \`deleted\` Bool DEFAULT false
    )
    ENGINE = ReplacingMergeTree
    PARTITION BY toYear(finished_at)
    ORDER BY (status, runner_type, project_id, finished_at, id)
    SETTINGS index_granularity = 8192, use_async_block_ids_cache = true, deduplicate_merge_projection_mode = 'rebuild'"
ch "INSERT INTO ci_finished_builds (id, project_id, status, finished_at, runner_type)
    VALUES (1, 100, 'success', now(), 1)"
ch "ALTER TABLE ci_finished_builds ADD COLUMN IF NOT EXISTS \`group_name\` String DEFAULT ''"
ch "ALTER TABLE ci_finished_builds ADD PROJECTION IF NOT EXISTS projection_by_project
    (SELECT project_id, count() AS total GROUP BY project_id)"

echo "--- OLD (broken) form, expect Code: 36 ---"
ch "CREATE TABLE ci_finished_builds_tmp AS ci_finished_builds
    ENGINE = ReplacingMergeTree(version, deleted)
    SETTINGS index_granularity = 8192"

echo "--- NEW (this MR) form, expect no error ---"
ch "CREATE TABLE ci_finished_builds_tmp AS ci_finished_builds
    ENGINE = ReplacingMergeTree(version, deleted)
    PARTITION BY toYear(finished_at)
    ORDER BY (status, runner_type, project_id, finished_at, id)
    SETTINGS index_granularity = 8192"
echo "tmp exists? $(ch 'EXISTS TABLE ci_finished_builds_tmp')"

echo "--- ATTACH PARTITION, expect no error ---"
ch "ALTER TABLE ci_finished_builds_tmp ATTACH PARTITION 2026 FROM ci_finished_builds"
echo "tmp row count: $(ch 'SELECT count() FROM ci_finished_builds_tmp')"

docker rm -f ch-review

Expected output:

--- OLD (broken) form, expect Code: 36 ---
Code: 36. DB::Exception: You must provide an ORDER BY or PRIMARY KEY expression
in the table definition. ... (BAD_ARGUMENTS) (version 24.9.1.3278 (official build))
--- NEW (this MR) form, expect no error ---
tmp exists? 1
--- ATTACH PARTITION, expect no error ---
tmp row count: 1

Option B — Full verification matrix (both CH 23.11 and 24.9)

To reproduce both rows of the verification matrix above, run the same body of the script in a loop across both ports:

docker run -d --name ch249  -p 18249:8123 clickhouse/clickhouse-server:24.9.1.3278-alpine
docker run -d --name ch2311 -p 18231:8123 clickhouse/clickhouse-server:23.11.3.23-alpine
sleep 5

setup_and_test() {
  local port=$1
  local extra_setting=""
  # deduplicate_merge_projection_mode was added in CH 24.1; CH 23.11 will reject it.
  [ "$port" = "18249" ] && extra_setting=", deduplicate_merge_projection_mode = 'rebuild'"

  ch() { curl -s --data-binary "$1" "http://localhost:${port}/"; }

  ch "DROP TABLE IF EXISTS ci_finished_builds_tmp SYNC"
  ch "DROP TABLE IF EXISTS ci_finished_builds SYNC"
  ch "CREATE TABLE ci_finished_builds (
        \`id\` UInt64 DEFAULT 0,
        \`project_id\` UInt64 DEFAULT 0,
        \`status\` LowCardinality(String) DEFAULT '',
        \`finished_at\` DateTime64(6, 'UTC') DEFAULT 0,
        \`runner_type\` UInt8 DEFAULT 0,
        \`version\` DateTime64(6, 'UTC') DEFAULT now(),
        \`deleted\` Bool DEFAULT false
      )
      ENGINE = ReplacingMergeTree
      PARTITION BY toYear(finished_at)
      ORDER BY (status, runner_type, project_id, finished_at, id)
      SETTINGS index_granularity = 8192, use_async_block_ids_cache = true${extra_setting}"
  ch "INSERT INTO ci_finished_builds (id, project_id, status, finished_at, runner_type)
      VALUES (1, 100, 'success', now(), 1)"
  ch "ALTER TABLE ci_finished_builds ADD COLUMN IF NOT EXISTS \`group_name\` String DEFAULT ''"
  ch "ALTER TABLE ci_finished_builds ADD PROJECTION IF NOT EXISTS projection_by_project
      (SELECT project_id, count() AS total GROUP BY project_id)"

  echo "=== CH $(ch 'SELECT version()') (port ${port}) ==="
  echo "OLD form: $(ch "CREATE TABLE ci_finished_builds_tmp AS ci_finished_builds ENGINE = ReplacingMergeTree(version, deleted) SETTINGS index_granularity = 8192" | head -1)"
  echo "NEW form: $(ch "CREATE TABLE ci_finished_builds_tmp AS ci_finished_builds ENGINE = ReplacingMergeTree(version, deleted) PARTITION BY toYear(finished_at) ORDER BY (status, runner_type, project_id, finished_at, id) SETTINGS index_granularity = 8192" || echo OK)"
  ch "ALTER TABLE ci_finished_builds_tmp ATTACH PARTITION 2026 FROM ci_finished_builds"
  echo "tmp row count after ATTACH: $(ch 'SELECT count() FROM ci_finished_builds_tmp')"
}

for port in 18249 18231; do setup_and_test "$port"; done

docker rm -f ch249 ch2311

Option C — RSpec against the local GDK ClickHouse (modern CH 25/26)

To confirm the fix doesn't regress on modern ClickHouse (the only version master's CI matrix covers):

bundle exec rspec spec/db/click_house/post_migrate/20260108073937_alter_ci_finished_builds_engine_with_version_spec.rb

Expected: 8 examples, 0 failures. Verified.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Narendran

Merge request reports