fixes

Author: DatGuyJonathan

apps/framework-cli-e2e/test/cluster.test.ts

@@ -13,6 +13,7 @@
  * 4. Mixed environments (some tables with cluster, some without) work correctly
  * 5. Both TypeScript and Python SDKs support cluster configuration
  * 6. ReplicatedMergeTree with explicit keeper_path/replica_name (no cluster) works correctly
+ * 7. ReplicatedMergeTree with auto-injected params (ClickHouse Cloud mode) works correctly
  */
 
 import { spawn, ChildProcess } from "child_process";
@@ -320,7 +321,7 @@ const createClusterTestSuite = (config: ClusterTestConfig) => {
       try {
         const result = await client.query({
           query:
-            "SELECT name FROM system.tables WHERE database = 'local' AND name IN ('TableA', 'TableB', 'TableC', 'TableD') ORDER BY name",
+            "SELECT name FROM system.tables WHERE database = 'local' AND name IN ('TableA', 'TableB', 'TableC', 'TableD', 'TableE') ORDER BY name",
           format: "JSONEachRow",
         });
 
@@ -331,6 +332,7 @@ const createClusterTestSuite = (config: ClusterTestConfig) => {
         expect(tableNames).to.include("TableB");
         expect(tableNames).to.include("TableC");
         expect(tableNames).to.include("TableD");
+        expect(tableNames).to.include("TableE");
       } finally {
         await client.close();
       }
@@ -353,6 +355,7 @@ const createClusterTestSuite = (config: ClusterTestConfig) => {
         { name: "TableB", cluster: "cluster_b" },
         { name: "TableC", cluster: null },
         { name: "TableD", cluster: null },
+        { name: "TableE", cluster: null },
       ]);
     });
 
@@ -365,6 +368,7 @@ const createClusterTestSuite = (config: ClusterTestConfig) => {
       await verifyTableExists("TableB");
       await verifyTableExists("TableC");
       await verifyTableExists("TableD");
+      await verifyTableExists("TableE");
     });
 
     it("should create TableD with explicit keeper args and no cluster", async function () {
@@ -401,6 +405,39 @@ const createClusterTestSuite = (config: ClusterTestConfig) => {
         await client.close();
       }
     });
+
+    it("should create TableE with auto-injected params (ClickHouse Cloud mode)", async function () {
+      this.timeout(TIMEOUTS.SCHEMA_VALIDATION_MS);
+
+      // Verify TableE was created with ReplicatedMergeTree and auto-injected params
+      const client = createClient({
+        url: CLICKHOUSE_CONFIG.url,
+        username: CLICKHOUSE_CONFIG.username,
+        password: CLICKHOUSE_CONFIG.password,
+        database: CLICKHOUSE_CONFIG.database,
+      });
+
+      try {
+        const result = await client.query({
+          query: "SHOW CREATE TABLE local.TableE",
+          format: "JSONEachRow",
+        });
+
+        const data = await result.json<{ statement: string }>();
+        const createStatement = data[0].statement;
+
+        console.log(`TableE CREATE statement: ${createStatement}`);
+
+        // Verify it's ReplicatedMergeTree
+        expect(createStatement).to.include("ReplicatedMergeTree");
+        // Verify it has auto-injected params (Moose injects these in dev mode)
+        expect(createStatement).to.match(/ReplicatedMergeTree\(/);
+        // Verify it does NOT have ON CLUSTER (no cluster specified)
+        expect(createStatement).to.not.include("ON CLUSTER");
+      } finally {
+        await client.close();
+      }
+    });
   });
 };
 

apps/framework-cli/src/cli/display/infrastructure.rs

@@ -236,6 +236,11 @@ fn format_table_display(
         details.push(format!("Order by: {}", table.order_by));
     }
 
+    // Cluster section (if present)
+    if let Some(ref cluster) = table.cluster_name {
+        details.push(format!("Cluster: {}", cluster));
+    }
+
     // Engine section (if present)
     if let Some(ref engine) = table.engine {
         details.push(format!("Engine: {}", Into::<String>::into(engine.clone())));

apps/framework-cli/src/framework/core/infrastructure/table.rs

@@ -511,6 +511,10 @@ impl Table {
                     })
             });
 
