rusackas opened a new pull request, #41125:
URL: https://github.com/apache/superset/pull/41125
### SUMMARY
**TDD / failing-test PR for #36113.** This PR currently contains *only a
regression test* that demonstrates the bug. A fix commit will follow on the
same branch (converting this into a `fix:` PR) once CI confirms the failure.
#### The bug
`superset.sql.parse.sanitize_clause` is documented as a validator ("Make
sure the SQL clause is valid"), but it round-trips the user's clause through
SQLGlot's **dialect generator** and returns the *re-rendered* SQL rather than
the original text:
```python
return Dialect.get_or_raise(dialect).generate(statement._parsed, ...)
```
SQLGlot's Postgres dialect rewrites `ROUND(AVG(x), n)` into
`ROUND(CAST(AVG(x) AS DECIMAL), n)` at generation time (Postgres only has
`round(numeric, int)`). That cast is harmless on real PostgreSQL, where
unqualified `numeric`/`DECIMAL` is *unconstrained* and preserves the value —
but several engines **borrow the Postgres dialect** while having a default
`DECIMAL` scale of **0**. On those engines the injected cast rounds the
aggregate to an integer *before* the user's explicit `ROUND`, so a metric like
`ROUND(AVG(0.949583), 4)` returns `1` instead of `0.9496` (exactly the report
in #36113).
This is a regression from the `sqlparse` → SQLGlot migration: the legacy
`sanitize_clause` only *validated* the clause and returned it **unchanged**.
#### What the test shows
The parametrized test asserts the clause is returned verbatim with no
injected cast. On `master` it fails for `postgresql`, `cockroachdb`, `netezza`,
and `hana`, and passes for `redshift` (which gained a dedicated SQLGlot dialect
that no longer injects the cast), `mysql`, and `base`. This both reproduces the
reported mechanism and shows the underlying mutation is broader than just
Redshift.
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/sql/parse_tests.py -k preserves_aggregation_semantics
```
Expected on this commit: failures for
`postgresql`/`cockroachdb`/`netezza`/`hana` (cast injected). After the
forthcoming fix commit, all parametrizations pass.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #36113
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
