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.

Sign up for GitHub

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

perf(sqla): avoid unnecessary type check on adhoc column #23491

Merged
merged 3 commits into from
Mar 31, 2023
Merged

perf(sqla): avoid unnecessary type check on adhoc column #23491

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
115ed7a
perf(sqla): avoid unnecessary type check on adhoc column
villebro Mar 27, 2023
de0c2bf
add arg to check expression
villebro Mar 31, 2023
3a8b4db
lint
villebro Mar 31, 2023
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
Prev Previous commit
Next Next commit
add arg to check expression
  • Loading branch information
@villebro
villebro committed Mar 31, 2023
commit de0c2bf42125c96c655b83b4ef043bc9b5b9ff35
12 changes: 10 additions & 2 deletions superset/connectors/sqla/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1009,12 +1009,16 @@ def adhoc_metric_to_sqla(
def adhoc_column_to_sqla(
self,
col: AdhocColumn,
force_check_expression: bool = False,
template_processor: Optional[BaseTemplateProcessor] = None,
) -> ColumnElement:
"""
Turn an adhoc column into a sqlalchemy column.

:param col: Adhoc column definition
:param force_check_expression: Should the expression be checked in the db.
This is needed to validate if a filter with an adhoc column
is applicable.
:param template_processor: template_processor instance
:returns: The metric defined as a sqlalchemy column
:rtype: sqlalchemy.sql.column
Expand All @@ -1037,7 +1041,7 @@ def adhoc_column_to_sqla(
is_dttm = col_in_metadata.is_temporal
else:
sqla_column = literal_column(expression)
if has_timegrain:
if has_timegrain or force_check_expression:
try:
# probe adhoc column type
tbl, _ = self.get_from_clause(template_processor)
Expand Down Expand Up @@ -1458,7 +1462,11 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
col_obj = dttm_col
elif is_adhoc_column(flt_col):
try:
sqla_col = self.adhoc_column_to_sqla(flt_col)
sqla_col = self.adhoc_column_to_sqla(
col=flt_col,
force_check_expression=True,
template_processor=template_processor,
Copy link
Member Author

Choose a reason for hiding this comment

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

bycatch - the template processor was missing here

)
applied_adhoc_filters_columns.append(flt_col)
except ColumnNotFoundException:
rejected_adhoc_filters_columns.append(flt_col)
Expand Down