john-bodley commented on pull request #16795:
URL: https://github.com/apache/superset/pull/16795#issuecomment-955122235


   @betodealmeida and @villebro I don't believe this logic works for all cases. 
You could have a temporal field which is encoded as a string per the Python 
date format which shouldn't be converted to a `TIMESTAMP`. 
   
   Isn't the correct logic to use the type of the column rather than assuming 
it's a `TIMESTAMP`? Furthermore `self.db_engine_spec.convert_dttm("TIMESTAMP", 
dttm)` could return `None` (even for the `TIMESTAMP` target type). I was 
thinking the logic should be of the form,
   
   ```python
   if column_map[dimension].is_temporal and and isinstance(value, str):
       if rv := self.db_engine_spec.convert_dttm(
           column_map[dimension].type, 
           dateutil.parser.parse(value),
       ):
           value = text(rv)
   ```
   


-- 
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]

Reply via email to