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 2022/08/22 22:35:08 UTC
[GitHub] [superset] codyml opened a new pull request, #21154: [WIP] fix(explore): Prevent unnecessary series limit subquery
codyml opened a new pull request, #21154:
URL: https://github.com/apache/superset/pull/21154
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] 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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
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] [superset] rusackas merged pull request #21154: fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
rusackas merged PR #21154:
URL: https://github.com/apache/superset/pull/21154
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] villebro commented on pull request #21154: [WIP] fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
villebro commented on PR #21154:
URL: https://github.com/apache/superset/pull/21154#issuecomment-1223611707
@codyml thanks for working on this - this is indeed an important fix. One of the reasons for that check is because legacy timeseries charts don't populate `series_columns`, and just assume `columns` or `groupby` is equal to `series_columns` (see #16660 for more context on this change). At the time we chose to ensure that the current functionality is unchanged, thereby adding that check. So I think removing it may cause a regression in some old charts. Let me see if I can propose an alternative solution that fixes the issue with `GENERIC_CHART_AXES` without introducing regression risk into old charts (give me a day or two).
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] hughhhh commented on a diff in pull request #21154: [WIP] fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #21154:
URL: https://github.com/apache/superset/pull/21154#discussion_r952026591
##########
superset/connectors/sqla/models.py:
##########
@@ -1410,7 +1410,7 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
col=selected, template_processor=template_processor
)
groupby_all_columns[outer.name] = outer
- if not series_column_names or outer.name in series_column_names:
+ if outer.name in series_column_names:
Review Comment:
Can you update this line for the `helpers.py` as well?
https://github.com/preset-io/superset/blob/cf02934b21ea608557ca600c28efc680647ef130/superset/models/helpers.py#L1518
I'm currently refactoring this piece of code for SIP-69 models via a Mixin
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] villebro commented on a diff in pull request #21154: [WIP] fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
villebro commented on code in PR #21154:
URL: https://github.com/apache/superset/pull/21154#discussion_r954038488
##########
superset/connectors/sqla/models.py:
##########
@@ -1410,7 +1410,7 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
col=selected, template_processor=template_processor
)
groupby_all_columns[outer.name] = outer
- if not series_column_names or outer.name in series_column_names:
+ if outer.name in series_column_names:
Review Comment:
The legacy time series line, bar etc charts. Essentially the ones with `is_timeseries` set to `True` and that are served by `viz.py`
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] github-actions[bot] commented on pull request #21154: [WIP] fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #21154:
URL: https://github.com/apache/superset/pull/21154#issuecomment-1223293634
@jinghua-qa Ephemeral environment spinning up at http://54.200.17.110:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] github-actions[bot] commented on pull request #21154: fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #21154:
URL: https://github.com/apache/superset/pull/21154#issuecomment-1227678754
Storybook has completed and can be viewed at
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] mistercrunch commented on pull request #21154: fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
mistercrunch commented on PR #21154:
URL: https://github.com/apache/superset/pull/21154#issuecomment-1228849288
Nice! thank you for fixing this!
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] jinghua-qa commented on pull request #21154: [WIP] fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
jinghua-qa commented on PR #21154:
URL: https://github.com/apache/superset/pull/21154#issuecomment-1223292439
/testenv up FEATURE_GENERIC_CHART_AXES=true
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] villebro commented on pull request #21154: [WIP] fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
villebro commented on PR #21154:
URL: https://github.com/apache/superset/pull/21154#issuecomment-1225978566
The legacy time series line, bar etc charts. Essentially the ones with `is_timeseries` set to `True` and that are served by `viz.py`
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] villebro commented on pull request #21154: [WIP] fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
villebro commented on PR #21154:
URL: https://github.com/apache/superset/pull/21154#issuecomment-1225223344
> Thanks for the info, that's the kind of thing I was worried about. One idea I had was to extend the query object to include the `x_axis` field that the frontend sends in `form_data`, and use that to conditionally exclude the `x_axis` column in that part of the code, which hopefully would leave legacy charts that don't support generic x-axis unaffected. Do you think that would work?
IMO including `x_axis` in the query object would be what we in Swedish call "tårta på tårta" ("cake on cake" in English referring to unnecessary redundancy), as `x_axis` should always be the complement of `columns` (`x_axis + series_columns = columns`). Incidentally, if we do want to reference `x_axis` in that part of the codebase, I would recommend calling it `base_axes` and making it a list/tuple of columns, as we may want to start introducing 3D charts or similar at some point, in which case there would be two base axes.
But back to the problem at hand - I have a hypothesis that I will validate now that should be uninvasive. Let me try it out and post my findings so we can get this fix merged asap.
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] villebro commented on a diff in pull request #21154: [WIP] fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
villebro commented on code in PR #21154:
URL: https://github.com/apache/superset/pull/21154#discussion_r953397704
##########
superset/connectors/sqla/models.py:
##########
@@ -1410,7 +1410,7 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
col=selected, template_processor=template_processor
)
groupby_all_columns[outer.name] = outer
- if not series_column_names or outer.name in series_column_names:
+ if outer.name in series_column_names:
Review Comment:
I think this should work - legacy timeseries charts will have set `is_timeseries` to `True`, and new charts will no longer be using it:
```suggestion
if (
is_timeseries and not series_column_names
) or outer.name in series_column_names:
```
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] codyml commented on a diff in pull request #21154: [WIP] fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
codyml commented on code in PR #21154:
URL: https://github.com/apache/superset/pull/21154#discussion_r954027427
##########
superset/connectors/sqla/models.py:
##########
@@ -1410,7 +1410,7 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
col=selected, template_processor=template_processor
)
groupby_all_columns[outer.name] = outer
- if not series_column_names or outer.name in series_column_names:
+ if outer.name in series_column_names:
Review Comment:
@villebro That makes sense to me, thanks! I can do some double-checking with the legacy timeseries charts – which ones are you talking about?
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] codyml commented on a diff in pull request #21154: [WIP] fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
codyml commented on code in PR #21154:
URL: https://github.com/apache/superset/pull/21154#discussion_r954052733
##########
superset/connectors/sqla/models.py:
##########
@@ -1410,7 +1410,7 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
col=selected, template_processor=template_processor
)
groupby_all_columns[outer.name] = outer
- if not series_column_names or outer.name in series_column_names:
+ if outer.name in series_column_names:
Review Comment:
Ok great, I'll update the PR ASAP!
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] codyml commented on pull request #21154: [WIP] fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
codyml commented on PR #21154:
URL: https://github.com/apache/superset/pull/21154#issuecomment-1224664202
> @codyml thanks for working on this - this is indeed an important fix. One of the reasons for that check is because legacy timeseries charts don't populate `series_columns`, and just assume `columns` or `groupby` is equal to `series_columns` (see #16660 for more context on this change). At the time we chose to ensure that the current functionality is unchanged, thereby adding that check. So I think removing it may cause a regression in some old charts. Let me see if I can propose an alternative solution that fixes the issue with `GENERIC_CHART_AXES` without introducing regression risk into old charts (give me a day or two).
@villebro Thanks for the info, that's the kind of thing I was worried about. One idea I had was to extend the query object to include the `x_axis` field that the frontend sends in `form_data`, and use that to conditionally exclude the `x_axis` column in that part of the code, which hopefully would leave legacy charts that don't support generic x-axis unaffected. Do you think that would work?
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] codecov[bot] commented on pull request #21154: [WIP] fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #21154:
URL: https://github.com/apache/superset/pull/21154#issuecomment-1223275786
# [Codecov](https://codecov.io/gh/apache/superset/pull/21154?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#21154](https://codecov.io/gh/apache/superset/pull/21154?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3c6ee9e) into [master](https://codecov.io/gh/apache/superset/commit/0bf4e56dc3e129d2b9239f055212249ba95521e4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0bf4e56) will **decrease** coverage by `11.45%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #21154 +/- ##
===========================================
- Coverage 66.16% 54.70% -11.46%
===========================================
Files 1773 1773
Lines 67689 67689
Branches 7218 7218
===========================================
- Hits 44786 37032 -7754
- Misses 21059 28813 +7754
Partials 1844 1844
```
| Flag | Coverage Δ | |
|---|---|---|
| mysql | `?` | |
| postgres | `?` | |
| presto | `53.07% <100.00%> (?)` | |
| python | `57.46% <100.00%> (-23.69%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `50.74% <100.00%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/21154?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/21154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `77.48% <100.00%> (-12.14%)` | :arrow_down: |
| [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/21154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvZGFzaGJvYXJkX2ltcG9ydF9leHBvcnQucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/21154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `0.00% <0.00%> (-88.89%)` | :arrow_down: |
| [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/21154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `0.00% <0.00%> (-85.30%)` | :arrow_down: |
| [superset/key\_value/commands/delete\_expired.py](https://codecov.io/gh/apache/superset/pull/21154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZV9leHBpcmVkLnB5) | `0.00% <0.00%> (-80.77%)` | :arrow_down: |
| [superset/dashboards/commands/importers/v0.py](https://codecov.io/gh/apache/superset/pull/21154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9jb21tYW5kcy9pbXBvcnRlcnMvdjAucHk=) | `15.62% <0.00%> (-76.25%)` | :arrow_down: |
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/21154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `25.00% <0.00%> (-69.05%)` | :arrow_down: |
| [superset/datasets/commands/create.py](https://codecov.io/gh/apache/superset/pull/21154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvY3JlYXRlLnB5) | `29.41% <0.00%> (-68.63%)` | :arrow_down: |
| [superset/datasets/commands/importers/v0.py](https://codecov.io/gh/apache/superset/pull/21154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvaW1wb3J0ZXJzL3YwLnB5) | `24.03% <0.00%> (-67.45%)` | :arrow_down: |
| [superset/reports/commands/execute.py](https://codecov.io/gh/apache/superset/pull/21154/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9leGVjdXRlLnB5) | `24.64% <0.00%> (-66.91%)` | :arrow_down: |
| ... and [280 more](https://codecov.io/gh/apache/superset/pull/21154/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] hughhhh commented on a diff in pull request #21154: [WIP] fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #21154:
URL: https://github.com/apache/superset/pull/21154#discussion_r952026591
##########
superset/connectors/sqla/models.py:
##########
@@ -1410,7 +1410,7 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
col=selected, template_processor=template_processor
)
groupby_all_columns[outer.name] = outer
- if not series_column_names or outer.name in series_column_names:
+ if outer.name in series_column_names:
Review Comment:
Can you update this line for the `helpers.py` as well?
https://github.com/preset-io/superset/blob/cf02934b21ea608557ca600c28efc680647ef130/superset/models/helpers.py#L1518
--
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: notifications-unsubscribe@superset.apache.org
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] [superset] github-actions[bot] commented on pull request #21154: fix(explore): Prevent unnecessary series limit subquery
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #21154:
URL: https://github.com/apache/superset/pull/21154#issuecomment-1228737733
Ephemeral environment shutdown and build artifacts deleted.
--
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: notifications-unsubscribe@superset.apache.org
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