Skip to content

Commit c974f4c

Browse files
authored
normalize both infra map in dev/prod mode (#3056)
to be the same as plan command <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Normalize both reconciled and target infrastructure maps before diffing, and refactor InfrastructureMap::normalize to mutate tables in place. > > - **Planning**: > - Normalize `reconciled_map` and `target_infra_map` before diffing to align behavior with remote plan and ensure consistent comparisons. > - **Infrastructure Map**: > - Refactor `InfrastructureMap::normalize` to mutate tables in-place via `values_mut()`. > - Applies ORDER BY fallback when empty and sets Array columns to `required=true` without rebuilding the map. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit c27302b. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 9748d76 commit c974f4c

File tree

2 files changed

+18
-20
lines changed

2 files changed

+18
-20
lines changed

apps/framework-cli/src/framework/core/infrastructure_map.rs

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2526,28 +2526,22 @@ impl InfrastructureMap {
25262526
pub fn normalize(mut self) -> Self {
25272527
use crate::framework::core::infrastructure::table::ColumnType;
25282528

2529-
self.tables = self
2530-
.tables
2531-
.into_iter()
2532-
.map(|(id, mut table)| {
2533-
// Fall back to primary key columns if order_by is empty for MergeTree engines
2534-
// This ensures backward compatibility when order_by isn't explicitly set
2535-
// We only do this for MergeTree family to avoid breaking S3 tables
2536-
if table.order_by.is_empty() {
2537-
table.order_by = table.order_by_with_fallback();
2538-
}
2529+
self.tables.values_mut().for_each(|table| {
2530+
// Fall back to primary key columns if order_by is empty for MergeTree engines
2531+
// This ensures backward compatibility when order_by isn't explicitly set
2532+
// We only do this for MergeTree family to avoid breaking S3 tables
2533+
if table.order_by.is_empty() {
2534+
table.order_by = table.order_by_with_fallback();
2535+
}
25392536

2540-
// Normalize columns: ClickHouse doesn't support Nullable(Array(...))
2541-
// Arrays must always be NOT NULL (required=true)
2542-
for col in &mut table.columns {
2543-
if matches!(col.data_type, ColumnType::Array { .. }) {
2544-
col.required = true;
2545-
}
2537+
// Normalize columns: ClickHouse doesn't support Nullable(Array(...))
2538+
// Arrays must always be NOT NULL (required=true)
2539+
for col in &mut table.columns {
2540+
if matches!(col.data_type, ColumnType::Array { .. }) {
2541+
col.required = true;
25462542
}
2547-
2548-
(id, table)
2549-
})
2550-
.collect();
2543+
}
2544+
});
25512545

25522546
self
25532547
}

apps/framework-cli/src/framework/core/plan.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,10 @@ pub async fn plan_changes(
409409
.unwrap_or("Could not serialize reconciled infrastructure map".to_string())
410410
);
411411

412+
// Normalize both infra maps, same as remote_plan
413+
let reconciled_map = reconciled_map.normalize();
414+
let target_infra_map = target_infra_map.normalize();
415+
412416
// Use the reconciled map for diffing with ClickHouse-specific strategy
413417
// Pass ignore_ops so the diff can normalize tables internally for comparison
414418
// while using original tables for the actual change operations

0 commit comments

Comments
 (0)