[GitHub] mistercrunch commented on a change in pull request #4343: fixing Issue #4335 - Not quoting column name when doing a time series

[GitBox <gi...@apache.org>]

mistercrunch commented on a change in pull request #4343: fixing Issue #4335 - Not quoting column name when doing a time series
URL: https://github.com/apache/incubator-superset/pull/4343#discussion_r166191162
 
 

 ##########
 File path: superset/connectors/sqla/models.py
 ##########
 @@ -113,7 +113,7 @@ def get_time_filter(self, start_dttm, end_dttm):
     def get_timestamp_expression(self, time_grain):
         """Getting the time component of the query"""
         expr = self.expression or self.column_name
-        if not self.expression and not time_grain:
+        if not self.expression and (not time_grain or time_grain == u'Time Column'):
 
 Review comment:
   `u'foo'` isn't python3 compatible I think. I'm surprised that label-looking `Time Column` is a possible value here, somehow it should be `time_column` or `None` instead.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services