Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(charts): allow query mutator to update queries after splitting original sql #21645

Merged
merged 24 commits into from
Jan 12, 2023
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a2c3780
added mutate after split functionality
anallani Mar 28, 2022
5924a75
added mutate_after_split to models.py
anallani Apr 1, 2022
ec89a38
Added debugging print statements
anallani Apr 11, 2022
af0eb77
update workflow
bakerbean Apr 11, 2022
689725a
more debugging print statements
anallani Apr 11, 2022
ea26dd5
Merge branch 'ofs-digital-superset' of https://github.com/OFS-Digital…
anallani Apr 11, 2022
67729c7
more debugging statements
anallani Apr 11, 2022
2f5cb7b
fixed query execution
anallani Apr 11, 2022
1ad3791
polished code
anallani Apr 20, 2022
a3db4e6
merging commits
anallani Apr 20, 2022
c0a725a
revert workflow change
AkashN7 Apr 21, 2022
cf9febf
workflow update
AkashN7 Apr 21, 2022
9904f60
Completed adding mutate_after_split functionality
AkashN7 Apr 25, 2022
e9b1f01
Merge branch 'apache:master' into ofs-digital-superset
AkashN7 Apr 25, 2022
dacebb5
Updated to include fix in documentation
AkashN7 May 19, 2022
b0ec840
Merge branch 'ofs-digital-superset' of https://github.com/AkashN7/sup…
AkashN7 May 19, 2022
88f54ff
Merge branch 'master' of https://github.com/AkashN7/superset into ofs…
AkashN7 May 19, 2022
38c450a
Restore workflow changes to prior commit
AkashN7 May 19, 2022
25f2dc1
Revert "Restore workflow changes to prior commit"
AkashN7 May 19, 2022
2e509ec
Revert "update workflow"
AkashN7 May 19, 2022
19b20e9
Fixed usage of deprecated parameter user_name
AkashN7 May 23, 2022
ee6d467
Commits on 17Oct2022 for SETROLE
solanksh Oct 17, 2022
4e70aca
Merge branch 'master' into ofs-digital-superset
villebro Dec 16, 2022
5c72880
remove redundant variable
villebro Dec 16, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,13 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument
) -> str:
return sql

# A variable that chooses whether to apply the SQL_QUERY_MUTATOR before or after splitting the input query
# It allows for using the SQL_QUERY_MUTATOR function for more than comments
# Usage: If you want to apply a change to every statement to a given query, set MUTATE_AFTER_SPLIT = True
# An example use case is if data has role based access controls, and you want to apply
# a SET ROLE statement alongside every user query. Changing this variable maintains
# functionality for both the SQL_Lab and Charts.
MUTATE_AFTER_SPLIT = False

# This auth provider is used by background (offline) tasks that need to access
# protected resources. Can be overridden by end users in order to support
Expand Down
4 changes: 3 additions & 1 deletion superset/connectors/sqla/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -922,7 +922,9 @@ def mutate_query_from_config(self, sql: str) -> str:

Typically adds comments to the query with context"""
sql_query_mutator = config["SQL_QUERY_MUTATOR"]
if sql_query_mutator:
mutate_after_split = config["MUTATE_AFTER_SPLIT"]
if sql_query_mutator and not mutate_after_split:
username = utils.get_username()
sql = sql_query_mutator(
sql,
user_name=get_username(), # TODO(john-bodley): Deprecate in 3.0.
Expand Down
3 changes: 2 additions & 1 deletion superset/db_engine_specs/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1166,7 +1166,8 @@ def process_statement(cls, statement: str, database: "Database") -> str:
parsed_query = ParsedQuery(statement)
sql = parsed_query.stripped()
sql_query_mutator = current_app.config["SQL_QUERY_MUTATOR"]
if sql_query_mutator:
mutate_after_split = current_app.config["MUTATE_AFTER_SPLIT"]
if sql_query_mutator and not mutate_after_split:
sql = sql_query_mutator(
sql,
user_name=get_username(), # TODO(john-bodley): Deprecate in 3.0.
Expand Down
27 changes: 24 additions & 3 deletions superset/models/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,11 @@ def get_df( # pylint: disable=too-many-locals
mutator: Optional[Callable[[pd.DataFrame], None]] = None,
) -> pd.DataFrame:
sqls = self.db_engine_spec.parse_sql(sql)

engine = self.get_sqla_engine(schema)
username = utils.get_username() or username

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@villebro Comment from original Pull Request
This variable doesn't appear to be defined in this scope?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@baz-bakerhughes @villebro If caching is enabled in the Superset configuration, then by default the username value will be used and so username might not need be defined in the config.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@villebro @baz-bakerhughes I have pushed the changes suggested.

mutate_after_split = config["MUTATE_AFTER_SPLIT"]
sql_query_mutator = config["SQL_QUERY_MUTATOR"]

def needs_conversion(df_series: pd.Series) -> bool:
return (
Expand All @@ -438,12 +442,29 @@ def _log_query(sql: str) -> None:
with closing(engine.raw_connection()) as conn:
cursor = conn.cursor()
for sql_ in sqls[:-1]:
if mutate_after_split:
sql_ = sql_query_mutator(
sql_,
user_name=username,
security_manager=security_manager,
database=None
)
_log_query(sql_)
self.db_engine_spec.execute(cursor, sql_)
cursor.fetchall()

_log_query(sqls[-1])
self.db_engine_spec.execute(cursor, sqls[-1])

if mutate_after_split:
last_sql = sql_query_mutator(
sqls[-1],
user_name=username,
security_manager=security_manager,
database=None
)
_log_query(last_sql)
self.db_engine_spec.execute(cursor, last_sql)
else:
_log_query(sqls[-1])
self.db_engine_spec.execute(cursor, sqls[-1])

data = self.db_engine_spec.fetch_data(cursor)
result_set = SupersetResultSet(
Expand Down