john-bodley commented on a change in pull request #16950:
URL: https://github.com/apache/superset/pull/16950#discussion_r732263165
##########
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:
@villebro the issue is any column which is flagged as being `is_dttm`
without a type is treated as a `DATETIME` type which could result in the wrong
conversion.
Again it seems like the fix was unnecessary as the actual solution/paved
path workflow is merely to define the type of the calculated column. We likely
should make this a required field and perform form validation.
--
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]
