Skip to content

Commit bfdae6c

Browse files
authored
allow ignoring partition by diff (#2946)
<!-- CURSOR_SUMMARY --> > [!NOTE] > Adds partition-by-aware table diffs and normalization-based ignoring (TTL/partition_by) wired through planning, admin/remote plan, and migration; updates APIs/types and tests accordingly. > > - **Core infra map/diffing**: > - Add `PartitionByChange`; include in `TableChange::Updated` and `TableDiffStrategy::diff_table_update`. > - Extend simple/table diffs to compute and carry `partition_by_change`. > - `diff_with_table_strategy(...)` now accepts `ignore_ops` and normalizes tables before comparison. > - **ClickHouse integration**: > - Add `IgnorableOperation::ModifyPartitionBy` and `normalize_table_for_diff` to strip ignored fields (table TTL, column TTL, partition_by). > - Update `diff_strategy` to take `partition_by_change` and trigger drop+create on partition-by changes; add tests. > - **Planning/CLI**: > - Pass `ignore_ops` from project config (prod) into diffs in `plan.rs` and admin `/admin/plan`. > - Client-side `calculate_plan_diff_local(...)` accepts `ignore_ops`; remote plan/migration paths updated accordingly. > - Drift detection (`migrate.rs`) strips metadata and ignored fields via normalization. > - Remove prior operation-level ignore filtering from `MigrationPlan` (and related tests). > - **Webserver**: > - Use `clickhouse::{create_client, check_ready}`; minor health/ready/reality-check refactors. > - **Tests**: > - Update signatures/usages across tests; add tests for normalization and partition-by behavior. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit ff98322. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 57bc05c commit bfdae6c

File tree

9 files changed

+605
-448
lines changed

9 files changed

+605
-448
lines changed

apps/framework-cli/src/cli/local_webserver.rs

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,11 @@ use crate::framework::versions::Version;
3939
use crate::metrics::Metrics;
4040
use crate::utilities::auth::{get_claims, validate_jwt};
4141

42-
use crate::infrastructure::stream::kafka;
43-
use crate::infrastructure::stream::kafka::models::ConfiguredProducer;
44-
4542
use crate::framework::core::infrastructure::topic::{KafkaSchemaKind, SchemaRegistryReference};
4643
use crate::framework::typescript::bin::CliMessage;
44+
use crate::infrastructure::olap::clickhouse;
45+
use crate::infrastructure::stream::kafka;
46+
use crate::infrastructure::stream::kafka::models::ConfiguredProducer;
4747
use crate::project::{JwtConfig, Project};
4848
use crate::utilities::docker::DockerClient;
4949
use bytes::Buf;
@@ -850,9 +850,7 @@ async fn health_route(
850850

851851
// Check ClickHouse connectivity only if OLAP is enabled
852852
if project.features.olap {
853-
let olap_client = crate::infrastructure::olap::clickhouse::create_client(
854-
project.clickhouse_config.clone(),
855-
);
853+
let olap_client = clickhouse::create_client(project.clickhouse_config.clone());
856854
match olap_client.client.query("SELECT 1").execute().await {
857855
Ok(_) => healthy.push("ClickHouse"),
858856
Err(e) => {
@@ -910,10 +908,8 @@ async fn ready_route(
910908

911909
// ClickHouse: run a small query using the configured client (ensures HTTP pool is ready)
912910
if project.features.olap {
913-
let ch = crate::infrastructure::olap::clickhouse::create_client(
914-
project.clickhouse_config.clone(),
915-
);
916-
match crate::infrastructure::olap::clickhouse::check_ready(&ch).await {
911+
let ch = clickhouse::create_client(project.clickhouse_config.clone());
912+
match clickhouse::check_ready(&ch).await {
917913
Ok(_) => healthy.push("ClickHouse"),
918914
Err(e) => {
919915
warn!("Ready check: ClickHouse not ready: {}", e);
@@ -999,9 +995,7 @@ async fn admin_reality_check_route(
999995
// Perform reality check (storage is guaranteed to be enabled at this point)
1000996
let discrepancies = {
1001997
// Create OLAP client and reality checker
1002-
let olap_client = crate::infrastructure::olap::clickhouse::create_client(
1003-
project.clickhouse_config.clone(),
1004-
);
998+
let olap_client = clickhouse::create_client(project.clickhouse_config.clone());
1005999
let reality_checker =
10061000
crate::framework::core::infra_reality_checker::InfraRealityChecker::new(olap_client);
10071001

@@ -3119,8 +3113,7 @@ async fn admin_integrate_changes_route(
31193113
}
31203114

31213115
// Get reality check
3122-
let olap_client =
3123-
crate::infrastructure::olap::clickhouse::create_client(project.clickhouse_config.clone());
3116+
let olap_client = clickhouse::create_client(project.clickhouse_config.clone());
31243117
let reality_checker =
31253118
crate::framework::core::infra_reality_checker::InfraRealityChecker::new(olap_client);
31263119

@@ -3352,13 +3345,18 @@ async fn admin_plan_route(
33523345

33533346
// Calculate the changes between the submitted infrastructure map and the current one
33543347
// Use ClickHouse-specific strategy for table diffing
3355-
let clickhouse_strategy =
3356-
crate::infrastructure::olap::clickhouse::diff_strategy::ClickHouseTableDiffStrategy;
3348+
let clickhouse_strategy = clickhouse::diff_strategy::ClickHouseTableDiffStrategy;
3349+
let ignore_ops: &[clickhouse::IgnorableOperation] = if project.is_production {
3350+
&project.migration_config.ignore_operations
3351+
} else {
3352+
&[]
3353+
};
33573354
let changes = current_infra_map.diff_with_table_strategy(
33583355
&plan_request.infra_map,
33593356
&clickhouse_strategy,
33603357
true,
33613358
project.is_production,
3359+
ignore_ops,
33623360
);
33633361

33643362
// Prepare the response

apps/framework-cli/src/cli/routines.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -862,18 +862,20 @@ pub(crate) async fn get_remote_inframap_protobuf(
862862
/// # Arguments
863863
/// * `current_map` - The current infrastructure map (from server)
864864
/// * `target_map` - The target infrastructure map (from local project)
865+
/// * `ignore_ops` - Operations to ignore during comparison (e.g., ModifyPartitionBy)
865866
///
866867
/// # Returns
867868
/// * `InfraChanges` - The calculated changes needed to go from current to target
868869
fn calculate_plan_diff_local(
869870
current_map: &InfrastructureMap,
870871
target_map: &InfrastructureMap,
872+
ignore_ops: &[crate::infrastructure::olap::clickhouse::IgnorableOperation],
871873
) -> crate::framework::core::infrastructure_map::InfraChanges {
872874
use crate::infrastructure::olap::clickhouse::diff_strategy::ClickHouseTableDiffStrategy;
873875

874876
let clickhouse_strategy = ClickHouseTableDiffStrategy;
875877
// planning about action on prod env, respect_life_cycle is true
876-
current_map.diff_with_table_strategy(target_map, &clickhouse_strategy, true, true)
878+
current_map.diff_with_table_strategy(target_map, &clickhouse_strategy, true, true, ignore_ops)
877879
}
878880

879881
/// Legacy implementation of remote_plan using the existing /admin/plan endpoint
@@ -1056,7 +1058,11 @@ pub async fn remote_plan(
10561058
};
10571059

10581060
// Calculate and display changes
1059-
let changes = calculate_plan_diff_local(&remote_infra_map, &local_infra_map);
1061+
let changes = calculate_plan_diff_local(
1062+
&remote_infra_map,
1063+
&local_infra_map,
1064+
&project.migration_config.ignore_operations,
1065+
);
10601066

10611067
display::show_message_wrapper(
10621068
MessageType::Success,
@@ -1149,7 +1155,11 @@ pub async fn remote_gen_migration(
11491155
}
11501156
};
11511157

1152-
let changes = calculate_plan_diff_local(&remote_infra_map, &local_infra_map);
1158+
let changes = calculate_plan_diff_local(
1159+
&remote_infra_map,
1160+
&local_infra_map,
1161+
&project.migration_config.ignore_operations,
1162+
);
11531163

11541164
display::show_message_wrapper(
11551165
MessageType::Success,
@@ -1159,12 +1169,9 @@ pub async fn remote_gen_migration(
11591169
},
11601170
);
11611171

1162-
let mut db_migration =
1172+
let db_migration =
11631173
MigrationPlan::from_infra_plan(&changes, &project.clickhouse_config.db_name)?;
11641174

1165-
// Filter out ignored operations based on project config
1166-
db_migration.filter_ignored_operations(&project.migration_config.ignore_operations);
1167-
11681175
Ok(MigrationPlanWithBeforeAfter {
11691176
remote_state: remote_infra_map,
11701177
local_infra_map,

apps/framework-cli/src/cli/routines/migrate.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,9 @@ fn strip_metadata_and_ignored_fields(
100100
let mut table = table.clone();
101101
table.metadata = None;
102102
// Also strip ignored fields
103-
let table =
104-
crate::framework::core::migration_plan::strip_ignored_fields(&table, ignore_ops);
103+
let table = crate::infrastructure::olap::clickhouse::normalize_table_for_diff(
104+
&table, ignore_ops,
105+
);
105106
(name.clone(), table)
106107
})
107108
.collect()

0 commit comments

Comments
 (0)