bito-code-review[bot] commented on PR #43864: URL: https://github.com/apache/superset/pull/43864#issuecomment-5541117800
<!-- Bito Reply --> The flagged issue is correct. The `get_timestamp_expression` method in `superset/connectors/sqla/models.py` currently bypasses the `Grouping` wrapper applied to other calculated columns, which can lead to operator precedence bugs in time-series queries. To resolve this, you should wrap the expression returned by `get_timestamp_expression` in a `Grouping` object, similar to how other calculated columns are handled in the PR. Would you like me to implement this fix for `get_timestamp_expression`? I can also check the rest of the PR comments if you would like to address other issues as well. **superset/connectors/sqla/models.py** ``` from sqlalchemy.sql.elements import ColumnClause, Grouping, TextClause # ... inside get_timestamp_expression ... return Grouping(literal_column(expression, type_=type_)) ``` -- 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]
