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 2020/09/07 16:05:28 UTC
[GitHub] [incubator-superset] binome74 opened a new pull request #10811: "P1W" grain should respect DATEFIRST setting in MS SQL Server
binome74 opened a new pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811
### SUMMARY
In MS SQL Server specifying SET DATEFIRST has no effect on DATEDIFF. [DATEDIFF always uses Sunday](https://docs.microsoft.com/en-us/sql/t-sql/functions/datediff-transact-sql?view=sql-server-2017#remarks) as the first day of the week to ensure the function operates in a deterministic way. This is not desirable for locales where weeks start on Monday. To get the first day of a calendar week for the current locale use DATEPART which does respect the DATEFIRST setting.
### TEST PLAN
1. Create a test table in SQL Server (any version).
```
SELECT d, n
INTO p1w_test
FROM (VALUES
('2020-08-01', 1), ('2020-08-02', 2), ('2020-08-03', 3), ('2020-08-04', 4), ('2020-08-05', 5), ('2020-08-06', 6)
,('2020-08-07', 7), ('2020-08-08', 8), ('2020-08-09', 9), ('2020-08-10', 10), ('2020-08-11', 11), ('2020-08-12', 12)
,('2020-08-13', 13), ('2020-08-14', 14), ('2020-08-15', 15), ('2020-08-16', 16), ('2020-08-17', 17), ('2020-08-18', 18)
,('2020-08-19', 19), ('2020-08-20', 20), ('2020-08-21', 21), ('2020-08-22', 22), ('2020-08-23', 23), ('2020-08-24', 24)
) T(d, n)
;
```
2. Add p1w_test table as a source in Superset.
3. Explore the table, set "Time Column" to "d", "Time Grain" to "week". For the query pick SUM(n).
4. For North American locales (@@datefirst == 7) the resulting table should be
```
2020-07-26 1
2020-08-02 35
2020-08-09 84
2020-08-16 133
2020-08-23 47
```
5. For European locales (@@datefirst == 1) the resulting table should be
```
2020-07-26 3
2020-08-02 42
2020-08-09 91
2020-08-16 140
2020-08-23 24
```
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] binome74 commented on a change in pull request #10811: "P1W" grain should respect DATEFIRST setting in MS SQL Server
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #10811: "P1W" grain should respect DATEFIRST setting in MS SQL Server
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#discussion_r535201186
##########
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:
Ok, sounds good. Stylistic request: can we change the reserved names (`day`, `weekday` etc) to all caps?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on pull request #10811: fix(mssql): week time grain should respect datefirst setting
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#issuecomment-737898321
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=h1) Report
> Merging [#10811](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=desc) (60f6fac) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1d76c5906e98ac7fd601e9ab643911d438e17744?el=desc) (1d76c59) will **decrease** coverage by `11.13%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10811 +/- ##
===========================================
- Coverage 65.40% 54.26% -11.14%
===========================================
Files 802 420 -382
Lines 37817 14796 -23021
Branches 3557 3812 +255
===========================================
- Hits 24733 8029 -16704
+ Misses 12978 6767 -6211
+ Partials 106 0 -106
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `54.26% <ø> (-1.57%)` | :arrow_down: |
| javascript | `?` | |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/components/Menu/LanguagePicker.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTWVudS9MYW5ndWFnZVBpY2tlci50c3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...components/AdhocFilterEditPopoverSqlTabContent.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9BZGhvY0ZpbHRlckVkaXRQb3BvdmVyU3FsVGFiQ29udGVudC5qc3g=) | `7.14% <0.00%> (-92.86%)` | :arrow_down: |
| [...ponents/Select/WindowedSelect/WindowedMenuList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L1dpbmRvd2VkU2VsZWN0L1dpbmRvd2VkTWVudUxpc3QudHN4) | `3.70% <0.00%> (-89.40%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...ontend/src/dashboard/util/getDashboardFilterKey.ts](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERhc2hib2FyZEZpbHRlcktleS50cw==) | `14.28% <0.00%> (-85.72%)` | :arrow_down: |
| ... and [785 more](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=footer). Last update [1d76c59...60f6fac](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] stale[bot] commented on pull request #10811: "P1W" grain should respect DATEFIRST setting in MS SQL Server
Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#issuecomment-734494756
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue `.pinned` to prevent stale bot from closing the issue.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on pull request #10811: "P1W" grain should respect DATEFIRST setting in MS SQL Server
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#issuecomment-737898321
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=h1) Report
> Merging [#10811](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=desc) (a7e72a0) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1d76c5906e98ac7fd601e9ab643911d438e17744?el=desc) (1d76c59) will **decrease** coverage by `11.09%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10811 +/- ##
===========================================
- Coverage 65.40% 54.30% -11.10%
===========================================
Files 802 420 -382
Lines 37817 14796 -23021
Branches 3557 3812 +255
===========================================
- Hits 24733 8035 -16698
+ Misses 12978 6761 -6217
+ Partials 106 0 -106
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `54.30% <ø> (-1.53%)` | :arrow_down: |
| javascript | `?` | |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/components/Menu/LanguagePicker.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTWVudS9MYW5ndWFnZVBpY2tlci50c3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...components/AdhocFilterEditPopoverSqlTabContent.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9BZGhvY0ZpbHRlckVkaXRQb3BvdmVyU3FsVGFiQ29udGVudC5qc3g=) | `7.14% <0.00%> (-92.86%)` | :arrow_down: |
| [...ponents/Select/WindowedSelect/WindowedMenuList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L1dpbmRvd2VkU2VsZWN0L1dpbmRvd2VkTWVudUxpc3QudHN4) | `3.70% <0.00%> (-89.40%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...ontend/src/dashboard/util/getDashboardFilterKey.ts](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERhc2hib2FyZEZpbHRlcktleS50cw==) | `14.28% <0.00%> (-85.72%)` | :arrow_down: |
| ... and [785 more](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=footer). Last update [1d76c59...a7e72a0](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on pull request #10811: fix(mssql): week time grain should respect datefirst setting
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#issuecomment-737898321
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=h1) Report
> Merging [#10811](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=desc) (60f6fac) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1d76c5906e98ac7fd601e9ab643911d438e17744?el=desc) (1d76c59) will **decrease** coverage by `10.49%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10811 +/- ##
===========================================
- Coverage 65.40% 54.90% -10.50%
===========================================
Files 802 420 -382
Lines 37817 14796 -23021
Branches 3557 3812 +255
===========================================
- Hits 24733 8124 -16609
+ Misses 12978 6672 -6306
+ Partials 106 0 -106
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `54.90% <ø> (-0.93%)` | :arrow_down: |
| javascript | `?` | |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/components/Menu/LanguagePicker.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTWVudS9MYW5ndWFnZVBpY2tlci50c3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...components/AdhocFilterEditPopoverSqlTabContent.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9BZGhvY0ZpbHRlckVkaXRQb3BvdmVyU3FsVGFiQ29udGVudC5qc3g=) | `7.14% <0.00%> (-92.86%)` | :arrow_down: |
| [...ponents/Select/WindowedSelect/WindowedMenuList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L1dpbmRvd2VkU2VsZWN0L1dpbmRvd2VkTWVudUxpc3QudHN4) | `3.70% <0.00%> (-89.40%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...ontend/src/dashboard/util/getDashboardFilterKey.ts](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERhc2hib2FyZEZpbHRlcktleS50cw==) | `14.28% <0.00%> (-85.72%)` | :arrow_down: |
| ... and [785 more](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=footer). Last update [1d76c59...60f6fac](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] binome74 commented on a change in pull request #10811: fix(mssql): week time grain should respect datefirst setting
Posted by GitBox <gi...@apache.org>.
binome74 commented on a change in pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#discussion_r535213459
##########
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:
No problem. Did it so.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #10811: "P1W" grain should respect DATEFIRST setting in MS SQL Server
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#discussion_r484666412
##########
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:
The `P1W` time grain tends to refer to the official weekly date truncation, in this case `WEEK`. There are a few time grains that target specific weekday starts that can be seen here: https://github.com/apache/incubator-superset/blob/master/superset/db_engine_specs/base.py#L94-L95 . Do you think it would make sense to leave `P1W` unchanged, and add two new time grains, specifically `1969-12-28T00:00:00Z/P1W` to also refer to the regular `WEEK` interval, and `1969-12-29T00:00:00Z/P1W` to refer to the proposed monday starting week definition?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #10811: fix(mssql): week time grain should respect datefirst setting
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#discussion_r535202520
##########
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:
You also need to make the changes proposed by the linter:
```
superset/db_engine_specs/mssql.py:54:0: C0301: Line too long (99/88) (line-too-long)
superset/db_engine_specs/mssql.py:49:0: C0301: Line too long (102/88) (line-too-long)
```
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] binome74 commented on a change in pull request #10811: "P1W" grain should respect DATEFIRST setting in MS SQL Server
Posted by GitBox <gi...@apache.org>.
binome74 commented on a change in pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#discussion_r535199163
##########
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:
Yes
`"P1W": "DATEADD(day, 1 - DATEPART(weekday, {col}), DATEADD(day, DATEDIFF(day, 0, {col}), 0))"`
here `DATEADD(day, 1 - DATEPART(weekday, {col})` does the job
and `DATEADD(day, DATEDIFF(day, 0, {col}), 0))` just truncates the time part.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] binome74 commented on a change in pull request #10811: "P1W" grain should respect DATEFIRST setting in MS SQL Server
Posted by GitBox <gi...@apache.org>.
binome74 commented on a change in pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#discussion_r490366875
##########
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:
I'll need some time to re-check it and possibly provide better solution.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] binome74 commented on a change in pull request #10811: "P1W" grain should respect DATEFIRST setting in MS SQL Server
Posted by GitBox <gi...@apache.org>.
binome74 commented on a change in pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#discussion_r484675230
##########
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:
I think if we want see more or less consistent behaviour of Superset across all the supported engines we should check how other engines calculate P1W interval i.e. what "the official weekly date truncation" actually means: respecting or not respecting the current locale. If they do respect the locale then we should change it for MS SQL server accordingly (as proposed). Else we surely can introduce these 2 explicit "starting Monday"/"starting Sunday" intervals, but I think it also worth mentioning as a remark in the documentation to avoid confusion.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] binome74 commented on a change in pull request #10811: "P1W" grain should respect DATEFIRST setting in MS SQL Server
Posted by GitBox <gi...@apache.org>.
binome74 commented on a change in pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#discussion_r490366875
##########
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:
I'll need some time to re-check it and possibly provide better solution.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on pull request #10811: fix(mssql): week time grain should respect datefirst setting
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#issuecomment-737898321
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=h1) Report
> Merging [#10811](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=desc) (8f09c18) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a7bba92469025a1f75149ff616af9dba24e81717?el=desc) (a7bba92) will **increase** coverage by `9.25%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10811 +/- ##
==========================================
+ Coverage 64.45% 73.71% +9.25%
==========================================
Files 937 472 -465
Lines 45319 16782 -28537
Branches 4317 4341 +24
==========================================
- Hits 29211 12371 -16840
+ Misses 15948 4308 -11640
+ Partials 160 103 -57
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `55.10% <ø> (+27.22%)` | :arrow_up: |
| javascript | `62.99% <ø> (ø)` | |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/SqlLab/components/LimitControl.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0xpbWl0Q29udHJvbC50c3g=) | `89.36% <0.00%> (-1.95%)` | :arrow_down: |
| [...ntend/src/views/CRUD/annotation/AnnotationList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYW5ub3RhdGlvbi9Bbm5vdGF0aW9uTGlzdC50c3g=) | `76.08% <0.00%> (-0.84%)` | :arrow_down: |
| [...ews/CRUD/annotationlayers/AnnotationLayerModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYW5ub3RhdGlvbmxheWVycy9Bbm5vdGF0aW9uTGF5ZXJNb2RhbC50c3g=) | `73.03% <0.00%> (-0.83%)` | :arrow_down: |
| [...t-frontend/src/views/CRUD/welcome/SavedQueries.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvd2VsY29tZS9TYXZlZFF1ZXJpZXMudHN4) | `62.24% <0.00%> (-0.65%)` | :arrow_down: |
| [...tend/src/views/CRUD/annotation/AnnotationModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYW5ub3RhdGlvbi9Bbm5vdGF0aW9uTW9kYWwudHN4) | `66.14% <0.00%> (-0.53%)` | :arrow_down: |
| [superset/result\_set.py](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQvcmVzdWx0X3NldC5weQ==) | | |
| [superset/connectors/druid/models.py](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9kcnVpZC9tb2RlbHMucHk=) | | |
| [superset/tasks/alerts/observer.py](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdGFza3MvYWxlcnRzL29ic2VydmVyLnB5) | | |
| [...s/versions/eca4694defa7\_sqllab\_setting\_defaults.py](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbWlncmF0aW9ucy92ZXJzaW9ucy9lY2E0Njk0ZGVmYTdfc3FsbGFiX3NldHRpbmdfZGVmYXVsdHMucHk=) | | |
| [superset/css\_templates/api.py](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY3NzX3RlbXBsYXRlcy9hcGkucHk=) | | |
| ... and [605 more](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=footer). Last update [a7bba92...8f09c18](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on pull request #10811: fix(mssql): week time grain should respect datefirst setting
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#issuecomment-737898321
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=h1) Report
> Merging [#10811](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=desc) (8f09c18) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a7bba92469025a1f75149ff616af9dba24e81717?el=desc) (a7bba92) will **increase** coverage by `8.92%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10811 +/- ##
==========================================
+ Coverage 64.45% 73.38% +8.92%
==========================================
Files 937 472 -465
Lines 45319 16782 -28537
Branches 4317 4341 +24
==========================================
- Hits 29211 12315 -16896
+ Misses 15948 4360 -11588
+ Partials 160 107 -53
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `54.51% <ø> (+26.63%)` | :arrow_up: |
| javascript | `62.99% <ø> (ø)` | |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...et-frontend/src/SqlLab/components/LimitControl.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0xpbWl0Q29udHJvbC50c3g=) | `89.36% <0.00%> (-1.95%)` | :arrow_down: |
| [...ntend/src/views/CRUD/annotation/AnnotationList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYW5ub3RhdGlvbi9Bbm5vdGF0aW9uTGlzdC50c3g=) | `76.08% <0.00%> (-0.84%)` | :arrow_down: |
| [...ews/CRUD/annotationlayers/AnnotationLayerModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYW5ub3RhdGlvbmxheWVycy9Bbm5vdGF0aW9uTGF5ZXJNb2RhbC50c3g=) | `73.03% <0.00%> (-0.83%)` | :arrow_down: |
| [...t-frontend/src/views/CRUD/welcome/SavedQueries.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvd2VsY29tZS9TYXZlZFF1ZXJpZXMudHN4) | `62.24% <0.00%> (-0.65%)` | :arrow_down: |
| [...tend/src/views/CRUD/annotation/AnnotationModal.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvYW5ub3RhdGlvbi9Bbm5vdGF0aW9uTW9kYWwudHN4) | `66.14% <0.00%> (-0.53%)` | :arrow_down: |
| [superset/utils/hashing.py](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvaGFzaGluZy5weQ==) | | |
| [superset/views/chart/mixin.py](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY2hhcnQvbWl4aW4ucHk=) | | |
| [...otation\_layers/annotations/commands/bulk\_delete.py](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYW5ub3RhdGlvbl9sYXllcnMvYW5ub3RhdGlvbnMvY29tbWFuZHMvYnVsa19kZWxldGUucHk=) | | |
| [superset/tasks/slack\_util.py](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdGFza3Mvc2xhY2tfdXRpbC5weQ==) | | |
| [superset/reports/commands/execute.py](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9leGVjdXRlLnB5) | | |
| ... and [603 more](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=footer). Last update [a7bba92...8f09c18](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #10811: "P1W" grain should respect DATEFIRST setting in MS SQL Server
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#discussion_r484678167
##########
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:
With "official weekly date truncation" I was referring to what the database by default means by `WEEK`, which can mean different things for different databases. Personally, I tend to use the weekly grain to see what 7 day aggregates are; I usually don't so much care if it's a week starting on Sunday or Monday. For the use case where this is important, I suggest using the dedicated time grains where supported. Going forward, I do agree it would be great to start introducing more consistency in what these terms mean, and we encourage and gladly accept contributions to that end.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] binome74 commented on a change in pull request #10811: "P1W" grain should respect DATEFIRST setting in MS SQL Server
Posted by GitBox <gi...@apache.org>.
binome74 commented on a change in pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#discussion_r484675230
##########
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:
I think if we want see more or less consistent behaviour of Superset across all the supported engines we should check how other engines calculate P1W interval i.e. what "the official weekly date truncation" actually means: respecting or not respecting the current locale. If they do respect the locale then we should change it for MS SQL server accordingly (as proposed). Else we surely can introduce these 2 explicit "starting Monday"/"starting Sunday" intervals, but I think it also worth mentioning as a remark in the documentation.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on pull request #10811: fix(mssql): week time grain should respect datefirst setting
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#issuecomment-737898321
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=h1) Report
> Merging [#10811](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=desc) (8f09c18) into [master](https://codecov.io/gh/apache/incubator-superset/commit/a7bba92469025a1f75149ff616af9dba24e81717?el=desc) (a7bba92) will **decrease** coverage by `1.46%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10811 +/- ##
==========================================
- Coverage 64.45% 62.99% -1.47%
==========================================
Files 937 472 -465
Lines 45319 16773 -28546
Branches 4317 4341 +24
==========================================
- Hits 29211 10566 -18645
+ Misses 15948 6030 -9918
- Partials 160 177 +17
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `62.99% <ø> (ø)` | |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/index.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9pbmRleC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupColors.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwQ29sb3JzLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/chart/ChartContainer.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0Q29udGFpbmVyLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/dashboard/containers/Dashboard.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...-frontend/src/visualizations/presets/MainPreset.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL3ByZXNldHMvTWFpblByZXNldC5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...tend/src/dashboard/containers/DashboardBuilder.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZEJ1aWxkZXIuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/setup/setupFormatters.ts](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRm9ybWF0dGVycy50cw==) | `0.00% <0.00%> (-75.00%)` | :arrow_down: |
| [...c/visualizations/FilterBox/FilterBoxChartPlugin.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL0ZpbHRlckJveC9GaWx0ZXJCb3hDaGFydFBsdWdpbi5qcw==) | `0.00% <0.00%> (-75.00%)` | :arrow_down: |
| [...c/visualizations/TimeTable/TimeTableChartPlugin.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL1RpbWVUYWJsZS9UaW1lVGFibGVDaGFydFBsdWdpbi5qcw==) | `0.00% <0.00%> (-75.00%)` | :arrow_down: |
| ... and [502 more](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=footer). Last update [a7bba92...8f09c18](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on pull request #10811: "P1W" grain should respect DATEFIRST setting in MS SQL Server
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#issuecomment-737898321
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=h1) Report
> Merging [#10811](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=desc) (a7e72a0) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1d76c5906e98ac7fd601e9ab643911d438e17744?el=desc) (1d76c59) will **decrease** coverage by `10.50%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #10811 +/- ##
===========================================
- Coverage 65.40% 54.89% -10.51%
===========================================
Files 802 420 -382
Lines 37817 14796 -23021
Branches 3557 3812 +255
===========================================
- Hits 24733 8122 -16611
+ Misses 12978 6674 -6304
+ Partials 106 0 -106
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `54.89% <ø> (-0.94%)` | :arrow_down: |
| javascript | `?` | |
| python | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/dashboard/util/dnd-reorder.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2RuZC1yZW9yZGVyLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...rset-frontend/src/dashboard/util/getEmptyLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEVtcHR5TGF5b3V0Lmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/components/Menu/LanguagePicker.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTWVudS9MYW5ndWFnZVBpY2tlci50c3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...dashboard/components/resizable/ResizableHandle.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL3Jlc2l6YWJsZS9SZXNpemFibGVIYW5kbGUuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../src/dashboard/util/getFilterScopeFromNodesTree.js](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldEZpbHRlclNjb3BlRnJvbU5vZGVzVHJlZS5qcw==) | `0.00% <0.00%> (-93.48%)` | :arrow_down: |
| [...components/AdhocFilterEditPopoverSqlTabContent.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9BZGhvY0ZpbHRlckVkaXRQb3BvdmVyU3FsVGFiQ29udGVudC5qc3g=) | `7.14% <0.00%> (-92.86%)` | :arrow_down: |
| [...ponents/Select/WindowedSelect/WindowedMenuList.tsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L1dpbmRvd2VkU2VsZWN0L1dpbmRvd2VkTWVudUxpc3QudHN4) | `3.70% <0.00%> (-89.40%)` | :arrow_down: |
| [...src/dashboard/components/gridComponents/Header.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0hlYWRlci5qc3g=) | `10.52% <0.00%> (-86.85%)` | :arrow_down: |
| [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | `13.33% <0.00%> (-86.67%)` | :arrow_down: |
| [...ontend/src/dashboard/util/getDashboardFilterKey.ts](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldERhc2hib2FyZEZpbHRlcktleS50cw==) | `14.28% <0.00%> (-85.72%)` | :arrow_down: |
| ... and [785 more](https://codecov.io/gh/apache/incubator-superset/pull/10811/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=footer). Last update [1d76c59...a7e72a0](https://codecov.io/gh/apache/incubator-superset/pull/10811?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #10811: "P1W" grain should respect DATEFIRST setting in MS SQL Server
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#discussion_r535162588
##########
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:
@binome74 I'm ok with defaulting to the default locale of the database. In the latest commit I didn't see the original `DATEPART` call; did I understand correctly that the current syntax works similarly, but supports older versions of SQL Server?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request #10811: "P1W" grain should respect DATEFIRST setting in MS SQL Server
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #10811:
URL: https://github.com/apache/incubator-superset/pull/10811#discussion_r484681966
##########
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:
With regard to the proposed change here, and given the ambiguity of what `P1W` means, I think this change makes sense as a "best effort" weekly time grain (for the lack of a better term). But if you could add the explicit Sunday and Monday starting grains, then users could have the option of using either the local week definition, or a fixed one.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org