villebro commented on a change in pull request #16950:
URL: https://github.com/apache/superset/pull/16950#discussion_r726814090
##########
File path: superset/connectors/sqla/models.py
##########
@@ -334,8 +334,7 @@ def dttm_sql_literal(
],
) -> str:
"""Convert datetime object to a SQL expression string"""
- dttm_type = self.type or ("DATETIME" if self.is_dttm else None)
- sql = self.db_engine_spec.convert_dttm(dttm_type, dttm) if dttm_type
else None
+ sql = self.db_engine_spec.convert_dttm(self.type, dttm) if self.type
else None
Review comment:
What types of problems did the original change cause? I agree that the
assumption that a column is `DATETIME` when the datatype is undefined and
`is_dttm` is `True` may be an oversimplification in some cases, but I would
expect it's probably usually the most correct assumption, as I think many users
don't bother populating the correct datatype in the widget.
Long term, maybe we should execute a query with the calculated column to see
what datatype the database returns and use that to populate the value if it's
undefined when saving the dataset (probably quite a bit of work, but would
probably solve this problem).
--
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]
