villebro commented on a change in pull request #14507:
URL: https://github.com/apache/superset/pull/14507#discussion_r628320875
##########
File path: superset/jinja_context.py
##########
@@ -213,6 +165,147 @@ def url_param(
self.cache_key_wrapper(result)
return result
+ def filter_values(
+ self, column: str, default: Optional[str] = None, remove_filter: bool
= True
+ ) -> List[str]:
+ """Gets a values for a particular filter as a list
+
+ This is useful if:
+ - you want to use a filter box to filter a query where the name of
filter
+ box column doesn't match the one in the select statement
+ - you want to have the ability for filter inside the main query
for speed
+ purposes
+
+ Usage example::
+
+ SELECT action, count(*) as times
+ FROM logs
+ WHERE
+ action in ({{ "'" + "','".join(filter_values('action_type')) +
"'" }})
+ GROUP BY action
+
+ :param column: column/filter name to lookup
+ :param default: default value to return if there's no matching columns
+ :param remove_filter:
+ flag indicating if superset should generate a condition for this
filter
Review comment:
This could probably be expressed more clearly. How does this sound?
(feel free to improvise/say if you disagree)
"When set to true, mark the filter as processed, removing it from the outer
query. Useful when a filter should only apply to the inner query."
##########
File path: superset/jinja_context.py
##########
@@ -213,6 +165,147 @@ def url_param(
self.cache_key_wrapper(result)
return result
+ def filter_values(
+ self, column: str, default: Optional[str] = None, remove_filter: bool
= True
+ ) -> List[str]:
+ """Gets a values for a particular filter as a list
+
+ This is useful if:
+ - you want to use a filter box to filter a query where the name of
filter
+ box column doesn't match the one in the select statement
+ - you want to have the ability for filter inside the main query
for speed
+ purposes
Review comment:
Let's remove references to filter box. Maybe we can talk about filter
components?
##########
File path: superset/connectors/sqla/models.py
##########
@@ -1161,6 +1161,21 @@ def get_sqla_query( # pylint:
disable=too-many-arguments,too-many-locals,too-ma
val = flt.get("val")
op = flt["op"].upper()
col_obj = columns_by_name.get(col)
+
+ if is_feature_enabled("ENABLE_TEMPLATE_REMOVE_FILTERS"):
+ # Assume the virtual table's jinja template does not process
+ # the filter.
+ filter_removed_by_vt = False
Review comment:
It took me a few seconds to process this variable name. Could we just do
something like `ignore_filter` and then make sure the comment + code explains
what's going on?
##########
File path: superset/jinja_context.py
##########
@@ -213,6 +165,147 @@ def url_param(
self.cache_key_wrapper(result)
return result
+ def filter_values(
+ self, column: str, default: Optional[str] = None, remove_filter: bool
= True
+ ) -> List[str]:
+ """Gets a values for a particular filter as a list
+
+ This is useful if:
+ - you want to use a filter box to filter a query where the name of
filter
+ box column doesn't match the one in the select statement
+ - you want to have the ability for filter inside the main query
for speed
+ purposes
+
+ Usage example::
+
+ SELECT action, count(*) as times
+ FROM logs
+ WHERE
+ action in ({{ "'" + "','".join(filter_values('action_type')) +
"'" }})
+ GROUP BY action
+
+ :param column: column/filter name to lookup
+ :param default: default value to return if there's no matching columns
+ :param remove_filter:
+ flag indicating if superset should generate a condition for this
filter
+ :return: returns a list of filter values
+ """
+ from superset.views.utils import get_form_data
+
+ form_data, _ = get_form_data()
+ convert_legacy_filters_into_adhoc(form_data)
+ merge_extra_filters(form_data)
+
+ return_val = [
+ comparator
+ for filter in form_data.get("adhoc_filters", [])
+ for comparator in (
+ filter["comparator"]
+ if isinstance(filter["comparator"], list)
+ else [filter["comparator"]]
+ )
+ if (
+ filter.get("expressionType") == "SIMPLE"
+ and filter.get("clause") == "WHERE"
+ and filter.get("subject") == column
+ and filter.get("comparator")
+ )
+ ]
+
+ if return_val:
+ if remove_filter:
+ self.cache_key_wrapper({"filter_removed_by_vt": column})
+ return return_val
+
+ if default:
+ return [default]
+
+ return []
+
+ def get_filters(self, column: str, remove_filter: bool = True) ->
List[str]:
Review comment:
Since `get_filters` is a superset of `filter_values`, could we implement
`filter_values` so that it calls `get_filters` and just removes the additional
context that it receives? This way we would avoid having duplicated logic for
similar operations. We may also want to mark `filter_values` as deprecated and
plan for its removal in 2.0.
##########
File path: superset/jinja_context.py
##########
@@ -213,6 +165,147 @@ def url_param(
self.cache_key_wrapper(result)
return result
+ def filter_values(
+ self, column: str, default: Optional[str] = None, remove_filter: bool
= True
+ ) -> List[str]:
+ """Gets a values for a particular filter as a list
+
+ This is useful if:
+ - you want to use a filter box to filter a query where the name of
filter
+ box column doesn't match the one in the select statement
+ - you want to have the ability for filter inside the main query
for speed
+ purposes
+
+ Usage example::
+
+ SELECT action, count(*) as times
+ FROM logs
+ WHERE
+ action in ({{ "'" + "','".join(filter_values('action_type')) +
"'" }})
+ GROUP BY action
+
+ :param column: column/filter name to lookup
+ :param default: default value to return if there's no matching columns
+ :param remove_filter:
+ flag indicating if superset should generate a condition for this
filter
+ :return: returns a list of filter values
+ """
+ from superset.views.utils import get_form_data
+
+ form_data, _ = get_form_data()
+ convert_legacy_filters_into_adhoc(form_data)
+ merge_extra_filters(form_data)
+
+ return_val = [
+ comparator
+ for filter in form_data.get("adhoc_filters", [])
+ for comparator in (
+ filter["comparator"]
+ if isinstance(filter["comparator"], list)
+ else [filter["comparator"]]
+ )
+ if (
+ filter.get("expressionType") == "SIMPLE"
+ and filter.get("clause") == "WHERE"
+ and filter.get("subject") == column
+ and filter.get("comparator")
+ )
+ ]
+
+ if return_val:
+ if remove_filter:
+ self.cache_key_wrapper({"filter_removed_by_vt": column})
Review comment:
I believe all filters are converted into adhocs filters that have a
unique id. IMO it would be optimal if we would collect the removed filter ids
in a dedicated `dict` on `ExtraCache` called `removed_filters` and then make it
possible to check if a filter has been removed or not without looping through
all extra cache keys.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
