refactor(cubesql): Simplify aliases handling in Remapper

Author: mcheshkov

rust/cubesql/cubesql/src/compile/engine/df/wrapper.rs

@@ -366,32 +366,25 @@ impl Remapper {
 
     fn insert_new_alias(&mut self, original_column: &Column, new_alias: &String) {
         self.used_targets.insert(new_alias.clone());
-        self.remapping.insert(
+
+        let target = match &self.from_alias {
+            // When result of this remapper will be used with FROM alias, it should always be safe to use it in target
+            Some(from_alias) => Column {
+                name: new_alias.clone(),
+                relation: Some(from_alias.clone()),
+            },
+            None => Column::from_name(new_alias),
+        };
+
+        for source in [
+            // This is not exactly safe to use any unqualified name as a source
+            // But we rely here on following validation from DF:
+            // If any column name is ambiguous, then it would not be used as unqualified column expression
+            // And then this source column would not be used ever
             Column::from_name(&original_column.name),
-            Column::from_name(new_alias),
-        );
-        if let Some(from_alias) = &self.from_alias {
-            self.remapping.insert(
-                Column {
-                    name: original_column.name.clone(),
-                    relation: Some(from_alias.clone()),
-                },
-                Column {
-                    name: new_alias.clone(),
-                    relation: Some(from_alias.clone()),
-                },
-            );
-            if let Some(original_relation) = &original_column.relation {
-                if original_relation != from_alias {
-                    self.remapping.insert(
-                        original_column.clone(),
-                        Column {
-                            name: new_alias.clone(),
-                            relation: Some(from_alias.clone()),
-                        },
-                    );
-                }
-            }
+            original_column.clone(),
+        ] {
+            self.remapping.insert(source, target.clone());
         }
     }
 

rust/cubesql/cubesql/src/compile/test/test_wrapper.rs

@@ -807,7 +807,7 @@ async fn test_case_wrapper_alias_with_order() {
         .wrapped_sql
         .unwrap()
         .sql
-        .contains("ORDER BY \"case_when_a_cust\""));
+        .contains("ORDER BY \"a\".\"case_when_a_cust\""));
 
     let physical_plan = query_plan.as_physical_plan().await.unwrap();
     println!(
@@ -935,7 +935,7 @@ async fn test_case_wrapper_ungrouped_sorted_aliased() {
         .unwrap()
         .sql
         // TODO test without depend on column name
-        .contains("ORDER BY \"case_when"));
+        .contains("ORDER BY \"a\".\"case_when"));
 }
 
 #[tokio::test]