mikebridge opened a new pull request, #42095: URL: https://github.com/apache/superset/pull/42095
### SUMMARY Fixes the PostgreSQL/Redshift grouping failure described in [SC-112048](https://app.shortcut.com/preset/story/112048). When SQL sanitization canonicalized a custom metric’s `DATE_TRUNC` unit to uppercase while the chart time-grain expression remained lowercase, PostgreSQL treated the expressions as different and required the raw datetime column in `GROUP BY`. This adds an engine-specific custom metric normalization hook, normalizes PostgreSQL-family `DATE_TRUNC` unit literals consistently, and applies it to saved and ad hoc metrics after subquery/RLS validation. Other database engines retain the identity behavior. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF Not applicable; this is query-generation behavior. Before the change, the reproduction raises a PostgreSQL `GroupingError`. After the change, the chart renders with matching `date_trunc` expressions in the metric and grouping clause. ### TESTING INSTRUCTIONS 1. Connect Superset to PostgreSQL and create a dataset with a timestamp column and data spanning multiple quarters. 2. Create a time-series chart with the timestamp as the x-axis and **Quarter** as the time grain. 3. Add a custom SQL metric containing mixed- or uppercase `DATE_TRUNC` units, for example: ```sql CASE WHEN DATE_TRUNC('QUARTER', created_at) = '2024-01-01' THEN COUNT(*) / 1000 WHEN DATE_TRUNC('quarter', created_at) = '2024-04-01' THEN COUNT(*) / 2000 END ``` 4. Run the chart and confirm it renders without a grouping error. Inspect the generated SQL and confirm the metric and `GROUP BY` use equivalent `date_trunc` unit literals. 5. Repeat with a different time grain and a metric using another function (for example `ROUND(AVG(amount), 2)`) to verify unrelated metric SQL is preserved. Automated verification: - `pre-commit run --all-files` - `pytest -q tests/unit_tests/db_engine_specs/test_base.py tests/unit_tests/db_engine_specs/test_postgres.py tests/unit_tests/db_engine_specs/test_redshift.py tests/unit_tests/models/helpers_test.py` (251 passed) - Focused query-context and saved/ad hoc metric integration tests (4 passed; PostgreSQL-only test also verified against PostgreSQL 16) ### ADDITIONAL INFORMATION - [x] Has associated issue: [SC-112048](https://app.shortcut.com/preset/story/112048) - [ ] 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 -- 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]
