binome74 commented on a change in pull request #10811:
URL:
https://github.com/apache/incubator-superset/pull/10811#discussion_r535140274
##########
File path: superset/db_engine_specs/mssql.py
##########
@@ -46,7 +46,7 @@ class MssqlEngineSpec(BaseEngineSpec):
"PT0.5H": "DATEADD(minute, DATEDIFF(minute, 0, {col}) / 30 * 30, 0)",
"PT1H": "DATEADD(hour, DATEDIFF(hour, 0, {col}), 0)",
"P1D": "DATEADD(day, DATEDIFF(day, 0, {col}), 0)",
- "P1W": "DATEADD(week, DATEDIFF(week, 0, {col}), 0)",
Review comment:
So, finally
1. As of "P1W" I still believe that Superset should behave expectedly i.e.
if the underlying database/connection has certain locale settings Superset
should not override them unless the user wants to specify it explicitly. I also
have added the `DATEADD(day, DATEDIFF(day, 0, {col}), 0)` wrap around the
`{col}` in order to correctly truncate the time part in case of MS SQL
2005/2008. Not the least is that the previous version has returned a wrong
(T+1) value for the axis' labels / cells text, though is hasn't affected the
grouping itself.
2. I added the support of "1969-12-28T00:00:00Z/P1W" and
"1969-12-29T00:00:00Z/P1W" grains for the user to have a choice to explicitly
indicate which start of the week he or she prefers for the query. For the
"week_start_sunday" I used the previous version of "P1W" with subtraction of
one day. The "week_start_monday" is calculated in similar fashion.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
