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/01/25 19:51:11 UTC
[GitHub] [superset] graceguo-supercat opened a new pull request #12742: fix: Prevent dashboard using filter_values to add incompatible indicator
graceguo-supercat opened a new pull request #12742:
URL: https://github.com/apache/superset/pull/12742
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### 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.
- [ ] 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] graceguo-supercat commented on a change in pull request #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r565036307
##########
File path: superset/viz.py
##########
@@ -474,17 +480,30 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
filters = self.form_data.get("filters", [])
filter_columns = [flt.get("col") for flt in filters]
columns = set(self.datasource.column_names)
+ filter_values_columns = []
+
+ # if using virtual datasource, check filter_values
+ if self.datasource.sql:
+ statements_without_comments = sqlparse.format(
Review comment:
thanks John! I didn't know the sql parser is this powerful. But i will go with solution 1). I just feel traverse the statement tree for this small thing is overkill (and probably slower than simple regex).
----------------------------------------------------------------
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] john-bodley commented on a change in pull request #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r564992134
##########
File path: superset/viz.py
##########
@@ -474,17 +480,30 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
filters = self.form_data.get("filters", [])
filter_columns = [flt.get("col") for flt in filters]
columns = set(self.datasource.column_names)
+ filter_values_columns = []
+
+ # if using virtual datasource, check filter_values
+ if self.datasource.sql:
+ statements_without_comments = sqlparse.format(
Review comment:
Because we're doing regex of the SQL statement (and not actual parsing of the statement) this seems somewhat of a hybrid approach. I would likely either:
1. Don't worry about parsing the statement and just run the regex logic on the full SQL statement.
2. Leverage `sqlparse` to actually identify the `filter_value` function and the items in the identifier list, i.e.,
```
stmt._pprint_tree()
...
| |- 7 Function 'filter...'
| | |- 0 Identifier 'filter...'
| | | `- 0 Name 'filter...'
| | `- 1 Parenthesis '('my_d...'
| | |- 0 Punctuation '('
| | |- 1 IdentifierList ''my_da...'
| | | |- 0 Single ''my_da...'
| | | |- 1 Punctuation ','
| | | |- 2 Whitespace ' '
| | | `- 3 Single ''birth...'
| | `- 2 Punctuation ')'
...
```
----------------------------------------------------------------
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 a change in pull request #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r564059729
##########
File path: superset/viz.py
##########
@@ -474,6 +475,18 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
filters = self.form_data.get("filters", [])
filter_columns = [flt.get("col") for flt in filters]
columns = set(self.datasource.column_names)
+ filter_values_columns = []
+
+ # if using virtual datasource, check filter_values
+ sql = self.datasource.sql
+ if sql:
+ statements_without_comments = sqlparse.format(sql, strip_comments=True)
+ parsed_query = sql_parse.ParsedQuery(statements_without_comments)
+ for stmt in parsed_query.get_statements():
+ filter_values_columns += (
+ re.findall(r"filter_values\(\"(\w+)\"\,", stmt)
Review comment:
Maybe make the RegExp a constant? It looks quite random without enough context. Newcomers will have a real hard time understanding this is somehow related to Jinja template.
##########
File path: superset/viz.py
##########
@@ -484,7 +497,7 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
payload["rejected_filters"] = [
{"reason": "not_in_datasource", "column": col}
for col in filter_columns
- if col not in columns
+ if col not in columns and col not in filter_values_columns
Review comment:
Should they be added to `applied_filters` if removed from `rejected_filters`?
----------------------------------------------------------------
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 #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
----------------------------------------------------------------
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] john-bodley commented on a change in pull request #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r565139486
##########
File path: superset/viz.py
##########
@@ -474,17 +480,30 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
filters = self.form_data.get("filters", [])
filter_columns = [flt.get("col") for flt in filters]
columns = set(self.datasource.column_names)
+ filter_values_columns = []
+
+ # if using virtual datasource, check filter_values
+ if self.datasource.sql:
+ statements_without_comments = sqlparse.format(
Review comment:
I agree.
----------------------------------------------------------------
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] graceguo-supercat commented on a change in pull request #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r565494359
##########
File path: superset/viz.py
##########
@@ -102,6 +102,11 @@
"size",
]
+# This regex is to get user defined filter column name, which is the first param in the filter_values function.
+# see the definition of filter_values template:
+# https://github.com/apache/superset/blob/24ad6063d736c1f38ad6f962e586b9b1a21946af/superset/jinja_context.py#L63
+FILTER_VALUES_REGEX = re.compile(r"filter_values\(['\"](\w+)['\"]\,")
Review comment:
no. filter_values could be introduced by some macro (and nested in another function etc):
```
# define other functions...
{%- set f_metric = f_metric or filter_values("sel_metric", "no_sel_metric_specified") -%}
{%- set f_aggregation = f_aggregation or filter_values("sel_aggregation", "no_aggregation_specified") -%}
SELECT
version AS sel_fcst_version
FROM
core_guest.active_booker_metrics_forecast_countries
WHERE
metric = '{{ f2t(f_metric) }}'
```
----------------------------------------------------------------
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] john-bodley commented on a change in pull request #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r564111690
##########
File path: superset/viz.py
##########
@@ -474,6 +475,18 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
filters = self.form_data.get("filters", [])
filter_columns = [flt.get("col") for flt in filters]
columns = set(self.datasource.column_names)
+ filter_values_columns = []
+
+ # if using virtual datasource, check filter_values
+ sql = self.datasource.sql
Review comment:
Note I don't think this indirection is necessary.
----------------------------------------------------------------
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 #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
# [Codecov](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=h1) Report
> Merging [#12742](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=desc) (5e39107) into [master](https://codecov.io/gh/apache/superset/commit/9e58eb809edf7aefac9dd86bf7e4e45f03acac6e?el=desc) (9e58eb8) will **decrease** coverage by `3.35%`.
> The diff coverage is `55.55%`.
[](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12742 +/- ##
==========================================
- Coverage 66.73% 63.38% -3.36%
==========================================
Files 1021 488 -533
Lines 49967 30110 -19857
Branches 4890 0 -4890
==========================================
- Hits 33347 19085 -14262
+ Misses 16491 11025 -5466
+ Partials 129 0 -129
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.38% <55.55%> (-0.62%)` | :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/12742?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.02% <55.55%> (-0.05%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `32.65% <0.00%> (-59.19%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-22.81%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-10.00%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.71%)` | :arrow_down: |
| [superset/databases/schemas.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `93.40% <0.00%> (-6.05%)` | :arrow_down: |
| [superset/databases/dao.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2Rhby5weQ==) | `94.11% <0.00%> (-5.89%)` | :arrow_down: |
| ... and [565 more](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12742?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/12742?src=pr&el=footer). Last update [9e58eb8...5e39107](https://codecov.io/gh/apache/superset/pull/12742?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] john-bodley commented on a change in pull request #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r564111690
##########
File path: superset/viz.py
##########
@@ -474,6 +475,18 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
filters = self.form_data.get("filters", [])
filter_columns = [flt.get("col") for flt in filters]
columns = set(self.datasource.column_names)
+ filter_values_columns = []
+
+ # if using virtual datasource, check filter_values
+ sql = self.datasource.sql
Review comment:
Note I don't think this indirection is necessary.
----------------------------------------------------------------
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] jkoj52jkoj52 commented on pull request #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
jkoj52jkoj52 commented on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767131087
Hi
----------------------------------------------------------------
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 #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
# [Codecov](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=h1) Report
> Merging [#12742](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=desc) (d25cbb8) into [master](https://codecov.io/gh/apache/superset/commit/9e58eb809edf7aefac9dd86bf7e4e45f03acac6e?el=desc) (9e58eb8) will **decrease** coverage by `2.94%`.
> The diff coverage is `55.55%`.
[](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12742 +/- ##
==========================================
- Coverage 66.73% 63.79% -2.95%
==========================================
Files 1021 488 -533
Lines 49967 30125 -19842
Branches 4890 0 -4890
==========================================
- Hits 33347 19218 -14129
+ Misses 16491 10907 -5584
+ Partials 129 0 -129
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.79% <55.55%> (-0.21%)` | :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/12742?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.02% <55.55%> (-0.05%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12742/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/12742/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/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/databases/commands/test\_connection.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3Rlc3RfY29ubmVjdGlvbi5weQ==) | `84.78% <0.00%> (-4.35%)` | :arrow_down: |
| [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `96.42% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.86% <0.00%> (-2.99%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `72.93% <0.00%> (-2.37%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `89.79% <0.00%> (-2.05%)` | :arrow_down: |
| ... and [553 more](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12742?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/12742?src=pr&el=footer). Last update [9e58eb8...d25cbb8](https://codecov.io/gh/apache/superset/pull/12742?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 #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
# [Codecov](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=h1) Report
> Merging [#12742](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=desc) (f4df867) into [master](https://codecov.io/gh/apache/superset/commit/9e58eb809edf7aefac9dd86bf7e4e45f03acac6e?el=desc) (9e58eb8) will **decrease** coverage by `3.36%`.
> The diff coverage is `55.55%`.
[](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12742 +/- ##
==========================================
- Coverage 66.73% 63.37% -3.37%
==========================================
Files 1021 488 -533
Lines 49967 30112 -19855
Branches 4890 0 -4890
==========================================
- Hits 33347 19083 -14264
+ Misses 16491 11029 -5462
+ Partials 129 0 -129
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.37% <55.55%> (-0.63%)` | :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/12742?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.02% <55.55%> (-0.05%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12742/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/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.69% <0.00%> (-24.88%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12742/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/12742/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/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `79.59% <0.00%> (-6.38%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `82.25% <0.00%> (-5.85%)` | :arrow_down: |
| [superset/views/database/forms.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvZm9ybXMucHk=) | `83.33% <0.00%> (-5.56%)` | :arrow_down: |
| ... and [547 more](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12742?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/12742?src=pr&el=footer). Last update [9e58eb8...f4df867](https://codecov.io/gh/apache/superset/pull/12742?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] graceguo-supercat commented on a change in pull request #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r565494359
##########
File path: superset/viz.py
##########
@@ -102,6 +102,11 @@
"size",
]
+# This regex is to get user defined filter column name, which is the first param in the filter_values function.
+# see the definition of filter_values template:
+# https://github.com/apache/superset/blob/24ad6063d736c1f38ad6f962e586b9b1a21946af/superset/jinja_context.py#L63
+FILTER_VALUES_REGEX = re.compile(r"filter_values\(['\"](\w+)['\"]\,")
Review comment:
no. filter_values could be introduced by some macro (and nested in another function etc):
```
...
{%- set f_metric = f_metric or filter_values("sel_metric", "no_sel_metric_specified") -%}
{%- set f_aggregation = f_aggregation or filter_values("sel_aggregation", "no_aggregation_specified") -%}
SELECT
version AS sel_fcst_version
FROM
core_guest.active_booker_metrics_forecast_countries
WHERE
metric = '{{ f2t(f_metric) }}'
...
```
----------------------------------------------------------------
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 #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
# [Codecov](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=h1) Report
> Merging [#12742](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=desc) (aedf42d) into [master](https://codecov.io/gh/apache/superset/commit/9e58eb809edf7aefac9dd86bf7e4e45f03acac6e?el=desc) (9e58eb8) will **decrease** coverage by `3.83%`.
> The diff coverage is `75.00%`.
[](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12742 +/- ##
==========================================
- Coverage 66.73% 62.90% -3.84%
==========================================
Files 1021 1022 +1
Lines 49967 50034 +67
Branches 4890 4914 +24
==========================================
- Hits 33347 31472 -1875
- Misses 16491 18362 +1871
- Partials 129 200 +71
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `61.68% <ø> (+0.77%)` | :arrow_up: |
| python | `63.70% <75.00%> (-0.29%)` | :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/12742?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.10% <75.00%> (+0.03%)` | :arrow_up: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/reducers/index.js](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvaW5kZXguanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/dashboard/containers/Dashboard.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/filters/components/Select/types.ts](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvdHlwZXMudHM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/dashboard/containers/SliceAdder.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL1NsaWNlQWRkZXIuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/explore/reducers/getInitialState.js](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvZ2V0SW5pdGlhbFN0YXRlLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...-frontend/src/visualizations/presets/MainPreset.js](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL3ByZXNldHMvTWFpblByZXNldC5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [219 more](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12742?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/12742?src=pr&el=footer). Last update [9e58eb8...aedf42d](https://codecov.io/gh/apache/superset/pull/12742?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 #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
# [Codecov](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=h1) Report
> Merging [#12742](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=desc) (5e39107) into [master](https://codecov.io/gh/apache/superset/commit/9e58eb809edf7aefac9dd86bf7e4e45f03acac6e?el=desc) (9e58eb8) will **increase** coverage by `0.04%`.
> The diff coverage is `55.55%`.
[](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12742 +/- ##
==========================================
+ Coverage 66.73% 66.78% +0.04%
==========================================
Files 1021 1022 +1
Lines 49967 50057 +90
Branches 4890 4914 +24
==========================================
+ Hits 33347 33432 +85
- Misses 16491 16501 +10
+ Partials 129 124 -5
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `50.88% <ø> (+0.36%)` | :arrow_up: |
| javascript | `61.68% <ø> (+0.77%)` | :arrow_up: |
| python | `63.73% <55.55%> (-0.26%)` | :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/12742?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.02% <55.55%> (-0.05%)` | :arrow_down: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12742/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/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-10.00%)` | :arrow_down: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `75.67% <0.00%> (-7.21%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.71%)` | :arrow_down: |
| [...et-frontend/src/components/Menu/LanguagePicker.tsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTWVudS9MYW5ndWFnZVBpY2tlci50c3g=) | `64.28% <0.00%> (-4.95%)` | :arrow_down: |
| [superset-frontend/src/utils/common.js](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3V0aWxzL2NvbW1vbi5qcw==) | `70.96% <0.00%> (-3.23%)` | :arrow_down: |
| [...nd/src/explore/components/ExploreViewContainer.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlVmlld0NvbnRhaW5lci5qc3g=) | `77.24% <0.00%> (-2.76%)` | :arrow_down: |
| [...rontend/src/explore/components/DatasourcePanel.tsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9EYXRhc291cmNlUGFuZWwudHN4) | `80.00% <0.00%> (-2.61%)` | :arrow_down: |
| [...perset-frontend/src/views/CRUD/chart/ChartList.tsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvY2hhcnQvQ2hhcnRMaXN0LnRzeA==) | `79.52% <0.00%> (-2.37%)` | :arrow_down: |
| ... and [57 more](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12742?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/12742?src=pr&el=footer). Last update [9e58eb8...5e39107](https://codecov.io/gh/apache/superset/pull/12742?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 #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
# [Codecov](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=h1) Report
> Merging [#12742](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=desc) (aedf42d) into [master](https://codecov.io/gh/apache/superset/commit/9e58eb809edf7aefac9dd86bf7e4e45f03acac6e?el=desc) (9e58eb8) will **increase** coverage by `0.05%`.
> The diff coverage is `75.00%`.
[](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12742 +/- ##
==========================================
+ Coverage 66.73% 66.78% +0.05%
==========================================
Files 1021 1022 +1
Lines 49967 50053 +86
Branches 4890 4914 +24
==========================================
+ Hits 33347 33430 +83
- Misses 16491 16499 +8
+ Partials 129 124 -5
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `50.87% <ø> (+0.36%)` | :arrow_up: |
| javascript | `61.68% <ø> (+0.77%)` | :arrow_up: |
| python | `63.74% <75.00%> (-0.26%)` | :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/12742?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.10% <75.00%> (+0.03%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12742/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/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-10.00%)` | :arrow_down: |
| [...ntend/src/dashboard/components/PropertiesModal.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1Byb3BlcnRpZXNNb2RhbC5qc3g=) | `75.67% <0.00%> (-7.21%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.71%)` | :arrow_down: |
| [...et-frontend/src/components/Menu/LanguagePicker.tsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTWVudS9MYW5ndWFnZVBpY2tlci50c3g=) | `64.28% <0.00%> (-4.95%)` | :arrow_down: |
| [superset-frontend/src/utils/common.js](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3V0aWxzL2NvbW1vbi5qcw==) | `70.96% <0.00%> (-3.23%)` | :arrow_down: |
| [...nd/src/explore/components/ExploreViewContainer.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlVmlld0NvbnRhaW5lci5qc3g=) | `77.24% <0.00%> (-2.76%)` | :arrow_down: |
| [...rontend/src/explore/components/DatasourcePanel.tsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9EYXRhc291cmNlUGFuZWwudHN4) | `80.00% <0.00%> (-2.61%)` | :arrow_down: |
| [...perset-frontend/src/views/CRUD/chart/ChartList.tsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvY2hhcnQvQ2hhcnRMaXN0LnRzeA==) | `79.52% <0.00%> (-2.37%)` | :arrow_down: |
| ... and [58 more](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12742?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/12742?src=pr&el=footer). Last update [9e58eb8...aedf42d](https://codecov.io/gh/apache/superset/pull/12742?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 commented on pull request #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
# [Codecov](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=h1) Report
> Merging [#12742](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=desc) (f4df867) into [master](https://codecov.io/gh/apache/superset/commit/9e58eb809edf7aefac9dd86bf7e4e45f03acac6e?el=desc) (9e58eb8) will **decrease** coverage by `3.36%`.
> The diff coverage is `55.55%`.
[](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12742 +/- ##
==========================================
- Coverage 66.73% 63.37% -3.37%
==========================================
Files 1021 488 -533
Lines 49967 30110 -19857
Branches 4890 0 -4890
==========================================
- Hits 33347 19081 -14266
+ Misses 16491 11029 -5462
+ Partials 129 0 -129
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.37% <55.55%> (-0.63%)` | :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/12742?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.02% <55.55%> (-0.05%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12742/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/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.50% <0.00%> (-25.07%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12742/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/12742/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/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `79.59% <0.00%> (-6.38%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `82.25% <0.00%> (-5.85%)` | :arrow_down: |
| [superset/views/database/forms.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvZm9ybXMucHk=) | `83.33% <0.00%> (-5.56%)` | :arrow_down: |
| ... and [548 more](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12742?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/12742?src=pr&el=footer). Last update [9e58eb8...f4df867](https://codecov.io/gh/apache/superset/pull/12742?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] suddjian commented on a change in pull request #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r564127506
##########
File path: superset/viz.py
##########
@@ -474,6 +475,18 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
filters = self.form_data.get("filters", [])
filter_columns = [flt.get("col") for flt in filters]
columns = set(self.datasource.column_names)
+ filter_values_columns = []
+
+ # if using virtual datasource, check filter_values
+ sql = self.datasource.sql
+ if sql:
+ statements_without_comments = sqlparse.format(sql, strip_comments=True)
+ parsed_query = sql_parse.ParsedQuery(statements_without_comments)
+ for stmt in parsed_query.get_statements():
+ filter_values_columns += (
+ re.findall(r"filter_values\(\"(\w+)\"\,", stmt)
Review comment:
Can we get a comment also explaining what this is doing and why?
----------------------------------------------------------------
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 #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
# [Codecov](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=h1) Report
> Merging [#12742](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=desc) (5e39107) into [master](https://codecov.io/gh/apache/superset/commit/9e58eb809edf7aefac9dd86bf7e4e45f03acac6e?el=desc) (9e58eb8) will **decrease** coverage by `3.81%`.
> The diff coverage is `55.55%`.
[](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12742 +/- ##
==========================================
- Coverage 66.73% 62.92% -3.82%
==========================================
Files 1021 1022 +1
Lines 49967 50038 +71
Branches 4890 4914 +24
==========================================
- Hits 33347 31484 -1863
- Misses 16491 18354 +1863
- Partials 129 200 +71
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `61.68% <ø> (+0.77%)` | :arrow_up: |
| python | `63.73% <55.55%> (-0.26%)` | :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/12742?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.02% <55.55%> (-0.05%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/reducers/index.js](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvaW5kZXguanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/dashboard/containers/Dashboard.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/filters/components/Select/types.ts](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvdHlwZXMudHM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/dashboard/containers/SliceAdder.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL1NsaWNlQWRkZXIuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/explore/reducers/getInitialState.js](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvZ2V0SW5pdGlhbFN0YXRlLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...-frontend/src/visualizations/presets/MainPreset.js](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL3ByZXNldHMvTWFpblByZXNldC5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [217 more](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12742?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/12742?src=pr&el=footer). Last update [9e58eb8...5e39107](https://codecov.io/gh/apache/superset/pull/12742?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 commented on pull request #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
# [Codecov](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=h1) Report
> Merging [#12742](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=desc) (f4df867) into [master](https://codecov.io/gh/apache/superset/commit/9e58eb809edf7aefac9dd86bf7e4e45f03acac6e?el=desc) (9e58eb8) will **decrease** coverage by `3.36%`.
> The diff coverage is `55.55%`.
[](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12742 +/- ##
==========================================
- Coverage 66.73% 63.37% -3.37%
==========================================
Files 1021 488 -533
Lines 49967 30110 -19857
Branches 4890 0 -4890
==========================================
- Hits 33347 19081 -14266
+ Misses 16491 11029 -5462
+ Partials 129 0 -129
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.37% <55.55%> (-0.63%)` | :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/12742?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.02% <55.55%> (-0.05%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12742/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/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.50% <0.00%> (-25.07%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12742/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/12742/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/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `79.59% <0.00%> (-6.38%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `82.25% <0.00%> (-5.85%)` | :arrow_down: |
| [superset/views/database/forms.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvZm9ybXMucHk=) | `83.33% <0.00%> (-5.56%)` | :arrow_down: |
| ... and [548 more](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12742?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/12742?src=pr&el=footer). Last update [9e58eb8...f4df867](https://codecov.io/gh/apache/superset/pull/12742?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 #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
# [Codecov](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=h1) Report
> Merging [#12742](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=desc) (5e39107) into [master](https://codecov.io/gh/apache/superset/commit/9e58eb809edf7aefac9dd86bf7e4e45f03acac6e?el=desc) (9e58eb8) will **decrease** coverage by `3.83%`.
> The diff coverage is `55.55%`.
[](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12742 +/- ##
==========================================
- Coverage 66.73% 62.89% -3.84%
==========================================
Files 1021 1022 +1
Lines 49967 50038 +71
Branches 4890 4914 +24
==========================================
- Hits 33347 31473 -1874
- Misses 16491 18365 +1874
- Partials 129 200 +71
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `61.68% <ø> (+0.77%)` | :arrow_up: |
| python | `63.70% <55.55%> (-0.30%)` | :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/12742?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.02% <55.55%> (-0.05%)` | :arrow_down: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/reducers/index.js](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvaW5kZXguanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/dashboard/containers/Dashboard.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/filters/components/Select/types.ts](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvdHlwZXMudHM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/dashboard/containers/SliceAdder.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL1NsaWNlQWRkZXIuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/explore/reducers/getInitialState.js](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvZ2V0SW5pdGlhbFN0YXRlLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...-frontend/src/visualizations/presets/MainPreset.js](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL3ByZXNldHMvTWFpblByZXNldC5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [219 more](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12742?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/12742?src=pr&el=footer). Last update [9e58eb8...5e39107](https://codecov.io/gh/apache/superset/pull/12742?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 #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
# [Codecov](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=h1) Report
> Merging [#12742](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=desc) (d25cbb8) into [master](https://codecov.io/gh/apache/superset/commit/9e58eb809edf7aefac9dd86bf7e4e45f03acac6e?el=desc) (9e58eb8) will **decrease** coverage by `3.35%`.
> The diff coverage is `55.55%`.
[](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12742 +/- ##
==========================================
- Coverage 66.73% 63.38% -3.36%
==========================================
Files 1021 488 -533
Lines 49967 30123 -19844
Branches 4890 0 -4890
==========================================
- Hits 33347 19094 -14253
+ Misses 16491 11029 -5462
+ Partials 129 0 -129
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.38% <55.55%> (-0.61%)` | :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/12742?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.02% <55.55%> (-0.05%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12742/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/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.50% <0.00%> (-25.07%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12742/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/12742/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/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `79.59% <0.00%> (-6.38%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `82.25% <0.00%> (-5.85%)` | :arrow_down: |
| [superset/views/database/forms.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvZm9ybXMucHk=) | `83.33% <0.00%> (-5.56%)` | :arrow_down: |
| ... and [557 more](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12742?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/12742?src=pr&el=footer). Last update [9e58eb8...d25cbb8](https://codecov.io/gh/apache/superset/pull/12742?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] graceguo-supercat commented on a change in pull request #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r565030685
##########
File path: superset/viz.py
##########
@@ -102,6 +103,8 @@
"size",
]
+FILTER_VALUES_REGEX = re.compile(r"FILTER_VALUES\(['\"](\w+)['\"]\,", re.IGNORECASE)
Review comment:
yes, you are right. filter_values is function not sql statement. I will fix 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.
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 #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
----------------------------------------------------------------
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] graceguo-supercat commented on a change in pull request #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r564939146
##########
File path: superset/viz.py
##########
@@ -474,6 +475,18 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
filters = self.form_data.get("filters", [])
filter_columns = [flt.get("col") for flt in filters]
columns = set(self.datasource.column_names)
+ filter_values_columns = []
+
+ # if using virtual datasource, check filter_values
+ sql = self.datasource.sql
+ if sql:
+ statements_without_comments = sqlparse.format(sql, strip_comments=True)
+ parsed_query = sql_parse.ParsedQuery(statements_without_comments)
+ for stmt in parsed_query.get_statements():
+ filter_values_columns += (
+ re.findall(r"filter_values\(\"(\w+)\"\,", stmt)
Review comment:
fixed.
##########
File path: superset/viz.py
##########
@@ -484,7 +497,7 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
payload["rejected_filters"] = [
{"reason": "not_in_datasource", "column": col}
for col in filter_columns
- if col not in columns
+ if col not in columns and col not in filter_values_columns
Review comment:
fixed.
##########
File path: superset/viz.py
##########
@@ -474,6 +475,18 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
filters = self.form_data.get("filters", [])
filter_columns = [flt.get("col") for flt in filters]
columns = set(self.datasource.column_names)
+ filter_values_columns = []
+
+ # if using virtual datasource, check filter_values
+ sql = self.datasource.sql
Review comment:
fixed.
----------------------------------------------------------------
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 #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
----------------------------------------------------------------
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] graceguo-supercat commented on a change in pull request #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r564939587
##########
File path: superset/viz.py
##########
@@ -102,6 +103,8 @@
"size",
]
+FILTER_VALUES_REGEX = re.compile(r"FILTER_VALUES\(['\"](\w+)['\"]\,", re.IGNORECASE)
Review comment:
case-insensitive, allow `'` and `"`
----------------------------------------------------------------
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 #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
# [Codecov](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=h1) Report
> Merging [#12742](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=desc) (aedf42d) into [master](https://codecov.io/gh/apache/superset/commit/9e58eb809edf7aefac9dd86bf7e4e45f03acac6e?el=desc) (9e58eb8) will **decrease** coverage by `3.03%`.
> The diff coverage is `75.00%`.
[](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12742 +/- ##
==========================================
- Coverage 66.73% 63.70% -3.04%
==========================================
Files 1021 488 -533
Lines 49967 30121 -19846
Branches 4890 0 -4890
==========================================
- Hits 33347 19189 -14158
+ Misses 16491 10932 -5559
+ Partials 129 0 -129
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.70% <75.00%> (-0.29%)` | :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/12742?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.10% <75.00%> (+0.03%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12742/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/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-10.00%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.71%)` | :arrow_down: |
| [superset/db\_engine\_specs/mysql.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL215c3FsLnB5) | `89.79% <0.00%> (-2.05%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `87.22% <0.00%> (-1.64%)` | :arrow_down: |
| [superset/models/tags.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL3RhZ3MucHk=) | `78.94% <0.00%> (-0.37%)` | :arrow_down: |
| [superset/views/core.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvY29yZS5weQ==) | `74.94% <0.00%> (-0.36%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `90.45% <0.00%> (-0.14%)` | :arrow_down: |
| ... and [543 more](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12742?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/12742?src=pr&el=footer). Last update [9e58eb8...aedf42d](https://codecov.io/gh/apache/superset/pull/12742?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] graceguo-supercat commented on a change in pull request #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r564939587
##########
File path: superset/viz.py
##########
@@ -102,6 +103,8 @@
"size",
]
+FILTER_VALUES_REGEX = re.compile(r"FILTER_VALUES\(['\"](\w+)['\"]\,", re.IGNORECASE)
Review comment:
case-insensitive, allow `'` or `"` in template
----------------------------------------------------------------
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 a change in pull request #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r564987551
##########
File path: superset/viz.py
##########
@@ -102,6 +103,8 @@
"size",
]
+FILTER_VALUES_REGEX = re.compile(r"FILTER_VALUES\(['\"](\w+)['\"]\,", re.IGNORECASE)
Review comment:
I think Jinja context variables/functions are case sensitive. Running `SELECT {{ FILTER_VALUES('region') }};` in SQL Lab gives me `undefined` errors.
----------------------------------------------------------------
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] graceguo-supercat commented on a change in pull request #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r565494359
##########
File path: superset/viz.py
##########
@@ -102,6 +102,11 @@
"size",
]
+# This regex is to get user defined filter column name, which is the first param in the filter_values function.
+# see the definition of filter_values template:
+# https://github.com/apache/superset/blob/24ad6063d736c1f38ad6f962e586b9b1a21946af/superset/jinja_context.py#L63
+FILTER_VALUES_REGEX = re.compile(r"filter_values\(['\"](\w+)['\"]\,")
Review comment:
no. define filter_values function could use another syntax without brackets:
```
...
{%- set f_metric = f_metric or filter_values("sel_metric", "no_sel_metric_specified") -%}
{%- set f_aggregation = f_aggregation or filter_values("sel_aggregation", "no_aggregation_specified") -%}
SELECT
version AS sel_fcst_version
FROM
core_guest.active_booker_metrics_forecast_countries
WHERE
metric = '{{ f2t(f_metric) }}'
...
```
----------------------------------------------------------------
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] graceguo-supercat merged pull request #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
graceguo-supercat merged pull request #12742:
URL: https://github.com/apache/superset/pull/12742
----------------------------------------------------------------
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 #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
----------------------------------------------------------------
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 #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
----------------------------------------------------------------
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] suddjian commented on a change in pull request #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r564127506
##########
File path: superset/viz.py
##########
@@ -474,6 +475,18 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
filters = self.form_data.get("filters", [])
filter_columns = [flt.get("col") for flt in filters]
columns = set(self.datasource.column_names)
+ filter_values_columns = []
+
+ # if using virtual datasource, check filter_values
+ sql = self.datasource.sql
+ if sql:
+ statements_without_comments = sqlparse.format(sql, strip_comments=True)
+ parsed_query = sql_parse.ParsedQuery(statements_without_comments)
+ for stmt in parsed_query.get_statements():
+ filter_values_columns += (
+ re.findall(r"filter_values\(\"(\w+)\"\,", stmt)
Review comment:
Can we get a comment here explaining what this is doing, too?
##########
File path: superset/viz.py
##########
@@ -474,6 +475,18 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
filters = self.form_data.get("filters", [])
filter_columns = [flt.get("col") for flt in filters]
columns = set(self.datasource.column_names)
+ filter_values_columns = []
+
+ # if using virtual datasource, check filter_values
+ sql = self.datasource.sql
+ if sql:
+ statements_without_comments = sqlparse.format(sql, strip_comments=True)
+ parsed_query = sql_parse.ParsedQuery(statements_without_comments)
+ for stmt in parsed_query.get_statements():
+ filter_values_columns += (
+ re.findall(r"filter_values\(\"(\w+)\"\,", stmt)
Review comment:
Can we get a comment also explaining what this is doing and why?
----------------------------------------------------------------
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] suddjian commented on a change in pull request #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r564127506
##########
File path: superset/viz.py
##########
@@ -474,6 +475,18 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
filters = self.form_data.get("filters", [])
filter_columns = [flt.get("col") for flt in filters]
columns = set(self.datasource.column_names)
+ filter_values_columns = []
+
+ # if using virtual datasource, check filter_values
+ sql = self.datasource.sql
+ if sql:
+ statements_without_comments = sqlparse.format(sql, strip_comments=True)
+ parsed_query = sql_parse.ParsedQuery(statements_without_comments)
+ for stmt in parsed_query.get_statements():
+ filter_values_columns += (
+ re.findall(r"filter_values\(\"(\w+)\"\,", stmt)
Review comment:
Can we get a comment here explaining what this is doing, 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] graceguo-supercat commented on a change in pull request #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r565036307
##########
File path: superset/viz.py
##########
@@ -474,17 +480,30 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
filters = self.form_data.get("filters", [])
filter_columns = [flt.get("col") for flt in filters]
columns = set(self.datasource.column_names)
+ filter_values_columns = []
+
+ # if using virtual datasource, check filter_values
+ if self.datasource.sql:
+ statements_without_comments = sqlparse.format(
Review comment:
thanks John! I didn't know the sql parser is this powerful. But i will go with solution 1). I just feel traverse the statement tree for this small thing is a little overkill (and probably slower than simple regex).
----------------------------------------------------------------
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 #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
# [Codecov](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=h1) Report
> Merging [#12742](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=desc) (f4df867) into [master](https://codecov.io/gh/apache/superset/commit/9e58eb809edf7aefac9dd86bf7e4e45f03acac6e?el=desc) (9e58eb8) will **decrease** coverage by `3.36%`.
> The diff coverage is `55.55%`.
[](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12742 +/- ##
==========================================
- Coverage 66.73% 63.37% -3.37%
==========================================
Files 1021 488 -533
Lines 49967 30112 -19855
Branches 4890 0 -4890
==========================================
- Hits 33347 19083 -14264
+ Misses 16491 11029 -5462
+ Partials 129 0 -129
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.37% <55.55%> (-0.63%)` | :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/12742?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.02% <55.55%> (-0.05%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12742/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/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2Uvdmlld3MucHk=) | `62.69% <0.00%> (-24.88%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12742/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/12742/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/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvYmFzZS5weQ==) | `93.33% <0.00%> (-6.67%)` | :arrow_down: |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `79.59% <0.00%> (-6.38%)` | :arrow_down: |
| [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `90.62% <0.00%> (-6.25%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `82.25% <0.00%> (-5.85%)` | :arrow_down: |
| [superset/views/database/forms.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvZm9ybXMucHk=) | `83.33% <0.00%> (-5.56%)` | :arrow_down: |
| ... and [547 more](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12742?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/12742?src=pr&el=footer). Last update [9e58eb8...f4df867](https://codecov.io/gh/apache/superset/pull/12742?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 #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
# [Codecov](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=h1) Report
> Merging [#12742](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=desc) (aedf42d) into [master](https://codecov.io/gh/apache/superset/commit/9e58eb809edf7aefac9dd86bf7e4e45f03acac6e?el=desc) (9e58eb8) will **decrease** coverage by `3.34%`.
> The diff coverage is `75.00%`.
[](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12742 +/- ##
==========================================
- Coverage 66.73% 63.38% -3.35%
==========================================
Files 1021 488 -533
Lines 49967 30106 -19861
Branches 4890 0 -4890
==========================================
- Hits 33347 19084 -14263
+ Misses 16491 11022 -5469
+ Partials 129 0 -129
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `?` | |
| python | `63.38% <75.00%> (-0.61%)` | :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/12742?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.10% <75.00%> (+0.03%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `32.65% <0.00%> (-59.19%)` | :arrow_down: |
| [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `59.64% <0.00%> (-22.81%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `73.84% <0.00%> (-10.00%)` | :arrow_down: |
| [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `81.38% <0.00%> (-6.71%)` | :arrow_down: |
| [superset/databases/schemas.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `93.40% <0.00%> (-6.05%)` | :arrow_down: |
| [superset/databases/dao.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2Rhby5weQ==) | `94.11% <0.00%> (-5.89%)` | :arrow_down: |
| ... and [565 more](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12742?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/12742?src=pr&el=footer). Last update [9e58eb8...aedf42d](https://codecov.io/gh/apache/superset/pull/12742?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 #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767101474
# [Codecov](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=h1) Report
> Merging [#12742](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=desc) (aedf42d) into [master](https://codecov.io/gh/apache/superset/commit/9e58eb809edf7aefac9dd86bf7e4e45f03acac6e?el=desc) (9e58eb8) will **decrease** coverage by `3.81%`.
> The diff coverage is `75.00%`.
[](https://codecov.io/gh/apache/superset/pull/12742?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #12742 +/- ##
==========================================
- Coverage 66.73% 62.92% -3.82%
==========================================
Files 1021 1022 +1
Lines 49967 50034 +67
Branches 4890 4914 +24
==========================================
- Hits 33347 31483 -1864
- Misses 16491 18351 +1860
- Partials 129 200 +71
```
| Flag | Coverage Δ | |
|---|---|---|
| cypress | `?` | |
| javascript | `61.68% <ø> (+0.77%)` | :arrow_up: |
| python | `63.74% <75.00%> (-0.26%)` | :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/12742?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdml6LnB5) | `59.10% <75.00%> (+0.03%)` | :arrow_up: |
| [superset-frontend/src/SqlLab/App.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/App.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvQXBwLmpzeA==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/dashboard/App.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9BcHAuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset-frontend/src/explore/reducers/index.js](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvaW5kZXguanM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/dashboard/containers/Dashboard.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZC5qc3g=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...et-frontend/src/filters/components/Select/types.ts](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvdHlwZXMudHM=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/dashboard/containers/SliceAdder.jsx](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL1NsaWNlQWRkZXIuanN4) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...t-frontend/src/explore/reducers/getInitialState.js](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvcmVkdWNlcnMvZ2V0SW5pdGlhbFN0YXRlLmpz) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...-frontend/src/visualizations/presets/MainPreset.js](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL3ByZXNldHMvTWFpblByZXNldC5qcw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [217 more](https://codecov.io/gh/apache/superset/pull/12742/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12742?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/12742?src=pr&el=footer). Last update [9e58eb8...aedf42d](https://codecov.io/gh/apache/superset/pull/12742?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] ktmud commented on a change in pull request #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r564059729
##########
File path: superset/viz.py
##########
@@ -474,6 +475,18 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
filters = self.form_data.get("filters", [])
filter_columns = [flt.get("col") for flt in filters]
columns = set(self.datasource.column_names)
+ filter_values_columns = []
+
+ # if using virtual datasource, check filter_values
+ sql = self.datasource.sql
+ if sql:
+ statements_without_comments = sqlparse.format(sql, strip_comments=True)
+ parsed_query = sql_parse.ParsedQuery(statements_without_comments)
+ for stmt in parsed_query.get_statements():
+ filter_values_columns += (
+ re.findall(r"filter_values\(\"(\w+)\"\,", stmt)
Review comment:
Maybe make the RegExp a constant? It looks quite random without enough context. Newcomers will have a real hard time understanding this is somehow related to Jinja template.
##########
File path: superset/viz.py
##########
@@ -484,7 +497,7 @@ def get_payload(self, query_obj: Optional[QueryObjectDict] = None) -> VizPayload
payload["rejected_filters"] = [
{"reason": "not_in_datasource", "column": col}
for col in filter_columns
- if col not in columns
+ if col not in columns and col not in filter_values_columns
Review comment:
Should they be added to `applied_filters` if removed from `rejected_filters`?
----------------------------------------------------------------
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] jkoj52jkoj52 commented on pull request #12742: fix: Prevent dashboard with filter_values to cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
jkoj52jkoj52 commented on pull request #12742:
URL: https://github.com/apache/superset/pull/12742#issuecomment-767131087
Hi
----------------------------------------------------------------
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 a change in pull request #12742: fix: Prevent dashboard with filter_values template cause incompatible indicator
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #12742:
URL: https://github.com/apache/superset/pull/12742#discussion_r565114081
##########
File path: superset/viz.py
##########
@@ -102,6 +102,11 @@
"size",
]
+# This regex is to get user defined filter column name, which is the first param in the filter_values function.
+# see the definition of filter_values template:
+# https://github.com/apache/superset/blob/24ad6063d736c1f38ad6f962e586b9b1a21946af/superset/jinja_context.py#L63
+FILTER_VALUES_REGEX = re.compile(r"filter_values\(['\"](\w+)['\"]\,")
Review comment:
Maybe add the Jinja brackets as well?
```suggestion
FILTER_VALUES_REGEX = re.compile(r"""\{\{\s*filter_values\(['"](\w+)['"]\)\s*\}\}""")
```
----------------------------------------------------------------
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