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/12/09 09:25:31 UTC
[GitHub] [superset] villebro opened a new pull request #17702: chore(sql): clean up invalid filter clause exception types
villebro opened a new pull request #17702:
URL: https://github.com/apache/superset/pull/17702
### SUMMARY
Sometimes invalid query object extra parameters cause incorrect results, e.g. an internal server error. These are now handled correctly in the both the legacy and v1 apis.
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] kgabryje commented on a change in pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
kgabryje commented on a change in pull request #17702:
URL: https://github.com/apache/superset/pull/17702#discussion_r765821100
##########
File path: tests/integration_tests/charts/data/api_tests.py
##########
@@ -425,6 +425,30 @@ def test_with_invalid_where_parameter__400(self):
assert rv.status_code == 400
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_with_invalid_where_parameter_closing_unclosed__400(self):
+ self.query_context_payload["queries"][0]["filters"] = []
+ # WHERE-clause closing unclosed parens
+ self.query_context_payload["queries"][0]["extras"][
+ "where"
+ ] = "state = 'CA') OR (state = 'NY'"
+
+ rv = self.post_assert_metric(CHART_DATA_URI, self.query_context_payload, "data")
+
+ assert rv.status_code == 400
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_with_invalid_having_parameter_closing_and_comment__400(self):
+ self.query_context_payload["queries"][0]["filters"] = []
+ # WHERE-clause with unclosed parens
Review comment:
```suggestion
# HAVING-clause with unclosed parens and comment
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ofekisr commented on a change in pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
ofekisr commented on a change in pull request #17702:
URL: https://github.com/apache/superset/pull/17702#discussion_r765677496
##########
File path: superset/sql_parse.py
##########
@@ -378,3 +380,23 @@ def set_or_update_query_limit(self, new_limit: int, force: bool = False) -> str:
for i in statement.tokens:
str_res += str(i.value)
return str_res
+
+
+def validate_filter_clause(clause: str) -> None:
Review comment:
The module and its class responsible are just parsing a query nothing else. I don't like either of the utils functions inside x the module. so I think it should be extracted to a different module
##########
File path: superset/sql_parse.py
##########
@@ -378,3 +380,23 @@ def set_or_update_query_limit(self, new_limit: int, force: bool = False) -> str:
for i in statement.tokens:
str_res += str(i.value)
return str_res
+
+
+def validate_filter_clause(clause: str) -> None:
+ if sqlparse.format(clause, strip_comments=True) != sqlparse.format(clause):
+ raise SupersetQueryParseException("Filter clause contains comment")
+
+ statements = sqlparse.parse(clause)
+ if len(statements) != 1:
Review comment:
same as previous comments
##########
File path: superset/sql_parse.py
##########
@@ -378,3 +380,23 @@ def set_or_update_query_limit(self, new_limit: int, force: bool = False) -> str:
for i in statement.tokens:
str_res += str(i.value)
return str_res
+
+
+def validate_filter_clause(clause: str) -> None:
+ if sqlparse.format(clause, strip_comments=True) != sqlparse.format(clause):
+ raise SupersetQueryParseException("Filter clause contains comment")
+
+ statements = sqlparse.parse(clause)
+ if len(statements) != 1:
+ raise SupersetQueryParseException("Filter clause contains multiple queries")
+ open_parens = 0
+
+ for token in statements[0]:
Review comment:
same as previous comments
##########
File path: superset/sql_parse.py
##########
@@ -378,3 +380,23 @@ def set_or_update_query_limit(self, new_limit: int, force: bool = False) -> str:
for i in statement.tokens:
str_res += str(i.value)
return str_res
+
+
+def validate_filter_clause(clause: str) -> None:
+ if sqlparse.format(clause, strip_comments=True) != sqlparse.format(clause):
+ raise SupersetQueryParseException("Filter clause contains comment")
Review comment:
1. This is not a parsing error but a constrained error for validation
2. IMO I would encapsulate it under a private method that tells me what is validating like _validate_clause_does_not_contains_comments, even the if under encapsulated method "is_caluse_contains_comments
3. Actually the code doesn't tell me why comments in clause are bad, anyway I don't like comments in code so instead of writing a comment describe it I would encapsulate bunch of the same purposes under a method with name describe the purpose
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #17702:
URL: https://github.com/apache/superset/pull/17702#issuecomment-989685202
# [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17702](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ff79357) into [master](https://codecov.io/gh/apache/superset/commit/04e3cfa6077faa84b07948fc6071e94d82a4a000?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (04e3cfa) will **decrease** coverage by `0.16%`.
> The diff coverage is `94.59%`.
[](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17702 +/- ##
==========================================
- Coverage 68.13% 67.96% -0.17%
==========================================
Files 1653 1653
Lines 66247 66283 +36
Branches 7107 7107
==========================================
- Hits 45138 45052 -86
- Misses 19220 19342 +122
Partials 1889 1889
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `82.16% <94.59%> (+0.02%)` | :arrow_up: |
| postgres | `82.16% <94.59%> (+0.01%)` | :arrow_up: |
| presto | `?` | |
| python | `82.24% <94.59%> (-0.39%)` | :arrow_down: |
| sqlite | `81.85% <94.59%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdml6LnB5) | `58.13% <75.00%> (+0.07%)` | :arrow_up: |
| [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `95.08% <100.00%> (+2.01%)` | :arrow_up: |
| [superset/exceptions.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhjZXB0aW9ucy5weQ==) | `91.42% <100.00%> (+0.16%)` | :arrow_up: |
| [superset/sql\_parse.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3BhcnNlLnB5) | `99.48% <100.00%> (+0.04%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `70.27% <0.00%> (-16.99%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.50% <0.00%> (-6.89%)` | :arrow_down: |
| [superset/reports/commands/log\_prune.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9sb2dfcHJ1bmUucHk=) | `85.71% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/commands/importers/v1/utils.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbWFuZHMvaW1wb3J0ZXJzL3YxL3V0aWxzLnB5) | `89.13% <0.00%> (-2.18%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| ... and [7 more](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [04e3cfa...ff79357](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] dpgaspar commented on a change in pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #17702:
URL: https://github.com/apache/superset/pull/17702#discussion_r765806997
##########
File path: superset/viz.py
##########
@@ -373,6 +375,15 @@ def query_obj(self) -> QueryObjectDict: # pylint: disable=too-many-locals
self.from_dttm = from_dttm
self.to_dttm = to_dttm
+ # validate sql filters
+ for param in ("where", "having"):
Review comment:
makes sense!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17702:
URL: https://github.com/apache/superset/pull/17702#issuecomment-989685202
# [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17702](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f704e00) into [master](https://codecov.io/gh/apache/superset/commit/04e3cfa6077faa84b07948fc6071e94d82a4a000?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (04e3cfa) will **decrease** coverage by `0.16%`.
> The diff coverage is `94.44%`.
> :exclamation: Current head f704e00 differs from pull request most recent head bbe8ca7. Consider uploading reports for the commit bbe8ca7 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17702 +/- ##
==========================================
- Coverage 68.13% 67.97% -0.17%
==========================================
Files 1653 1653
Lines 66247 66282 +35
Branches 7107 7107
==========================================
- Hits 45138 45055 -83
- Misses 19220 19338 +118
Partials 1889 1889
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `82.16% <94.44%> (+0.02%)` | :arrow_up: |
| postgres | `82.17% <94.44%> (+0.03%)` | :arrow_up: |
| presto | `?` | |
| python | `82.25% <94.44%> (-0.38%)` | :arrow_down: |
| sqlite | `81.85% <94.44%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdml6LnB5) | `58.13% <75.00%> (+0.07%)` | :arrow_up: |
| [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `95.62% <100.00%> (+2.56%)` | :arrow_up: |
| [superset/exceptions.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhjZXB0aW9ucy5weQ==) | `91.42% <100.00%> (+0.16%)` | :arrow_up: |
| [superset/sql\_parse.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3BhcnNlLnB5) | `99.48% <100.00%> (+0.04%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `70.27% <0.00%> (-16.99%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.50% <0.00%> (-6.89%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `86.85% <0.00%> (-1.58%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `89.26% <0.00%> (-0.74%)` | :arrow_down: |
| ... and [4 more](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [04e3cfa...bbe8ca7](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro commented on a change in pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #17702:
URL: https://github.com/apache/superset/pull/17702#discussion_r765795306
##########
File path: superset/viz.py
##########
@@ -373,6 +375,15 @@ def query_obj(self) -> QueryObjectDict: # pylint: disable=too-many-locals
self.from_dttm = from_dttm
self.to_dttm = to_dttm
+ # validate sql filters
+ for param in ("where", "having"):
Review comment:
I know.. I considered splitting this out into a util, but then decided not to as I'm still hopeful we can remove this file some day, in which case the util would no longer be needed. But happy to follow up and DRY it up if people feel strongly about this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17702:
URL: https://github.com/apache/superset/pull/17702#issuecomment-989685202
# [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17702](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f704e00) into [master](https://codecov.io/gh/apache/superset/commit/04e3cfa6077faa84b07948fc6071e94d82a4a000?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (04e3cfa) will **decrease** coverage by `0.16%`.
> The diff coverage is `94.44%`.
> :exclamation: Current head f704e00 differs from pull request most recent head e45cb52. Consider uploading reports for the commit e45cb52 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17702 +/- ##
==========================================
- Coverage 68.13% 67.97% -0.17%
==========================================
Files 1653 1653
Lines 66247 66282 +35
Branches 7107 7107
==========================================
- Hits 45138 45055 -83
- Misses 19220 19338 +118
Partials 1889 1889
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `82.16% <94.44%> (+0.02%)` | :arrow_up: |
| postgres | `82.17% <94.44%> (+0.03%)` | :arrow_up: |
| presto | `?` | |
| python | `82.25% <94.44%> (-0.38%)` | :arrow_down: |
| sqlite | `81.85% <94.44%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdml6LnB5) | `58.13% <75.00%> (+0.07%)` | :arrow_up: |
| [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `95.62% <100.00%> (+2.56%)` | :arrow_up: |
| [superset/exceptions.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhjZXB0aW9ucy5weQ==) | `91.42% <100.00%> (+0.16%)` | :arrow_up: |
| [superset/sql\_parse.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3BhcnNlLnB5) | `99.48% <100.00%> (+0.04%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `70.27% <0.00%> (-16.99%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.50% <0.00%> (-6.89%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `86.85% <0.00%> (-1.58%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `89.26% <0.00%> (-0.74%)` | :arrow_down: |
| ... and [4 more](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [04e3cfa...e45cb52](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro commented on a change in pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #17702:
URL: https://github.com/apache/superset/pull/17702#discussion_r765864344
##########
File path: superset/sql_parse.py
##########
@@ -378,3 +380,23 @@ def set_or_update_query_limit(self, new_limit: int, force: bool = False) -> str:
for i in statement.tokens:
str_res += str(i.value)
return str_res
+
+
+def validate_filter_clause(clause: str) -> None:
+ if sqlparse.format(clause, strip_comments=True) != sqlparse.format(clause):
+ raise SupersetQueryParseException("Filter clause contains comment")
Review comment:
As it currently stands, I don't see the need to break this up into multiple functions. That can be done later if/when it becomes 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.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ofekisr commented on a change in pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
ofekisr commented on a change in pull request #17702:
URL: https://github.com/apache/superset/pull/17702#discussion_r765833852
##########
File path: tests/integration_tests/charts/data/api_tests.py
##########
@@ -425,6 +425,30 @@ def test_with_invalid_where_parameter__400(self):
assert rv.status_code == 400
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_with_invalid_where_parameter_closing_unclosed__400(self):
+ self.query_context_payload["queries"][0]["filters"] = []
+ # WHERE-clause closing unclosed parens
Review comment:
redundant comment
you should encapsulate the assignment into get method or constant who express the where instead using a comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] bkyryliuk commented on a change in pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #17702:
URL: https://github.com/apache/superset/pull/17702#discussion_r768084399
##########
File path: superset/sql_parse.py
##########
@@ -378,3 +380,23 @@ def set_or_update_query_limit(self, new_limit: int, force: bool = False) -> str:
for i in statement.tokens:
str_res += str(i.value)
return str_res
+
+
+def validate_filter_clause(clause: str) -> None:
+ if sqlparse.format(clause, strip_comments=True) != sqlparse.format(clause):
+ raise QueryClauseValidationException("Filter clause contains comment")
Review comment:
@villebro is this intended ? this exception broke a number of our dashboards and seems to be a valid use case for the where clause sql. e.g. comment out different filter options while iterating on the chart
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ofekisr commented on a change in pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
ofekisr commented on a change in pull request #17702:
URL: https://github.com/apache/superset/pull/17702#discussion_r765833852
##########
File path: tests/integration_tests/charts/data/api_tests.py
##########
@@ -425,6 +425,30 @@ def test_with_invalid_where_parameter__400(self):
assert rv.status_code == 400
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_with_invalid_where_parameter_closing_unclosed__400(self):
+ self.query_context_payload["queries"][0]["filters"] = []
+ # WHERE-clause closing unclosed parens
Review comment:
redundant comment
you should encapsulate the right side of the assignment into get method or constant which expresses the where instead of using a comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ofekisr commented on a change in pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
ofekisr commented on a change in pull request #17702:
URL: https://github.com/apache/superset/pull/17702#discussion_r765833852
##########
File path: tests/integration_tests/charts/data/api_tests.py
##########
@@ -425,6 +425,30 @@ def test_with_invalid_where_parameter__400(self):
assert rv.status_code == 400
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_with_invalid_where_parameter_closing_unclosed__400(self):
+ self.query_context_payload["queries"][0]["filters"] = []
+ # WHERE-clause closing unclosed parens
Review comment:
redundant comment
you should encapsulate the right side of the assignment into get method or constant who express the where instead using a comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17702:
URL: https://github.com/apache/superset/pull/17702#issuecomment-989685202
# [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17702](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f704e00) into [master](https://codecov.io/gh/apache/superset/commit/04e3cfa6077faa84b07948fc6071e94d82a4a000?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (04e3cfa) will **decrease** coverage by `0.16%`.
> The diff coverage is `94.44%`.
> :exclamation: Current head f704e00 differs from pull request most recent head 646a085. Consider uploading reports for the commit 646a085 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17702 +/- ##
==========================================
- Coverage 68.13% 67.96% -0.17%
==========================================
Files 1653 1653
Lines 66247 66282 +35
Branches 7107 7107
==========================================
- Hits 45138 45051 -87
- Misses 19220 19342 +122
Partials 1889 1889
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `82.16% <94.44%> (+0.02%)` | :arrow_up: |
| postgres | `82.16% <94.44%> (+0.01%)` | :arrow_up: |
| presto | `?` | |
| python | `82.24% <94.44%> (-0.39%)` | :arrow_down: |
| sqlite | `81.85% <94.44%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdml6LnB5) | `58.13% <75.00%> (+0.07%)` | :arrow_up: |
| [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `95.08% <100.00%> (+2.01%)` | :arrow_up: |
| [superset/exceptions.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhjZXB0aW9ucy5weQ==) | `91.42% <100.00%> (+0.16%)` | :arrow_up: |
| [superset/sql\_parse.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3BhcnNlLnB5) | `99.48% <100.00%> (+0.04%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `70.27% <0.00%> (-16.99%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.50% <0.00%> (-6.89%)` | :arrow_down: |
| [superset/reports/commands/log\_prune.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9sb2dfcHJ1bmUucHk=) | `85.71% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/commands/importers/v1/utils.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbWFuZHMvaW1wb3J0ZXJzL3YxL3V0aWxzLnB5) | `89.13% <0.00%> (-2.18%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| ... and [7 more](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [04e3cfa...646a085](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17702:
URL: https://github.com/apache/superset/pull/17702#issuecomment-989685202
# [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17702](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f5f14db) into [master](https://codecov.io/gh/apache/superset/commit/04e3cfa6077faa84b07948fc6071e94d82a4a000?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (04e3cfa) will **decrease** coverage by `0.16%`.
> The diff coverage is `94.44%`.
[](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17702 +/- ##
==========================================
- Coverage 68.13% 67.96% -0.17%
==========================================
Files 1653 1653
Lines 66247 66282 +35
Branches 7107 7107
==========================================
- Hits 45138 45051 -87
- Misses 19220 19342 +122
Partials 1889 1889
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `82.16% <94.44%> (+0.02%)` | :arrow_up: |
| postgres | `82.16% <94.44%> (+0.01%)` | :arrow_up: |
| presto | `?` | |
| python | `82.24% <94.44%> (-0.39%)` | :arrow_down: |
| sqlite | `81.85% <94.44%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdml6LnB5) | `58.13% <75.00%> (+0.07%)` | :arrow_up: |
| [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `95.08% <100.00%> (+2.01%)` | :arrow_up: |
| [superset/exceptions.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhjZXB0aW9ucy5weQ==) | `91.42% <100.00%> (+0.16%)` | :arrow_up: |
| [superset/sql\_parse.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3BhcnNlLnB5) | `99.48% <100.00%> (+0.04%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `70.27% <0.00%> (-16.99%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.50% <0.00%> (-6.89%)` | :arrow_down: |
| [superset/reports/commands/log\_prune.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9sb2dfcHJ1bmUucHk=) | `85.71% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/commands/importers/v1/utils.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbWFuZHMvaW1wb3J0ZXJzL3YxL3V0aWxzLnB5) | `89.13% <0.00%> (-2.18%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| ... and [7 more](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [04e3cfa...f5f14db](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17702:
URL: https://github.com/apache/superset/pull/17702#issuecomment-989685202
# [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17702](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f5f14db) into [master](https://codecov.io/gh/apache/superset/commit/04e3cfa6077faa84b07948fc6071e94d82a4a000?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (04e3cfa) will **decrease** coverage by `0.16%`.
> The diff coverage is `94.44%`.
> :exclamation: Current head f5f14db differs from pull request most recent head 646a085. Consider uploading reports for the commit 646a085 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17702 +/- ##
==========================================
- Coverage 68.13% 67.97% -0.17%
==========================================
Files 1653 1653
Lines 66247 66282 +35
Branches 7107 7107
==========================================
- Hits 45138 45055 -83
- Misses 19220 19338 +118
Partials 1889 1889
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `82.16% <94.44%> (+0.02%)` | :arrow_up: |
| postgres | `82.17% <94.44%> (+0.03%)` | :arrow_up: |
| presto | `?` | |
| python | `82.25% <94.44%> (-0.38%)` | :arrow_down: |
| sqlite | `81.85% <94.44%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdml6LnB5) | `58.13% <75.00%> (+0.07%)` | :arrow_up: |
| [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `95.62% <100.00%> (+2.56%)` | :arrow_up: |
| [superset/exceptions.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhjZXB0aW9ucy5weQ==) | `91.42% <100.00%> (+0.16%)` | :arrow_up: |
| [superset/sql\_parse.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3BhcnNlLnB5) | `99.48% <100.00%> (+0.04%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `70.27% <0.00%> (-16.99%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.50% <0.00%> (-6.89%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `86.85% <0.00%> (-1.58%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `89.26% <0.00%> (-0.74%)` | :arrow_down: |
| ... and [4 more](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [04e3cfa...646a085](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro commented on a change in pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #17702:
URL: https://github.com/apache/superset/pull/17702#discussion_r765860104
##########
File path: superset/sql_parse.py
##########
@@ -378,3 +380,23 @@ def set_or_update_query_limit(self, new_limit: int, force: bool = False) -> str:
for i in statement.tokens:
str_res += str(i.value)
return str_res
+
+
+def validate_filter_clause(clause: str) -> None:
Review comment:
I agree this module is slightly messy right now, but it's not totally clear to me how this functionality will evolve. Since we already have some util-type of functionality, I will add this here for now, but going forward this should probably be refactored. But that refactoring should not be done as part of this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17702:
URL: https://github.com/apache/superset/pull/17702#issuecomment-989685202
# [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17702](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e45cb52) into [master](https://codecov.io/gh/apache/superset/commit/04e3cfa6077faa84b07948fc6071e94d82a4a000?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (04e3cfa) will **decrease** coverage by `0.16%`.
> The diff coverage is `94.59%`.
[](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17702 +/- ##
==========================================
- Coverage 68.13% 67.97% -0.17%
==========================================
Files 1653 1653
Lines 66247 66282 +35
Branches 7107 7107
==========================================
- Hits 45138 45055 -83
- Misses 19220 19338 +118
Partials 1889 1889
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `82.16% <94.59%> (+0.02%)` | :arrow_up: |
| postgres | `82.17% <94.59%> (+0.03%)` | :arrow_up: |
| presto | `?` | |
| python | `82.25% <94.59%> (-0.38%)` | :arrow_down: |
| sqlite | `81.85% <94.59%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdml6LnB5) | `58.13% <75.00%> (+0.07%)` | :arrow_up: |
| [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `95.62% <100.00%> (+2.56%)` | :arrow_up: |
| [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `97.36% <100.00%> (ø)` | |
| [superset/exceptions.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhjZXB0aW9ucy5weQ==) | `91.42% <100.00%> (+0.16%)` | :arrow_up: |
| [superset/sql\_parse.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3BhcnNlLnB5) | `99.48% <100.00%> (+0.04%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `70.27% <0.00%> (-16.99%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.50% <0.00%> (-6.89%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `86.85% <0.00%> (-1.58%)` | :arrow_down: |
| ... and [5 more](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [04e3cfa...e45cb52](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro merged pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
villebro merged pull request #17702:
URL: https://github.com/apache/superset/pull/17702
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17702:
URL: https://github.com/apache/superset/pull/17702#issuecomment-989685202
# [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17702](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ff79357) into [master](https://codecov.io/gh/apache/superset/commit/04e3cfa6077faa84b07948fc6071e94d82a4a000?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (04e3cfa) will **decrease** coverage by `0.16%`.
> The diff coverage is `94.59%`.
> :exclamation: Current head ff79357 differs from pull request most recent head f5f14db. Consider uploading reports for the commit f5f14db to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17702 +/- ##
==========================================
- Coverage 68.13% 67.97% -0.17%
==========================================
Files 1653 1653
Lines 66247 66283 +36
Branches 7107 7107
==========================================
- Hits 45138 45056 -82
- Misses 19220 19338 +118
Partials 1889 1889
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `82.16% <94.59%> (+0.02%)` | :arrow_up: |
| postgres | `82.17% <94.59%> (+0.03%)` | :arrow_up: |
| presto | `?` | |
| python | `82.25% <94.59%> (-0.38%)` | :arrow_down: |
| sqlite | `81.85% <94.59%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdml6LnB5) | `58.13% <75.00%> (+0.07%)` | :arrow_up: |
| [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `95.62% <100.00%> (+2.56%)` | :arrow_up: |
| [superset/exceptions.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhjZXB0aW9ucy5weQ==) | `91.42% <100.00%> (+0.16%)` | :arrow_up: |
| [superset/sql\_parse.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3BhcnNlLnB5) | `99.48% <100.00%> (+0.04%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `70.27% <0.00%> (-16.99%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.50% <0.00%> (-6.89%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `86.85% <0.00%> (-1.58%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `89.26% <0.00%> (-0.74%)` | :arrow_down: |
| ... and [4 more](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [04e3cfa...f5f14db](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17702:
URL: https://github.com/apache/superset/pull/17702#issuecomment-989685202
# [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17702](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f5f14db) into [master](https://codecov.io/gh/apache/superset/commit/04e3cfa6077faa84b07948fc6071e94d82a4a000?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (04e3cfa) will **decrease** coverage by `0.16%`.
> The diff coverage is `94.44%`.
> :exclamation: Current head f5f14db differs from pull request most recent head 646a085. Consider uploading reports for the commit 646a085 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17702 +/- ##
==========================================
- Coverage 68.13% 67.96% -0.17%
==========================================
Files 1653 1653
Lines 66247 66282 +35
Branches 7107 7107
==========================================
- Hits 45138 45051 -87
- Misses 19220 19342 +122
Partials 1889 1889
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `82.16% <94.44%> (+0.02%)` | :arrow_up: |
| postgres | `82.16% <94.44%> (+0.01%)` | :arrow_up: |
| presto | `?` | |
| python | `82.24% <94.44%> (-0.39%)` | :arrow_down: |
| sqlite | `81.85% <94.44%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdml6LnB5) | `58.13% <75.00%> (+0.07%)` | :arrow_up: |
| [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `95.08% <100.00%> (+2.01%)` | :arrow_up: |
| [superset/exceptions.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhjZXB0aW9ucy5weQ==) | `91.42% <100.00%> (+0.16%)` | :arrow_up: |
| [superset/sql\_parse.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3BhcnNlLnB5) | `99.48% <100.00%> (+0.04%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `70.27% <0.00%> (-16.99%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.50% <0.00%> (-6.89%)` | :arrow_down: |
| [superset/reports/commands/log\_prune.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvcmVwb3J0cy9jb21tYW5kcy9sb2dfcHJ1bmUucHk=) | `85.71% <0.00%> (-3.58%)` | :arrow_down: |
| [superset/commands/importers/v1/utils.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbWFuZHMvaW1wb3J0ZXJzL3YxL3V0aWxzLnB5) | `89.13% <0.00%> (-2.18%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| ... and [7 more](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [04e3cfa...646a085](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17702:
URL: https://github.com/apache/superset/pull/17702#issuecomment-989685202
# [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17702](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f704e00) into [master](https://codecov.io/gh/apache/superset/commit/04e3cfa6077faa84b07948fc6071e94d82a4a000?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (04e3cfa) will **decrease** coverage by `0.16%`.
> The diff coverage is `94.44%`.
> :exclamation: Current head f704e00 differs from pull request most recent head 646a085. Consider uploading reports for the commit 646a085 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17702 +/- ##
==========================================
- Coverage 68.13% 67.97% -0.17%
==========================================
Files 1653 1653
Lines 66247 66282 +35
Branches 7107 7107
==========================================
- Hits 45138 45055 -83
- Misses 19220 19338 +118
Partials 1889 1889
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `82.16% <94.44%> (+0.02%)` | :arrow_up: |
| postgres | `82.17% <94.44%> (+0.03%)` | :arrow_up: |
| presto | `?` | |
| python | `82.25% <94.44%> (-0.38%)` | :arrow_down: |
| sqlite | `81.85% <94.44%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdml6LnB5) | `58.13% <75.00%> (+0.07%)` | :arrow_up: |
| [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `95.62% <100.00%> (+2.56%)` | :arrow_up: |
| [superset/exceptions.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhjZXB0aW9ucy5weQ==) | `91.42% <100.00%> (+0.16%)` | :arrow_up: |
| [superset/sql\_parse.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3BhcnNlLnB5) | `99.48% <100.00%> (+0.04%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `70.27% <0.00%> (-16.99%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.50% <0.00%> (-6.89%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `86.85% <0.00%> (-1.58%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `89.26% <0.00%> (-0.74%)` | :arrow_down: |
| ... and [4 more](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [04e3cfa...646a085](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] bkyryliuk commented on a change in pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #17702:
URL: https://github.com/apache/superset/pull/17702#discussion_r768084399
##########
File path: superset/sql_parse.py
##########
@@ -378,3 +380,23 @@ def set_or_update_query_limit(self, new_limit: int, force: bool = False) -> str:
for i in statement.tokens:
str_res += str(i.value)
return str_res
+
+
+def validate_filter_clause(clause: str) -> None:
+ if sqlparse.format(clause, strip_comments=True) != sqlparse.format(clause):
+ raise QueryClauseValidationException("Filter clause contains comment")
Review comment:
@villebro is this intended ? this exception broke a number of our dashboards and seems to be a valid use case for the where clause sql. e.g. comment out different filter options while iterating on the chart
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17702:
URL: https://github.com/apache/superset/pull/17702#issuecomment-989685202
# [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17702](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ff79357) into [master](https://codecov.io/gh/apache/superset/commit/04e3cfa6077faa84b07948fc6071e94d82a4a000?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (04e3cfa) will **decrease** coverage by `0.16%`.
> The diff coverage is `94.59%`.
[](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17702 +/- ##
==========================================
- Coverage 68.13% 67.97% -0.17%
==========================================
Files 1653 1653
Lines 66247 66283 +36
Branches 7107 7107
==========================================
- Hits 45138 45056 -82
- Misses 19220 19338 +118
Partials 1889 1889
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `82.16% <94.59%> (+0.02%)` | :arrow_up: |
| postgres | `82.17% <94.59%> (+0.03%)` | :arrow_up: |
| presto | `?` | |
| python | `82.25% <94.59%> (-0.38%)` | :arrow_down: |
| sqlite | `81.85% <94.59%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/viz.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdml6LnB5) | `58.13% <75.00%> (+0.07%)` | :arrow_up: |
| [superset/common/query\_object.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29tbW9uL3F1ZXJ5X29iamVjdC5weQ==) | `95.62% <100.00%> (+2.56%)` | :arrow_up: |
| [superset/exceptions.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZXhjZXB0aW9ucy5weQ==) | `91.42% <100.00%> (+0.16%)` | :arrow_up: |
| [superset/sql\_parse.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3BhcnNlLnB5) | `99.48% <100.00%> (+0.04%)` | :arrow_up: |
| [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `70.27% <0.00%> (-16.99%)` | :arrow_down: |
| [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `83.50% <0.00%> (-6.89%)` | :arrow_down: |
| [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `81.03% <0.00%> (-1.73%)` | :arrow_down: |
| [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `86.85% <0.00%> (-1.58%)` | :arrow_down: |
| [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `89.26% <0.00%> (-0.74%)` | :arrow_down: |
| ... and [4 more](https://codecov.io/gh/apache/superset/pull/17702/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [04e3cfa...ff79357](https://codecov.io/gh/apache/superset/pull/17702?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] dpgaspar commented on a change in pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #17702:
URL: https://github.com/apache/superset/pull/17702#discussion_r765704627
##########
File path: superset/viz.py
##########
@@ -373,6 +375,15 @@ def query_obj(self) -> QueryObjectDict: # pylint: disable=too-many-locals
self.from_dttm = from_dttm
self.to_dttm = to_dttm
+ # validate sql filters
+ for param in ("where", "having"):
Review comment:
optional: but this could be DRY(er)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro commented on a change in pull request #17702: chore(sql): clean up invalid filter clause exception types
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #17702:
URL: https://github.com/apache/superset/pull/17702#discussion_r765870994
##########
File path: tests/integration_tests/charts/data/api_tests.py
##########
@@ -425,6 +425,30 @@ def test_with_invalid_where_parameter__400(self):
assert rv.status_code == 400
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_with_invalid_where_parameter_closing_unclosed__400(self):
+ self.query_context_payload["queries"][0]["filters"] = []
+ # WHERE-clause closing unclosed parens
+ self.query_context_payload["queries"][0]["extras"][
+ "where"
+ ] = "state = 'CA') OR (state = 'NY'"
+
+ rv = self.post_assert_metric(CHART_DATA_URI, self.query_context_payload, "data")
+
+ assert rv.status_code == 400
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_with_invalid_having_parameter_closing_and_comment__400(self):
+ self.query_context_payload["queries"][0]["filters"] = []
+ # WHERE-clause with unclosed parens
Review comment:
Good catch - I removed both comments, as they were really not needed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org