[mday-io]

Description

Why

Five ClickHouse overwrite tests compared generated SQL with expected statements without asserting the result, allowing incorrect replacement SQL to pass. Partition actions also varied with Python set order.

How

Restore the assertions across full, time-range, key-based, and partition-based overwrites. Give staging and affected-partition tables distinct test names, and correct DISTINCT and cleanup expectations. Sort replacement and drop partition IDs when building the ALTER statement so tests can assert complete SQL call lists and logs show stable action order. Partition selection and statement count stay the same.

Test Plan

• All 68 ClickHouse unit cases passed.
• Six overwrite cases passed with PYTHONHASHSEED=1 and 42; before sorting, the three partitioned cases failed on action order under both seeds.
• Earlier mutation checks rejected wrong exchange targets, inverted retained-row predicates, and skipped partition replacements.
• make style passed.
• make fast-test passed: 2,895 tests, 13 skipped across all required batches; four xdist workers.
• Local results above are from October 21; the published patch matches that validated revision.

Checklist

• I have run make style and fixed any issues
• I have added tests for my changes (if applicable)
• All existing tests pass (make fast-test)
• My commits are signed off (git commit -s) per the DCO

Summary by CodeRabbit

• Bug Fixes
  • ClickHouse partition replacement and removal actions now run in a consistent order, making partition overwrite operations more predictable across runs. This change affects the ordering of partition actions; it does not add or remove user-facing capabilities or change the overall overwrite workflow.

[StuffbyYuki]

An optional:

Instead of the prefix/sort/split checks in the tests, perhaps consider making the generated ALTER deterministic at the source. In sqlmesh/core/engine_adapter/clickhouse.py, _build_alter_partition_exp loops over partitions_to_replace and partitions_to_drop, which are Python sets, so the action order can change between runs. Sorting them fixes that:

for partition in sorted(partitions_to_replace):
    ...
for partition in sorted(partitions_to_drop):
    ...

This only changes the order of actions within the single ALTER statement. It doesn't change which partitions are replaced or dropped, or how many statements are executed. As a side benefit, the SQL in logs and ClickHouse query history becomes stable.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: e4557fbc-2441-4f1d-832c-5da9665d0a80

Commits

Reviewing files that changed from the base of the PR and between 02e6551 and a2d027a.

Files selected for processing: 2

• sqlmesh/core/engine_adapter/clickhouse.py
• tests/core/engine_adapter/test_clickhouse.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

ClickHouse partition replacement and drop actions now use sorted partition IDs. Insert-overwrite tests update their expected SQL for affected-partition tables and distinct ID selection.

Changes: ClickHouse partition overwrite

Layer / File(s)	Summary
Partition action ordering and overwrite expectations sqlmesh/core/engine_adapter/clickhouse.py, tests/core/engine_adapter/test_clickhouse.py	Partition replacement and drop actions now use sorted partition IDs. Tests update partitioned and dynamic-key overwrite expectations for the affected_partitions table, distinct ID selection, and replacement order.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Possibly related issues

• SQLMesh/sqlmesh#6171: The changes cover partition replacement SQL and action ordering, but do not address the issue’s insert_quorum or replica synchronization concerns.

Suggested reviewers: treysp

Merge Risk: Minimal · up to a2d02

This change makes partition actions deterministic and strengthens overwrite SQL coverage without introducing a demonstrated merge-blocking risk.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: adding assertions for ClickHouse overwrite SQL.
Description check		The description includes the required Description, Test Plan, and Checklist sections. It explains the changes, testing performed, and completed checklist items.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts