You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2019/03/26 18:05:20 UTC

[GitHub] [incubator-superset] agrawaldevesh commented on issue #7131: Make time expression native SQLAlchemy element

agrawaldevesh commented on issue #7131: Make time expression native SQLAlchemy element
URL: https://github.com/apache/incubator-superset/pull/7131#issuecomment-476777473
 
 
   Thanks @villebro, Happy to "test this in prod" at Uber and report back to you. The code changes look okay to me and I think they should work.
   
   I have just one question: I am wondering if the TimestampExpression should be a good vehicle to fix the TZ issues plaguing superset: https://github.com/apache/incubator-superset/issues/6768 and https://github.com/apache/incubator-superset/issues/1149 for example.
   
   The problem is that Superset is essentially TZ unaware, and it assumes that the stored data is in the local TZ. I was curious if TimestampExpression could have a ".tz" field in it eventually ? And for databases that do support TZ's, that tz field is plumbed in when generating the timestamp expression sql. 
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org