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 2021/02/08 14:24:58 UTC

[GitHub] [superset] nikolagigic opened a new pull request #12998: feat(explore): Execute predicate query string if set

nikolagigic opened a new pull request #12998:
URL: https://github.com/apache/superset/pull/12998


   ### SUMMARY
   Execute predicate query string if set to improve the filters loads performance.
   https://github.com/apache/superset/issues/12531
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   https://user-images.githubusercontent.com/26679866/107232342-97e89800-6a21-11eb-86c9-580ef6c8c1c1.mp4
   
   ### TEST PLAN
   <!--- What steps should be taken to 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:
   - [ ] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [x] 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] [superset] villebro commented on a change in pull request #12998: feat(explore): Execute predicate query string if set

Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #12998:
URL: https://github.com/apache/superset/pull/12998#discussion_r573818208



##########
File path: superset/viz.py
##########
@@ -177,6 +177,11 @@ def process_metrics(self) -> None:
         self.all_metrics = list(self.metric_dict.values())
         self.metric_labels = list(self.metric_dict.keys())
 
+    def construct_where(self, query_obj: QueryObjectDict, predicate_string: str) -> str:
+        if query_obj:
+            predicate_string = "({}) AND ({})".format(query_obj, predicate_string)

Review comment:
       The signature here doesn't seem correct, `query_obj` here seems to refer to the original `where` and be `Optional[str]`?




----------------------------------------------------------------
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] [superset] junlincc commented on pull request #12998: feat(explore): Execute predicate query string if set

Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #12998:
URL: https://github.com/apache/superset/pull/12998#issuecomment-779336224


   @eugeniamz @ktmud requesting review to make sure this change is okay with you both. 


