From b72f5769789c8ad698b1723805f8cb49e62d4e72 Mon Sep 17 00:00:00 2001 From: Alex Qyoun-ae <4062971+MazterQyou@users.noreply.github.com> Date: Fri, 3 Jul 2026 16:27:18 +0400 Subject: [PATCH] fix(cubesql): Keep distinct aliases for same-named columns across join sides Signed-off-by: Alex Qyoun-ae <4062971+MazterQyou@users.noreply.github.com> --- .../cubesql/src/compile/engine/df/wrapper.rs | 35 ++++++--- .../cubesql/src/compile/test/test_wrapper.rs | 73 +++++++++++++++++++ 2 files changed, 96 insertions(+), 12 deletions(-) diff --git a/rust/cubesql/cubesql/src/compile/engine/df/wrapper.rs b/rust/cubesql/cubesql/src/compile/engine/df/wrapper.rs index 63686b47d596d..3ac870302a7fc 100644 --- a/rust/cubesql/cubesql/src/compile/engine/df/wrapper.rs +++ b/rust/cubesql/cubesql/src/compile/engine/df/wrapper.rs @@ -575,18 +575,29 @@ impl Remapper { } if let Some(from_alias) = &self.from_alias { - // Always map the column under the new from_alias so plans above (which see - // this remapper's output through `from_alias`) can resolve it. When the - // source column already had a different relation, also keep a mapping under - // that original relation, so plans that still reference the inner qualifier - // continue to resolve. - self.remapping.insert( - Column { - name: original_column.name.clone(), - relation: Some(from_alias.clone()), - }, - target_column.clone(), - ); + // Map the column under the new from_alias so plans above (which see + // this remapper's output through `from_alias`) can resolve it. When join sides + // project colliding column names, the column actually qualified with `from_alias` + // owns this mapping; a column from another relation only fills a vacancy, so it + // cannot clobber the from-side column's mapping. When the source column has a + // different relation, also keep a mapping under that original relation, so plans + // that still reference the inner qualifier continue to resolve. + // + // NOTE: this relies on the from-side column being added before a colliding + // column from another relation. If the other side is added first, its vacancy + // fill maps `{from_alias}.{name}` to its own target, and a later add of the + // actual from-side column short-circuits on that entry in `add_column`/`add_expr`, + // returning the other side's alias. Callers iterate the from side's columns + // before joined sides' ones, preserving this invariant. + let from_alias_column = Column { + name: original_column.name.clone(), + relation: Some(from_alias.clone()), + }; + let original_is_from_side = original_column.relation.as_ref() == Some(from_alias); + if original_is_from_side || !self.remapping.contains_key(&from_alias_column) { + self.remapping + .insert(from_alias_column, target_column.clone()); + } if let Some(original_relation) = &original_column.relation { if original_relation != from_alias { self.remapping diff --git a/rust/cubesql/cubesql/src/compile/test/test_wrapper.rs b/rust/cubesql/cubesql/src/compile/test/test_wrapper.rs index d08f94d71c2af..2e41b770e086c 100644 --- a/rust/cubesql/cubesql/src/compile/test/test_wrapper.rs +++ b/rust/cubesql/cubesql/src/compile/test/test_wrapper.rs @@ -1447,6 +1447,79 @@ async fn test_grouped_join_wrapper_full_outer_cross_cube() { ); } +/// LEFT JOIN between grouped CTEs from two *different* cubes (no join relationship +/// defined between them), joined on multiple dimensions sharing the same output names. +#[tokio::test] +async fn test_grouped_join_wrapper_left_cross_cube_multi_condition() { + if !Rewriter::sql_push_down_enabled() { + return; + } + init_testing_logger(); + + let query_plan = convert_select_to_query_plan( + // language=PostgreSQL + r#" + WITH m1 AS ( + SELECT + customer_gender, + notes, + sum(sumPrice) AS sum_price + FROM KibanaSampleDataEcommerce + WHERE + has_subscription = true + AND customer_gender IS NOT NULL + AND order_date BETWEEN '2024-01-01T00:00:00.000' AND '2024-01-31T23:59:59.999' + GROUP BY 1, 2 + ), + m2 AS ( + SELECT + dim0 AS customer_gender, + dim1 AS notes, + MEASURE(WideCube.count) AS cnt + FROM WideCube + WHERE + dim2 IN ('female') + AND dim3 IS NOT NULL + GROUP BY 1, 2 + ) + SELECT + COALESCE(m1.customer_gender, m2.customer_gender) AS customer_gender, + COALESCE(m1.notes, m2.notes) AS notes, + m1.sum_price, + m2.cnt + FROM m1 + LEFT JOIN m2 ON m1.customer_gender = m2.customer_gender AND m1.notes = m2.notes + "# + .to_string(), + DatabaseProtocol::PostgreSQL, + ) + .await; + + // Whole query must be pushed down to a single wrapped SQL with a LEFT JOIN + let _physical_plan = query_plan.as_physical_plan().await.unwrap(); + + let logical_plan = query_plan.as_logical_plan(); + let sql = logical_plan.find_cube_scan_wrapped_sql().wrapped_sql.sql; + assert!( + sql.contains("LEFT JOIN"), + "wrapped SQL is missing LEFT JOIN:\n{}", + sql + ); + // Same-named columns from both join sides must keep distinct aliases in the outer + // projection: COALESCE over both sides must not reference the same (right-side) column + // twice, or LEFT JOIN rows without a match would coalesce to NULL. + assert!( + sql.contains(r#"COALESCE("m1"."customer_gender", "m1"."customer_gender_1")"#), + "left side of COALESCE was remapped to the right-side column:\n{}", + sql + ); + assert!( + sql.contains(r#"COALESCE("m1"."notes", "m1"."notes_1")"#), + "left side of COALESCE was remapped to the right-side column:\n{}", + sql + ); +} + /// Regression test for smoke test "select __user and literal grouped under wrapper". /// Inner CTE has unaliased DATE_TRUNC expressions; outer query references those columns /// through the SubqueryAlias qualifier (`cube_scan_subq`). The CTE-level Remapper must