aminghadersohi opened a new pull request, #44715: URL: https://github.com/apache/superset/pull/44715
### SUMMARY ClickHouse resolves an identifier to a SELECT alias before a source column of the same name, and it does so in every clause: WHERE, GROUP BY, HAVING and ORDER BY. Explore labels a time-grain x-axis after its column, so a chart query on ClickHouse selects `toStartOfDay(toDateTime(ts)) AS ts`. That breaks in two ways, depending on the driver: - **clickhouse-sqlalchemy** (`clickhouse://`, `clickhouse+native://`) repeats the expression in `GROUP BY toStartOfDay(toDateTime(ts))`. Its `ts` resolves to the alias, so the key is truncated twice and ClickHouse rejects the query: `Code: 215 ... Column 'ts' is not under aggregate function and not in GROUP BY keys`. This happens on every time grain. - **clickhouse-connect** (`clickhousedb://`) groups by the label, so the query runs. But the time-range filter `WHERE ts >= '2024-01-01 12:00:00'` also resolves to the alias and compares the truncated bucket instead of the row's timestamp. Rows after the range start on the first bucket are **silently dropped**. With a mid-day range start, 7 of the 18 grains we tested returned wrong counts. The existing `allows_alias_to_source_column = False` switch (Presto, Trino) does not cover this. It renames an alias only when the alias appears inside an ORDER BY expression, so a chart sorted by its metric still fails, and the WHERE is never considered. This PR: - adds a `select_alias_shadows_source_column` engine-spec flag (default `False`) and sets it on `ClickHouseBaseEngineSpec`, so both ClickHouse specs inherit it; - adds `ExploreMixin.rename_shadowing_aliases`, which runs on the final chart query for engines that set the flag, subqueries included. It renames an alias to `<name>__` when the alias names a column of the datasource, or a column its own expression reads, and its expression is not simply that column. `labels_expected` already maps the result columns back to the Explore labels; - in `query()`, applies `labels_expected` to an empty result too. Before, only non-empty frames were relabelled, so an empty result kept the engine's (renamed) column names. This also applies to the existing Presto/Trino ORDER BY rename. Engines that don't set the flag are unchanged. Apache Kylin appears to have the same class of problem: a label equal to its source column name breaks a time-grain chart. It is not changed here. If that is confirmed, `KylinEngineSpec` could opt in to the same flag in a follow-up. ### BEFORE/AFTER We ran a live check against ClickHouse 26.9 through `/api/v1/chart/data`. It covered every time grain Superset offers for the dataset (18), with the x-axis labelled after its column, a `TEMPORAL_RANGE` filter starting mid-day, and two sort orders (by the axis and by the metric). Each result was compared with a reference query that uses a non-shadowing alias. | Driver | Before | After | |---|---|---| | `clickhouse://` | 0 / 36 (Code 215) | 36 / 36 | | `clickhouse+native://` | 0 / 36 (Code 215) | 36 / 36 | | `clickhousedb://` | 22 / 36 (wrong buckets, rows dropped) | 36 / 36 | | `clickhousedb+connect://` | 22 / 36 (wrong buckets, rows dropped) | 36 / 36 | ### TESTING INSTRUCTIONS `pytest tests/unit_tests/models/select_alias_shadowing_test.py`: 4 of 8 fail before the change and all 8 pass after. The tests compile the chart query Explore sends on SQLite, with the flag enabled through a patch, and check: - the alias is renamed and the WHERE reads the column; - the alias is kept by default; - an alias of the bare column is kept; - a metric named after a column is renamed; - results, empty or not, carry the Explore labels; - both ClickHouse specs set the flag. `tests/unit_tests/models`, `tests/unit_tests/connectors` and `tests/unit_tests/db_engine_specs/test_clickhouse.py` pass unchanged: 742 before and after. ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- 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]