----------------------------------------------------------------
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] [superset] codecov-io commented on pull request #12998: feat(explore): Execute predicate query string if set

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12998:
URL: https://github.com/apache/superset/pull/12998#issuecomment-776723646


   # [Codecov](https://codecov.io/gh/apache/superset/pull/12998?src=pr&el=h1) Report
   > Merging [#12998](https://codecov.io/gh/apache/superset/pull/12998?src=pr&el=desc) (b7b675d) into [master](https://codecov.io/gh/apache/superset/commit/76c225db7ee464d5b642cf7e0aa6c990df88f99d?el=desc) (76c225d) will **decrease** coverage by `2.16%`.
   > The diff coverage is `14.28%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12998/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12998?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12998      +/-   ##
   ==========================================
   - Coverage   69.14%   66.97%   -2.17%     
   ==========================================
     Files        1025      489     -536     
     Lines       48765    28787   -19978     
     Branches     5188        0    -5188     
   ==========================================
   - Hits        33718    19280   -14438     
   + Misses      14913     9507    -5406     
   + Partials      134        0     -134     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `66.97% <14.28%> (-0.66%)` | :arrow_down: |
   
   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/superset/pull/12998?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.63% <ø> (-0.92%)` | :arrow_down: |
   | [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `58.86% <14.28%> (-0.19%)` | :arrow_down: |
   | [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [superset/views/database/views.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.50% <0.00%> (-25.07%)` | :arrow_down: |
   | [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `91.66% <0.00%> (-8.34%)` | :arrow_down: |
   | [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
   | [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
   | [superset/sql\_validators/base.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
   | [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
   | [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `79.85% <0.00%> (-6.15%)` | :arrow_down: |
   | ... and [570 more](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12998?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/superset/pull/12998?src=pr&el=footer). Last update [76c225d...eeec214](https://codecov.io/gh/apache/superset/pull/12998?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] [superset] nikolagigic commented on a change in pull request #12998: feat(explore): Execute predicate query string if set

Posted by GitBox <gi...@apache.org>.
nikolagigic commented on a change in pull request #12998:
URL: https://github.com/apache/superset/pull/12998#discussion_r573876990



##########
File path: superset/viz.py
##########
@@ -177,6 +177,11 @@ def process_metrics(self) -> None:
         self.all_metrics = list(self.metric_dict.values())
         self.metric_labels = list(self.metric_dict.keys())
 
+    def construct_where(self, query_obj: QueryObjectDict, predicate_string: str) -> str:
+        if query_obj:
+            predicate_string = "({}) AND ({})".format(query_obj, predicate_string)

Review comment:
       @villebro makes sense. Thanks for the catch!




----------------------------------------------------------------
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] [superset] stale[bot] commented on pull request #12998: feat(explore): Execute predicate query string if set

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #12998:
URL: https://github.com/apache/superset/pull/12998#issuecomment-859200944


   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] [superset] ktmud commented on pull request #12998: feat(explore): Execute predicate query string if set

Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12998:
URL: https://github.com/apache/superset/pull/12998#issuecomment-779516758


   From the code it seems this only applies to `FilterBox`, but the screenshot shows histogram. Is it only for demonstration's purpose?
   
   The autocomplete predicate query string is used for this dropdown:
   
   <img src="https://user-images.githubusercontent.com/335541/108005026-65541780-6fac-11eb-97c9-d656bec24482.png" width="400">
   
   which then calls [`/filter/<datasource_type>/<datasource_id>/<column>`](https://github.com/apache/superset/blob/d7cbd53fce2c6ec2301d99bfda0b1969e5ec36e5/superset/views/core.py#L836) and [`values_for_column`](https://github.com/apache/superset/blob/eeec2140656ed862531252c69dfd616a91f8ce74/superset/connectors/sqla/models.py#L723). I'm wondering whether we should just reuse this API for FilterBox, too?
   
   


----------------------------------------------------------------
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] [superset] codecov-io edited a comment on pull request #12998: feat(explore): Execute predicate query string if set

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12998:
URL: https://github.com/apache/superset/pull/12998#issuecomment-776723646


   # [Codecov](https://codecov.io/gh/apache/superset/pull/12998?src=pr&el=h1) Report
   > Merging [#12998](https://codecov.io/gh/apache/superset/pull/12998?src=pr&el=desc) (fd0d1f6) into [master](https://codecov.io/gh/apache/superset/commit/76c225db7ee464d5b642cf7e0aa6c990df88f99d?el=desc) (76c225d) will **decrease** coverage by `2.27%`.
   > The diff coverage is `14.28%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12998/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12998?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12998      +/-   ##
   ==========================================
   - Coverage   69.14%   66.86%   -2.28%     
   ==========================================
     Files        1025      490     -535     
     Lines       48765    28863   -19902     
     Branches     5188        0    -5188     
   ==========================================
   - Hits        33718    19300   -14418     
   + Misses      14913     9563    -5350     
   + Partials      134        0     -134     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `66.86% <14.28%> (-0.76%)` | :arrow_down: |
   
   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/superset/pull/12998?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `58.86% <14.28%> (-0.19%)` | :arrow_down: |
   | [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
   | [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-16.93%)` | :arrow_down: |
   | [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `91.66% <0.00%> (-8.34%)` | :arrow_down: |
   | [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
   | [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
   | [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `84.55% <0.00%> (-6.00%)` | :arrow_down: |
   | [superset/dashboards/commands/update.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9jb21tYW5kcy91cGRhdGUucHk=) | `83.07% <0.00%> (-5.61%)` | :arrow_down: |
   | ... and [570 more](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12998?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/superset/pull/12998?src=pr&el=footer). Last update [76c225d...fd0d1f6](https://codecov.io/gh/apache/superset/pull/12998?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] [superset] codecov-io edited a comment on pull request #12998: feat(explore): Execute predicate query string if set

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12998:
URL: https://github.com/apache/superset/pull/12998#issuecomment-776723646


   # [Codecov](https://codecov.io/gh/apache/superset/pull/12998?src=pr&el=h1) Report
   > Merging [#12998](https://codecov.io/gh/apache/superset/pull/12998?src=pr&el=desc) (eeec214) into [master](https://codecov.io/gh/apache/superset/commit/76c225db7ee464d5b642cf7e0aa6c990df88f99d?el=desc) (76c225d) will **decrease** coverage by `1.88%`.
   > The diff coverage is `14.28%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12998/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12998?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12998      +/-   ##
   ==========================================
   - Coverage   69.14%   67.25%   -1.89%     
   ==========================================
     Files        1025      489     -536     
     Lines       48765    28761   -20004     
     Branches     5188        0    -5188     
   ==========================================
   - Hits        33718    19343   -14375     
   + Misses      14913     9418    -5495     
   + Partials      134        0     -134     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `67.25% <14.28%> (-0.38%)` | :arrow_down: |
   
   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/superset/pull/12998?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `58.86% <14.28%> (-0.19%)` | :arrow_down: |
   | [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
   | [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-16.93%)` | :arrow_down: |
   | [superset/dataframe.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWZyYW1lLnB5) | `91.66% <0.00%> (-8.34%)` | :arrow_down: |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.93%)` | :arrow_down: |
   | [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
   | [superset/dashboards/commands/update.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9jb21tYW5kcy91cGRhdGUucHk=) | `83.07% <0.00%> (-5.61%)` | :arrow_down: |
   | [superset/utils/decorators.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvZGVjb3JhdG9ycy5weQ==) | `94.44% <0.00%> (-5.56%)` | :arrow_down: |
   | [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `86.20% <0.00%> (-3.45%)` | :arrow_down: |
   | [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `89.79% <0.00%> (-2.05%)` | :arrow_down: |
   | ... and [566 more](https://codecov.io/gh/apache/superset/pull/12998/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12998?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/superset/pull/12998?src=pr&el=footer). Last update [76c225d...eeec214](https://codecov.io/gh/apache/superset/pull/12998?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] [superset] nikolagigic commented on pull request #12998: feat(explore): Execute predicate query string if set

Posted by GitBox <gi...@apache.org>.
nikolagigic commented on pull request #12998:
URL: https://github.com/apache/superset/pull/12998#issuecomment-775198787


   @junlincc @eugeniamz @villebro 


----------------------------------------------------------------
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] [superset] stale[bot] closed pull request #12998: feat(explore): Execute predicate query string if set

Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #12998:
URL: https://github.com/apache/superset/pull/12998


   


-- 
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 change in pull request #12998: feat(explore): Execute predicate query string if set

Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #12998:
URL: https://github.com/apache/superset/pull/12998#discussion_r573727487



##########
File path: superset/viz.py
##########
@@ -177,6 +177,11 @@ def process_metrics(self) -> None:
         self.all_metrics = list(self.metric_dict.values())
         self.metric_labels = list(self.metric_dict.keys())
 
+    def construct_where(self, query_obj: QueryObjectDict, predicate_string: str) -> str:
+        if query_obj:
+            predicate_string = "{} AND {}".format(query_obj, predicate_string)

Review comment:
       We probably need to add parenthesis here, if either of these has an OR it will break.
   ```suggestion
               predicate_string = "({}) AND ({})".format(query_obj, predicate_string)
   ```




----------------------------------------------------------------
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