You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "dpgaspar (via GitHub)" <gi...@apache.org> on 2024/04/19 10:45:18 UTC
[PR] fix: bump sqlparse to 0.5.0 [superset]
dpgaspar opened a new pull request, #28144:
URL: https://github.com/apache/superset/pull/28144
### SUMMARY
Bumps sqlparse to 0.5.0 to address a potencial vulnerability.
Followup from: https://github.com/apache/superset/pull/28042
Don't think we will have breaking changes here, I think that our upper bound was a cautions bound following semantic versioning for 0.X.X versions.
Changelog: https://github.com/andialbrecht/sqlparse/blob/master/CHANGELOG
```
Notable Changes
* Drop support for Python 3.5, 3.6, and 3.7.
* Python 3.12 is now supported (pr725, by hugovk).
* IMPORTANT: Fixes a potential denial of service attack (DOS) due to recursion
error for deeply nested statements. Instead of recursion error a generic
SQLParseError is raised. See the security advisory for details:
https://github.com/andialbrecht/sqlparse/security/advisories/GHSA-2m57-hf25-phgg
The vulnerability was discovered by @uriyay-jfrog. Thanks for reporting!
Enhancements:
* Splitting statements now allows to remove the semicolon at the end.
Some database backends love statements without semicolon (issue742).
* Support TypedLiterals in get_parameters (pr649, by Khrol).
* Improve splitting of Transact SQL when using GO keyword (issue762).
* Support for some JSON operators (issue682).
* Improve formatting of statements containing JSON operators (issue542).
* Support for BigQuery and Snowflake keywords (pr699, by griffatrasgo).
* Support parsing of OVER clause (issue701, pr768 by r33s3n6).
Bug Fixes
* Ignore dunder attributes when creating Tokens (issue672).
* Allow operators to precede dollar-quoted strings (issue763).
* Fix parsing of nested order clauses (issue745, pr746 by john-bodley).
* Thread-safe initialization of Lexer class (issue730).
* Classify TRUNCATE as DDL and GRANT/REVOKE as DCL keywords (based on pr719
by josuc1, thanks for bringing this up!).
* Fix parsing of PRIMARY KEY (issue740).
```
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: bump sqlparse to 0.5.0 [superset]
Posted by "betodealmeida (via GitHub)" <gi...@apache.org>.
betodealmeida commented on PR #28144:
URL: https://github.com/apache/superset/pull/28144#issuecomment-2110606019
> @dpgaspar and @betodealmeida is the plan sitll—per [[SIP-117] Improve SQL parsing](https://github.com/apache/superset/issues/26786) —to remove `sqlparse` completely?
Yes, I'm still working on it. Had to put aside to work on catalogs (SIP-95), but I'm going to finish the work on SIP-117 soon.
--
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
Re: [PR] fix: bump sqlparse to 0.5.0 [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #28144:
URL: https://github.com/apache/superset/pull/28144#discussion_r1572740500
##########
pyproject.toml:
##########
@@ -89,7 +89,7 @@ dependencies = [
"sqlalchemy>=1.4, <2",
"sqlalchemy-utils>=0.38.3, <0.39",
"sqlglot>=23.0.2,<24",
- "sqlparse>=0.4.4, <0.5",
+ "sqlparse>=0.4.4, <0.6",
Review Comment:
Unless we know there is a problem with `0.6`, I would not put an upper bound. And if there is a know issue, we should put a clear comment as to what the known blocker is right above.
Maybe that's a topic for the devX group though, but when we unleash `supersetbot bump-python` it will only generate PRs that bumps `requirements/*.txt` within the ranges specified in `pyproject.toml`, so generally we'll want no upper bound to see the PR come through and whether CI passes. There we'll be able to use our judgement as to how much scrutiny the PR has based on things like semver and related changelog
--
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
Re: [PR] fix: bump sqlparse to 0.5.0 [superset]
Posted by "mistercrunch (via GitHub)" <gi...@apache.org>.
mistercrunch commented on code in PR #28144:
URL: https://github.com/apache/superset/pull/28144#discussion_r1572740500
##########
pyproject.toml:
##########
@@ -89,7 +89,7 @@ dependencies = [
"sqlalchemy>=1.4, <2",
"sqlalchemy-utils>=0.38.3, <0.39",
"sqlglot>=23.0.2,<24",
- "sqlparse>=0.4.4, <0.5",
+ "sqlparse>=0.4.4, <0.6",
Review Comment:
Unless we know there is a problem with `0.6`, I would not put an upper bound. And if there is a know issue, we should put a clear comment as to what the known blocker is right above.
Maybe that's a topic for the devX group though, but when we unleash `supersetbot bump-python` it will only generate PRs that bumps `requirements/*.txt` within the ranges specified in `pyproject.toml`, so generally we'll want no upper bound to see the PR come through and whether CI passes.
--
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
Re: [PR] fix: bump sqlparse to 0.5.0 [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #28144:
URL: https://github.com/apache/superset/pull/28144#issuecomment-2110981381
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/28144?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 64.43%. Comparing base [(`76d897e`)](https://app.codecov.io/gh/apache/superset/commit/76d897eaa2f9e137102bc194c2e3109c29d0348f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`fe92963`)](https://app.codecov.io/gh/apache/superset/pull/28144?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 111 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #28144 +/- ##
==========================================
+ Coverage 60.48% 64.43% +3.94%
==========================================
Files 1931 521 -1410
Lines 76236 37467 -38769
Branches 8568 0 -8568
==========================================
- Hits 46114 24141 -21973
+ Misses 28017 13326 -14691
+ Partials 2105 0 -2105
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/28144/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/28144/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `49.09% <ø> (-0.07%)` | :arrow_down: |
| [javascript](https://app.codecov.io/gh/apache/superset/pull/28144/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/28144/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.65% <ø> (-0.15%)` | :arrow_down: |
| [python](https://app.codecov.io/gh/apache/superset/pull/28144/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `64.43% <ø> (+0.94%)` | :arrow_up: |
| [unit](https://app.codecov.io/gh/apache/superset/pull/28144/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.81% <ø> (+1.19%)` | :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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/28144?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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
Re: [PR] fix: bump sqlparse to 0.5.0 [superset]
Posted by "dpgaspar (via GitHub)" <gi...@apache.org>.
dpgaspar merged PR #28144:
URL: https://github.com/apache/superset/pull/28144
--
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
Re: [PR] fix: bump sqlparse to 0.5.0 [superset]
Posted by "john-bodley (via GitHub)" <gi...@apache.org>.
john-bodley commented on PR #28144:
URL: https://github.com/apache/superset/pull/28144#issuecomment-2066993669
@dpgaspar and @betodealmeida is the plan sitll—per [[SIP-117] Improve SQL parsing](https://github.com/apache/superset/issues/26786) —to remove `sqlparse` completely?
--
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
Re: [PR] fix: bump sqlparse to 0.5.0 [superset]
Posted by "denodo-research-labs (via GitHub)" <gi...@apache.org>.
denodo-research-labs commented on PR #28144:
URL: https://github.com/apache/superset/pull/28144#issuecomment-2120192800
[CVE-2024-4340](https://nvd.nist.gov/vuln/detail/CVE-2024-4340) is reported as `HIGH` by some security tools like PRISMA.
Will this version bump be applied to the 3.1-x branch too to avoid that CVE affecting current stable versions?
--
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
Re: [PR] fix: bump sqlparse to 0.5.0 [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #28144:
URL: https://github.com/apache/superset/pull/28144#discussion_r1600287831
##########
pyproject.toml:
##########
@@ -89,7 +89,7 @@ dependencies = [
"sqlalchemy>=1.4, <2",
"sqlalchemy-utils>=0.38.3, <0.39",
"sqlglot>=23.0.2,<24",
- "sqlparse>=0.4.4, <0.5",
+ "sqlparse>=0.4.4",
Review Comment:
Sorry for a noob question, but should this be `sqlparse>=0.5.0`, to preclude people from using anything less?
--
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
Re: [PR] fix: bump sqlparse to 0.5.0 [superset]
Posted by "dpgaspar (via GitHub)" <gi...@apache.org>.
dpgaspar commented on code in PR #28144:
URL: https://github.com/apache/superset/pull/28144#discussion_r1600529686
##########
pyproject.toml:
##########
@@ -89,7 +89,7 @@ dependencies = [
"sqlalchemy>=1.4, <2",
"sqlalchemy-utils>=0.38.3, <0.39",
"sqlglot>=23.0.2,<24",
- "sqlparse>=0.4.4, <0.5",
+ "sqlparse>=0.4.4",
Review Comment:
not noob, I think it 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