feat: improve adhoc SQL validation #19454
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
betodealmeida
commented
Mar 31, 2022
| @@ -984,15 +992,14 @@ def is_alias_used_in_orderby(col: ColumnElement) -> bool: | |||
|
|
|||
| def _get_sqla_row_level_filters( | |||
| self, template_processor: BaseTemplateProcessor | |||
| ) -> List[str]: | |||
| ) -> List[TextClause]: | |||
There was a problem hiding this comment.
Not sure why mypy didn't catch this.
betodealmeida
commented
Mar 31, 2022
| @@ -1420,7 +1441,6 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma | |||
| and db_engine_spec.allows_hidden_cc_in_orderby | |||
| and col.name in [select_col.name for select_col in select_exprs] | |||
| ): | |||
| validate_adhoc_subquery(str(col.expression)) | |||
There was a problem hiding this comment.
Moved this up, where it's still a string.
betodealmeida
force-pushed
the
insert_rls
branch
from
b86e904
to
b4cf9d8
Compare
betodealmeida
force-pushed
the
insert_rls
branch
from
b4cf9d8
to
b6ab91a
Compare
villebro
reviewed
Mar 31, 2022
betodealmeida
force-pushed
the
insert_rls
branch
from
d171500
to
b83a64f
Compare
villebro
approved these changes
Mar 31, 2022
Codecov Report
@@ Coverage Diff @@
## master #19454 +/- ##
=======================================
Coverage 66.57% 66.58%
=======================================
Files 1675 1675
Lines 64092 64107 +15
Branches 6519 6519
=======================================
+ Hits 42672 42684 +12
- Misses 19729 19732 +3
Partials 1691 1691
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
suddjian
pushed a commit
to preset-io/superset
that referenced
this pull request
Mar 31, 2022
* feat: improve adhoc SQL validation * Small changes * Add more unit tests
sadpandajoe
pushed a commit
to preset-io/superset
that referenced
this pull request
Apr 1, 2022
* feat: improve adhoc SQL validation * Small changes * Add more unit tests (cherry picked from commit 6828624)
villebro
pushed a commit
that referenced
this pull request
Apr 3, 2022
* feat: improve adhoc SQL validation * Small changes * Add more unit tests (cherry picked from commit 6828624)
villebro
pushed a commit
that referenced
this pull request
Apr 4, 2022
* feat: improve adhoc SQL validation * Small changes * Add more unit tests (cherry picked from commit 6828624)
|
🏷️ preset:2022.11 |
philipher29
pushed a commit
to ValtechMobility/superset
that referenced
this pull request
Jun 9, 2022
* feat: improve adhoc SQL validation * Small changes * Add more unit tests
This was referenced Oct 4, 2022
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
Improve the
validate_adhoc_subqueryfunction.I changed the logic of
insert_rls. As originally written we'd pass (1) the adhoc SQL, (2) the dataset name and (3) the associated RLS predicate. In order to use it here I modified it to receive (1) the adhoc SQL with the (2) database and (3) schema where it will run. We then parse the adhoc SQL, and for each table found we check if there's an associated RLS rule; if so, we modify the SQL to include the RLS predicate(s).BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TESTING INSTRUCTIONS
Updated unit tests, will add more.
ADDITIONAL INFORMATION