Skip to content

Commit fbefc26

Browse files
cleanup
1 parent a838ea5 commit fbefc26

File tree

3 files changed

+0
-174
lines changed

3 files changed

+0
-174
lines changed

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -511,10 +511,6 @@ impl Table {
511511
})
512512
});
513513

514-
// Normalize auto-injected replication params so they match user code
515-
// This prevents spurious diffs when comparing tables from inframap with code
516-
let engine = engine.map(|e| e.normalize_auto_injected_params(&proto.name));
517-
518514
// Engine settings are now handled via table_settings field
519515

520516
let fallback = || -> OrderBy {

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1701,11 +1701,6 @@ impl OlapOperations for ConfiguredDBClient {
17011701
engine.as_str().try_into().ok()
17021702
};
17031703

1704-
// Normalize auto-injected replication params so they match user code
1705-
// This prevents spurious diffs when comparing tables from the DB with code
1706-
let engine_parsed = engine_parsed
1707-
.map(|e: ClickhouseEngine| e.normalize_auto_injected_params(&table_name));
1708-
17091704
let engine_params_hash = engine_parsed
17101705
.as_ref()
17111706
.map(|e: &ClickhouseEngine| e.non_alterable_params_hash());

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

Lines changed: 0 additions & 165 deletions
Original file line numberDiff line numberDiff line change
@@ -885,93 +885,6 @@ impl ClickhouseEngine {
885885
self.is_merge_tree_family() || matches!(self, ClickhouseEngine::S3 { .. })
886886
}
887887

888-
/// Normalize auto-injected replication parameters back to None
889-
///
890-
/// When Moose auto-injects replication params in dev mode or with clusters,
891-
/// it uses a standard pattern. When reading tables back from ClickHouse,
892-
/// we normalize these back to None so they match the user's code (which
893-
/// doesn't specify params) and don't show spurious diffs.
894-
///
895-
/// Auto-injected pattern:
896-
/// - keeper_path: `/clickhouse/tables/{database}/{shard}/{table_name}`
897-
/// - replica_name: `{replica}`
898-
pub fn normalize_auto_injected_params(&self, table_name: &str) -> Self {
899-
let expected_keeper_path =
900-
format!("/clickhouse/tables/{{database}}/{{shard}}/{}", table_name);
901-
let expected_replica_name = "{replica}";
902-
903-
match self {
904-
ClickhouseEngine::ReplicatedMergeTree {
905-
keeper_path,
906-
replica_name,
907-
} => {
908-
if keeper_path.as_deref() == Some(&expected_keeper_path)
909-
&& replica_name.as_deref() == Some(expected_replica_name)
910-
{
911-
ClickhouseEngine::ReplicatedMergeTree {
912-
keeper_path: None,
913-
replica_name: None,
914-
}
915-
} else {
916-
self.clone()
917-
}
918-
}
919-
ClickhouseEngine::ReplicatedReplacingMergeTree {
920-
keeper_path,
921-
replica_name,
922-
ver,
923-
is_deleted,
924-
} => {
925-
if keeper_path.as_deref() == Some(&expected_keeper_path)
926-
&& replica_name.as_deref() == Some(expected_replica_name)
927-
{
928-
ClickhouseEngine::ReplicatedReplacingMergeTree {
929-
keeper_path: None,
930-
replica_name: None,
931-
ver: ver.clone(),
932-
is_deleted: is_deleted.clone(),
933-
}
934-
} else {
935-
self.clone()
936-
}
937-
}
938-
ClickhouseEngine::ReplicatedAggregatingMergeTree {
939-
keeper_path,
940-
replica_name,
941-
} => {
942-
if keeper_path.as_deref() == Some(&expected_keeper_path)
943-
&& replica_name.as_deref() == Some(expected_replica_name)
944-
{
945-
ClickhouseEngine::ReplicatedAggregatingMergeTree {
946-
keeper_path: None,
947-
replica_name: None,
948-
}
949-
} else {
950-
self.clone()
951-
}
952-
}
953-
ClickhouseEngine::ReplicatedSummingMergeTree {
954-
keeper_path,
955-
replica_name,
956-
columns,
957-
} => {
958-
if keeper_path.as_deref() == Some(&expected_keeper_path)
959-
&& replica_name.as_deref() == Some(expected_replica_name)
960-
{
961-
ClickhouseEngine::ReplicatedSummingMergeTree {
962-
keeper_path: None,
963-
replica_name: None,
964-
columns: columns.clone(),
965-
}
966-
} else {
967-
self.clone()
968-
}
969-
}
970-
// Non-replicated engines don't need normalization
971-
_ => self.clone(),
972-
}
973-
}
974-
975888
/// Convert engine to string for proto storage (no sensitive data)
976889
pub fn to_proto_string(&self) -> String {
977890
match self {
@@ -4799,82 +4712,4 @@ ENGINE = S3Queue('s3://my-bucket/data/*.csv', NOSIGN, 'CSV')"#;
47994712
_ => panic!("Expected InvalidParameters error"),
48004713
}
48014714
}
4802-
4803-
#[test]
4804-
fn test_normalize_auto_injected_params_replicated_merge_tree() {
4805-
// Test auto-injected params are normalized to None
4806-
let engine = ClickhouseEngine::ReplicatedMergeTree {
4807-
keeper_path: Some("/clickhouse/tables/{database}/{shard}/MyTable".to_string()),
4808-
replica_name: Some("{replica}".to_string()),
4809-
};
4810-
4811-
let normalized = engine.normalize_auto_injected_params("MyTable");
4812-
4813-
assert!(matches!(
4814-
normalized,
4815-
ClickhouseEngine::ReplicatedMergeTree {
4816-
keeper_path: None,
4817-
replica_name: None,
4818-
}
4819-
));
4820-
}
4821-
4822-
#[test]
4823-
fn test_normalize_auto_injected_params_custom_params_unchanged() {
4824-
// Test custom params are NOT normalized
4825-
let engine = ClickhouseEngine::ReplicatedMergeTree {
4826-
keeper_path: Some("/custom/path/MyTable".to_string()),
4827-
replica_name: Some("custom_replica".to_string()),
4828-
};
4829-
4830-
let normalized = engine.normalize_auto_injected_params("MyTable");
4831-
4832-
// Should remain unchanged
4833-
match normalized {
4834-
ClickhouseEngine::ReplicatedMergeTree {
4835-
keeper_path,
4836-
replica_name,
4837-
} => {
4838-
assert_eq!(keeper_path, Some("/custom/path/MyTable".to_string()));
4839-
assert_eq!(replica_name, Some("custom_replica".to_string()));
4840-
}
4841-
_ => panic!("Expected ReplicatedMergeTree"),
4842-
}
4843-
}
4844-
4845-
#[test]
4846-
fn test_normalize_auto_injected_params_replicated_replacing_merge_tree() {
4847-
// Test ReplicatedReplacingMergeTree with auto-injected params
4848-
let engine = ClickhouseEngine::ReplicatedReplacingMergeTree {
4849-
keeper_path: Some("/clickhouse/tables/{database}/{shard}/MyTable".to_string()),
4850-
replica_name: Some("{replica}".to_string()),
4851-
ver: Some("version_col".to_string()),
4852-
is_deleted: None,
4853-
};
4854-
4855-
let normalized = engine.normalize_auto_injected_params("MyTable");
4856-
4857-
match normalized {
4858-
ClickhouseEngine::ReplicatedReplacingMergeTree {
4859-
keeper_path,
4860-
replica_name,
4861-
ver,
4862-
is_deleted,
4863-
} => {
4864-
assert_eq!(keeper_path, None);
4865-
assert_eq!(replica_name, None);
4866-
assert_eq!(ver, Some("version_col".to_string()));
4867-
assert_eq!(is_deleted, None);
4868-
}
4869-
_ => panic!("Expected ReplicatedReplacingMergeTree"),
4870-
}
4871-
}
4872-
4873-
#[test]
4874-
fn test_normalize_auto_injected_params_non_replicated_unchanged() {
4875-
// Test non-replicated engines are unchanged
4876-
let engine = ClickhouseEngine::MergeTree;
4877-
let normalized = engine.normalize_auto_injected_params("MyTable");
4878-
assert!(matches!(normalized, ClickhouseEngine::MergeTree));
4879-
}
48804715
}

0 commit comments

Comments
 (0)