+        // Normalize auto-injected replication params so they match user code
+        // This prevents spurious diffs when comparing tables from inframap with code
+        let engine = engine.map(|e| e.normalize_auto_injected_params(&proto.name));
+
         // Engine settings are now handled via table_settings field
 
         let fallback = || -> OrderBy {

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

@@ -1749,6 +1749,9 @@ impl InfrastructureMap {
                         // Detect engine change (e.g., MergeTree -> ReplacingMergeTree)
                         let engine_changed = table.engine != target_table.engine;
 
+                        // Detect cluster name change
+                        let cluster_changed = table.cluster_name != target_table.cluster_name;
+
                         let order_by_change = if order_by_changed {
                             OrderByChange {
                                 before: table.order_by.clone(),
@@ -1793,6 +1796,7 @@ impl InfrastructureMap {
                             || order_by_changed
                             || engine_changed
                             || indexes_changed
+                            || cluster_changed
                         {
                             // Use the strategy to determine the appropriate changes
                             let strategy_changes = strategy.diff_table_update(

apps/framework-cli/src/infrastructure/olap/clickhouse.rs

@@ -1700,6 +1700,12 @@ impl OlapOperations for ConfiguredDBClient {
                 debug!("Could not extract engine from CREATE TABLE query, falling back to system.tables engine column");
                 engine.as_str().try_into().ok()
             };
+
+            // Normalize auto-injected replication params so they match user code
+            // This prevents spurious diffs when comparing tables from the DB with code
+            let engine_parsed = engine_parsed
+                .map(|e: ClickhouseEngine| e.normalize_auto_injected_params(&table_name));
+
             let engine_params_hash = engine_parsed
                 .as_ref()
                 .map(|e: &ClickhouseEngine| e.non_alterable_params_hash());

apps/framework-cli/src/infrastructure/olap/clickhouse/diff_strategy.rs

@@ -50,49 +50,6 @@ fn format_database_change_error(table_name: &str, before_db: &str, after_db: &st
     )
 }
 
-/// Generates a formatted error message for cluster field changes.
-///
-/// This function creates a user-friendly error message explaining that cluster field
-/// changes require manual intervention to prevent data loss. Cluster changes are even
-/// more critical than database changes because they affect replication topology.
-///
-/// # Arguments
-/// * `table_name` - The name of the table being changed
-/// * `before_cluster` - The original cluster name (or "<none>" if None)
-/// * `after_cluster` - The new cluster name (or "<none>" if None)
-///
-/// # Returns
-/// A formatted string with migration instructions
-fn format_cluster_change_error(
-    table_name: &str,
-    before_cluster: &str,
-    after_cluster: &str,
-) -> String {
-    format!(
-        "\n\n\
-        ERROR: Cluster field change detected for table '{}'\n\
-        \n\
-        The cluster field changed from '{}' to '{}'\n\
-        \n\
-        Changing the cluster field is a destructive operation that requires\n\
-        manual intervention to ensure data safety and proper replication.\n\
-        \n\
-        The cluster name is embedded in the table's replication path and cannot\n\
-        be changed without recreating the table. This would cause data loss.\n\
-        \n\
-        To migrate this table to a different cluster:\n\
-        \n\
-        1. Export your existing data\n\
-        2. Delete the old table definition from your code\n\
-        3. Create a new table definition with the target cluster\n\
-        4. Re-import your data if needed\n\
-        \n\
-        This ensures you maintain control over data migration and prevents\n\
-        accidental data loss.\n",
-        table_name, before_cluster, after_cluster
-    )
-}
-
 /// ClickHouse-specific table diff strategy
 ///
 /// ClickHouse has several limitations that require drop+create operations instead of ALTER:
@@ -348,25 +305,15 @@ impl TableDiffStrategy for ClickHouseTableDiffStrategy {
         }
 
         // Check if cluster has changed
-        // Cluster name cannot be changed after table creation for replicated tables
-        // as it's embedded in the replication path
-        let cluster_changed = before.cluster_name != after.cluster_name;
-
-        if cluster_changed {
-            let before_cluster = before.cluster_name.as_deref().unwrap_or("<none>");
-            let after_cluster = after.cluster_name.as_deref().unwrap_or("<none>");
-
-            let error_message =
-                format_cluster_change_error(&before.name, before_cluster, after_cluster);
-
-            log::error!("{}", error_message);
-
-            return vec![OlapChange::Table(TableChange::ValidationError {
-                table_name: before.name.clone(),
-                message: error_message,
-                before: Box::new(before.clone()),
-                after: Box::new(after.clone()),
-            })];
+        if before.cluster_name != after.cluster_name {
+            log::warn!(
+                "ClickHouse: cluster changed for table '{}' (from {:?} to {:?}), requiring drop+create",
+                before.name, before.cluster_name, after.cluster_name
+            );
+            return vec![
+                OlapChange::Table(TableChange::Removed(before.clone())),
+                OlapChange::Table(TableChange::Added(after.clone())),
+            ];
         }
 
         // Check if PARTITION BY has changed
@@ -1337,27 +1284,16 @@ mod tests {
 
         let changes = strategy.diff_table_update(&before, &after, vec![], order_by_change, "local");
 
-        // Should return exactly one ValidationError
-        assert_eq!(changes.len(), 1);
+        // Should return DROP + CREATE (like ORDER BY changes)
+        assert_eq!(changes.len(), 2);
         assert!(matches!(
             changes[0],
-            OlapChange::Table(TableChange::ValidationError { .. })
+            OlapChange::Table(TableChange::Removed(_))
+        ));
+        assert!(matches!(
+            changes[1],
+            OlapChange::Table(TableChange::Added(_))
         ));
-
-        // Check the error message contains expected information
-        if let OlapChange::Table(TableChange::ValidationError {
-            table_name,
-            message,
-            ..
-        }) = &changes[0]
-        {
-            assert_eq!(table_name, "test");
-            assert!(message.contains("<none>"));
-            assert!(message.contains("test_cluster"));
-            assert!(message.contains("manual intervention"));
-        } else {
-            panic!("Expected ValidationError variant");
-        }
     }
 
     #[test]
@@ -1378,27 +1314,16 @@ mod tests {
 
         let changes = strategy.diff_table_update(&before, &after, vec![], order_by_change, "local");
 
-        // Should return exactly one ValidationError
-        assert_eq!(changes.len(), 1);
+        // Should return DROP + CREATE (like ORDER BY changes)
+        assert_eq!(changes.len(), 2);
         assert!(matches!(
             changes[0],
-            OlapChange::Table(TableChange::ValidationError { .. })
+            OlapChange::Table(TableChange::Removed(_))
+        ));
+        assert!(matches!(
+            changes[1],
+            OlapChange::Table(TableChange::Added(_))
         ));
-
-        // Check the error message contains expected information
-        if let OlapChange::Table(TableChange::ValidationError {
-            table_name,
-            message,
-            ..
-        }) = &changes[0]
-        {
-            assert_eq!(table_name, "test");
-            assert!(message.contains("test_cluster"));
-            assert!(message.contains("<none>"));
-            assert!(message.contains("manual intervention"));
-        } else {
-            panic!("Expected ValidationError variant");
-        }
     }
 
     #[test]
@@ -1419,27 +1344,16 @@ mod tests {
 
         let changes = strategy.diff_table_update(&before, &after, vec![], order_by_change, "local");
 
-        // Should return exactly one ValidationError
-        assert_eq!(changes.len(), 1);
+        // Should return DROP + CREATE (like ORDER BY changes)
+        assert_eq!(changes.len(), 2);
         assert!(matches!(
             changes[0],
-            OlapChange::Table(TableChange::ValidationError { .. })
+            OlapChange::Table(TableChange::Removed(_))
+        ));
+        assert!(matches!(
+            changes[1],
+            OlapChange::Table(TableChange::Added(_))
         ));
-
-        // Check the error message contains expected information
-        if let OlapChange::Table(TableChange::ValidationError {
-            table_name,
-            message,
-            ..
-        }) = &changes[0]
-        {
-            assert_eq!(table_name, "test");
-            assert!(message.contains("cluster_a"));
-            assert!(message.contains("cluster_b"));
-            assert!(message.contains("replication path"));
-        } else {
-            panic!("Expected ValidationError variant");
-        }
     }
 
     #[test]