bmuraykhi opened a new pull request, #45739: URL: https://github.com/apache/superset/pull/45739
### SUMMARY #44715 renames a SELECT alias that would shadow a source column by appending `__`. This applies on engines with `select_alias_shadows_source_column`, which is ClickHouse. The series-limit subquery already names its dimension columns `<label>__`, and the outer query joins on that name unqualified: `ON <expr> = <label>__`. When an outer dimension alias gets renamed, it becomes that same `<label>__`. ClickHouse resolves the identifier in `ON` to the outer SELECT alias first, so the join condition compares the expression with itself. Every row then matches all N subquery rows. Values come out multiplied by the series limit, and the limit stops filtering any series. The query still succeeds, so the results are silently wrong. Fixes #45738. The fix keeps the renamed alias unique across the whole query. `rename_shadowing_aliases` collects every alias already used by any SELECT in the statement, plus the dataset's column names. It then renames to `<label>__`, `<label>___` and so on, taking the first name that is free. Picking the alias moved into a small `_unused_alias` helper so the method stays within the C901 complexity limit. Output column names are unchanged, since `labels_expected` still restores the original labels. Another option was to qualify the `ON` column with the subquery alias (`series_limit.<label>__`). That would also fix it, but it changes the generated SQL for every engine. This change stays within the code path #44715 introduced and only affects engines that opt into alias renaming. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF ClickHouse 25.3, Superset 7.0.0rc2. A line chart of `SUM(amount)` by the calculated column `app` (`upper(label)`), with series limit 3 and sorted by `SUM(amount)` descending. Each label has 6 rows per hour, so the true hourly values are L9 = 60, L8 = 54, L7 = 48. **Before:** all 10 series are drawn, each at 3× its value (L9 = 180), in 170 rows. <img width="2880" height="2000" alt="before" src="https://github.com/user-attachments/assets/46061cff-bcb1-47e6-9ea7-de1e245ae26c"; /> **After:** only the top 3 series, with correct values, in 51 rows. <img width="2880" height="2000" alt="after" src="https://github.com/user-attachments/assets/1d53eccc-1d2a-4fb2-b8ff-bc667af45b1e"; /> SQL Superset generates on ClickHouse for a chart with x-axis `ts`, dimension `app` (calculated column `upper(label)`), metric `sum(amount)`, series limit 3: Before: ```sql SELECT toStartOfDay(toDateTime(`ts`)) AS `ts__`, (upper(label)) AS `app__`, sum(amount) AS `total` FROM `default`.`t` INNER JOIN (SELECT (upper(label)) AS `app__`, sum(amount) AS `mme_inner__` ... LIMIT 3) AS `series_limit` ON (upper(label)) = `app__` ... ``` The result has all 10 series, each value ×3. After: ```sql SELECT toStartOfDay(toDateTime(`ts`)) AS `ts__`, (upper(label)) AS `app___`, sum(amount) AS `total` FROM `default`.`t` INNER JOIN (SELECT (upper(label)) AS `app__`, sum(amount) AS `mme_inner__` ... LIMIT 3) AS `series_limit` ON (upper(label)) = `app__` ... ``` The result is the top 3 series with correct values. ### TESTING INSTRUCTIONS - **Unit test:** `pytest tests/unit_tests/models/select_alias_shadowing_test.py`. The new `test_renamed_alias_does_not_collide_with_series_limit_alias` fails on `master` and passes with this change. - **Against ClickHouse 25.3:** 1. Create `t (ts DateTime, label String, amount Float64)` and insert `numbers(1000)` with `label = concat('l', toString(number % 10))`, `amount = (number % 10) + 1`. 2. Add a dataset on `t` with a calculated column `app` = `upper(label)`. 3. Build a bar chart with x-axis `ts`, dimension `app`, metric `SUM(amount)`, series limit 3. 4. Before this change the chart shows 10 series totalling 16500. With it, the chart shows `L9`/`L8`/`L7` totalling 2700, which is the correct result. - **ClickHouse versions:** the fixed SQL was checked on ClickHouse 23.8, 24.3, 24.8, 25.3, 25.8, 26.3 and 26.10, with both the old and new analyzers. Without the fix, 23.8-24.8 fail with `Code: 403 Cannot get JOIN keys from JOIN ON section`, and 25.3-26.10 return the inflated result. ### ADDITIONAL INFORMATION - [x] Has associated issue: Fixes #45738 - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API The regression ships in `7.0.0rc2`, so this is a candidate for the 7.0 release branch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
