Skip to content

fix(clickhouse): renumber task_kind migration 029 → 031#3631

Merged
ericallam merged 1 commit into
mainfrom
fix/renumber-task-kind-clickhouse-migration
May 15, 2026
Merged

fix(clickhouse): renumber task_kind migration 029 → 031#3631
ericallam merged 1 commit into
mainfrom
fix/renumber-task-kind-clickhouse-migration

Conversation

@ericallam
Copy link
Copy Markdown
Member

@ericallam ericallam commented May 15, 2026

Summary

Renumber 029_add_task_kind_to_task_runs_v2.sql031_add_task_kind_to_task_runs_v2.sql to fix a deploy-blocking out-of-order migration, and make the DDL idempotent with ADD COLUMN IF NOT EXISTS / DROP COLUMN IF EXISTS.

Root cause

  • Migration 030_create_sessions_v1.sql landed on main on 2026-04-28 (PR feat: Sessions - bidirectional durable agent streams #3417) and was applied to test cloud ClickHouse on a subsequent deploy. Current goose version on test ClickHouse: 30.
  • Migration 029_add_task_kind_to_task_runs_v2.sql was authored later on 2026-05-10 as part of the Sessions primitive PR series (be1a6cf8).
  • The next test cloud deploy failed because goose strict-mode refused to apply a missing version before the current version:
goose run: error: found 1 missing migrations before current version 30:
  version 29: 029_add_task_kind_to_task_runs_v2.sql

Fix

  1. Rename to 031_* (next available number after 030). Goose now treats it as a new migration after 030 and applies it cleanly on test/prod where the column does not yet exist.
  2. Make the DDL idempotent (ADD COLUMN IF NOT EXISTS). The original 029 may have been applied in environments that ran goose with --allow-missing (e.g. some local dev databases) — those would have the column already, and the rename causes goose to see 031 as new and re-attempt the ADD. Idempotent DDL keeps that path safe. The Down mirrors with DROP COLUMN IF EXISTS.

Test plan

  • Test cloud deploy (after this lands) successfully runs the ClickHouse migration step
  • task_kind column shows up on trigger_dev.task_runs_v2 post-migration
  • Local environments that had previously applied 029 do not error on the next goose up

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 15, 2026

⚠️ No Changeset found

Latest commit: 775287c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 15, 2026

Review Change Stack

Walkthrough

This pull request adds a ClickHouse schema migration to introduce a task_kind column to the trigger_dev.task_runs_v2 table. The migration uses idempotent SQL operations with ADD COLUMN IF NOT EXISTS in the Up path and DROP COLUMN IF EXISTS in the Down path. The new column is defined as LowCardinality(String) with a default value of an empty string.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description provides clear context on the root cause, fix, and test plan, but does not follow the required template structure with explicit sections like Checklist, Testing, Changelog, and Screenshots. Restructure the description to match the template: add the Checklist section with required items (contributing guide, PR title convention, code testing), separate Testing section, and Changelog section summarizing the changes.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: renumbering a ClickHouse migration file from 029 to 031 to fix a deploy-blocking issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/renumber-task-kind-clickhouse-migration

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ericallam ericallam marked this pull request as ready for review May 15, 2026 16:58
@ericallam ericallam enabled auto-merge (squash) May 15, 2026 16:58
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

🐛 1 issue in files not directly in the diff

🐛 Renaming already-merged goose migration causes duplicate column application on deployed environments (internal-packages/clickhouse/schema/031_add_task_kind_to_task_runs_v2.sql:2-3)

Migration 029_add_task_kind_to_task_runs_v2.sql was already merged to main (in commit be1a6cf8d) and may have been applied in existing environments. Renaming it to 031_ means goose will treat it as a brand-new migration: the old version 029 is recorded as applied in goose's goose_db_version table, but the file no longer exists; meanwhile, version 031 appears as a new unapplied migration. Running goose up will attempt to execute ALTER TABLE trigger_dev.task_runs_v2 ADD COLUMN task_kind ... again, which will fail in ClickHouse because the column already exists (the DDL does not use IF NOT EXISTS). No other ADD COLUMN migration in the schema directory uses IF NOT EXISTS either, so there's no established pattern of idempotent column additions to fall back on.

View 1 additional finding in Devin Review.

Open in Devin Review

@ericallam ericallam disabled auto-merge May 15, 2026 17:02
Migration 030_create_sessions_v1.sql landed on main on 2026-04-28 (PR #3417)
and was applied to test/prod ClickHouse on subsequent deploys. Migration
029_add_task_kind_to_task_runs_v2.sql was authored later on 2026-05-10 as
part of the Sessions primitive PR, after 030 was already at version 30 in
the goose history.

The next deploy fails because goose strict-mode refuses to apply a missing
version 'before' current_version 30:

  goose run: error: found 1 missing migrations before current version 30:
    version 29: 029_add_task_kind_to_task_runs_v2.sql

Renumber to 031 so goose applies it as the next version after 030. SQL is
unchanged.
@ericallam ericallam force-pushed the fix/renumber-task-kind-clickhouse-migration branch from ed86704 to 775287c Compare May 15, 2026 17:02
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review

@ericallam ericallam enabled auto-merge (squash) May 15, 2026 17:05
@ericallam ericallam merged commit 032b5a1 into main May 15, 2026
39 checks passed
@ericallam ericallam deleted the fix/renumber-task-kind-clickhouse-migration branch May 15, 2026 17:17
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.

3 participants