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

Jump to bottom

feat(cross-filters): add support for temporal filters #16139

Merged
merged 11 commits into from
Aug 10, 2021
Merged

feat(cross-filters): add support for temporal filters #16139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e480b4b
feat(cross-filters): add support for temporal filters
villebro Aug 5, 2021
a11c77a
fix test
villebro Aug 9, 2021
846abaa
make filter optional
villebro Aug 9, 2021
63251e9
remove mocks
villebro Aug 9, 2021
fc375cd
fix more tests
villebro Aug 9, 2021
9527819
remove unnecessary optionality
villebro Aug 9, 2021
b0271e6
fix even more tests
villebro Aug 9, 2021
c4aae4e
bump superset-ui
villebro Aug 9, 2021
d71f328
add isExtra to schema
villebro Aug 9, 2021
7f11b86
address comments
villebro Aug 9, 2021
2ff35a8
fix presto test
villebro Aug 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
604 changes: 302 additions & 302 deletions superset-frontend/package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

56 changes: 28 additions & 28 deletions superset-frontend/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,35 +67,35 @@
"@emotion/babel-preset-css-prop": "^11.2.0",
"@emotion/cache": "^11.1.3",
"@emotion/react": "^11.1.5",
"@superset-ui/chart-controls": "^0.17.79",
"@superset-ui/core": "^0.17.75",
"@superset-ui/legacy-plugin-chart-calendar": "^0.17.79",
"@superset-ui/legacy-plugin-chart-chord": "^0.17.79",
"@superset-ui/legacy-plugin-chart-country-map": "^0.17.79",
"@superset-ui/legacy-plugin-chart-event-flow": "^0.17.79",
"@superset-ui/legacy-plugin-chart-force-directed": "^0.17.79",
"@superset-ui/legacy-plugin-chart-heatmap": "^0.17.79",
"@superset-ui/legacy-plugin-chart-histogram": "^0.17.79",
"@superset-ui/legacy-plugin-chart-horizon": "^0.17.79",
"@superset-ui/legacy-plugin-chart-map-box": "^0.17.79",
"@superset-ui/legacy-plugin-chart-paired-t-test": "^0.17.79",
"@superset-ui/legacy-plugin-chart-parallel-coordinates": "^0.17.79",
"@superset-ui/legacy-plugin-chart-partition": "^0.17.79",
"@superset-ui/legacy-plugin-chart-pivot-table": "^0.17.79",
"@superset-ui/legacy-plugin-chart-rose": "^0.17.79",
"@superset-ui/legacy-plugin-chart-sankey": "^0.17.79",
"@superset-ui/legacy-plugin-chart-sankey-loop": "^0.17.79",
"@superset-ui/legacy-plugin-chart-sunburst": "^0.17.79",
"@superset-ui/legacy-plugin-chart-treemap": "^0.17.79",
"@superset-ui/legacy-plugin-chart-world-map": "^0.17.79",
"@superset-ui/legacy-preset-chart-big-number": "^0.17.79",
"@superset-ui/chart-controls": "^0.17.80",
"@superset-ui/core": "^0.17.80",
"@superset-ui/legacy-plugin-chart-calendar": "^0.17.80",
"@superset-ui/legacy-plugin-chart-chord": "^0.17.80",
"@superset-ui/legacy-plugin-chart-country-map": "^0.17.80",
"@superset-ui/legacy-plugin-chart-event-flow": "^0.17.80",
"@superset-ui/legacy-plugin-chart-force-directed": "^0.17.80",
"@superset-ui/legacy-plugin-chart-heatmap": "^0.17.80",
"@superset-ui/legacy-plugin-chart-histogram": "^0.17.80",
"@superset-ui/legacy-plugin-chart-horizon": "^0.17.80",
"@superset-ui/legacy-plugin-chart-map-box": "^0.17.80",
"@superset-ui/legacy-plugin-chart-paired-t-test": "^0.17.80",
"@superset-ui/legacy-plugin-chart-parallel-coordinates": "^0.17.80",
"@superset-ui/legacy-plugin-chart-partition": "^0.17.80",
"@superset-ui/legacy-plugin-chart-pivot-table": "^0.17.80",
"@superset-ui/legacy-plugin-chart-rose": "^0.17.80",
"@superset-ui/legacy-plugin-chart-sankey": "^0.17.80",
"@superset-ui/legacy-plugin-chart-sankey-loop": "^0.17.80",
"@superset-ui/legacy-plugin-chart-sunburst": "^0.17.80",
"@superset-ui/legacy-plugin-chart-treemap": "^0.17.80",
"@superset-ui/legacy-plugin-chart-world-map": "^0.17.80",
"@superset-ui/legacy-preset-chart-big-number": "^0.17.80",
"@superset-ui/legacy-preset-chart-deckgl": "^0.4.9",
"@superset-ui/legacy-preset-chart-nvd3": "^0.17.79",
"@superset-ui/plugin-chart-echarts": "^0.17.79",
"@superset-ui/plugin-chart-pivot-table": "^0.17.79",
"@superset-ui/plugin-chart-table": "^0.17.79",
"@superset-ui/plugin-chart-word-cloud": "^0.17.79",
"@superset-ui/preset-chart-xy": "^0.17.79",
"@superset-ui/legacy-preset-chart-nvd3": "^0.17.80",
"@superset-ui/plugin-chart-echarts": "^0.17.80",
"@superset-ui/plugin-chart-pivot-table": "^0.17.80",
"@superset-ui/plugin-chart-table": "^0.17.80",
"@superset-ui/plugin-chart-word-cloud": "^0.17.80",
"@superset-ui/preset-chart-xy": "^0.17.80",
"@vx/responsive": "^0.0.195",
"abortcontroller-polyfill": "^1.1.9",
"antd": "^4.9.4",
Expand Down
14 changes: 14 additions & 0 deletions superset/charts/schemas.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,13 @@ class ChartDataAdhocMetricSchema(Schema):
"will be generated.",
example="metric_aec60732-fac0-4b17-b736-93f1a5c93e30",
)
timeGrain = fields.String(
description="Optional time grain for temporal filters", example="PT1M",
)
isExtra = fields.Boolean(
description="Indicates if the filter has been added by a filter component as "
"opposed to being a part of the original query."
)


class ChartDataAggregateConfigField(fields.Dict):
Expand Down Expand Up @@ -771,6 +778,13 @@ class ChartDataFilterSchema(Schema):
"integer, decimal or list, depending on the operator.",
example=["China", "France", "Japan"],
)
grain = fields.String(
description="Optional time grain for temporal filters", example="PT1M",
)
isExtra = fields.Boolean(
description="Indicates if the filter has been added by a filter component as "
"opposed to being a part of the original query."
)


class ChartDataExtrasSchema(Schema):
Expand Down
5 changes: 3 additions & 2 deletions superset/common/query_object.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
get_metric_names,
is_adhoc_metric,
json_int_dttm_ser,
QueryObjectFilterClause,
)
from superset.utils.date_parser import get_since_until, parse_human_timedelta
from superset.utils.hashing import md5_sha_from_dict
Expand Down Expand Up @@ -85,7 +86,7 @@ class QueryObject:
metrics: Optional[List[Metric]]
row_limit: int
row_offset: int
filter: List[Dict[str, Any]]
filter: List[QueryObjectFilterClause]
timeseries_limit: int
timeseries_limit_metric: Optional[Metric]
order_desc: bool
Expand All @@ -108,7 +109,7 @@ def __init__(
granularity: Optional[str] = None,
metrics: Optional[List[Metric]] = None,
groupby: Optional[List[str]] = None,
filters: Optional[List[Dict[str, Any]]] = None,
filters: Optional[List[QueryObjectFilterClause]] = None,
time_range: Optional[str] = None,
time_shift: Optional[str] = None,
is_timeseries: Optional[bool] = None,
Expand Down
65 changes: 40 additions & 25 deletions superset/connectors/sqla/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,11 @@
from superset.sql_parse import ParsedQuery
from superset.typing import AdhocMetric, Metric, OrderBy, QueryObjectDict
from superset.utils import core as utils
from superset.utils.core import GenericDataType, remove_duplicates
from superset.utils.core import (
GenericDataType,
QueryObjectFilterClause,
remove_duplicates,
)

config = app.config
metadata = Model.metadata # pylint: disable=no-member
Expand Down Expand Up @@ -303,13 +307,15 @@ def get_timestamp_expression(

pdf = self.python_date_format
is_epoch = pdf in ("epoch_s", "epoch_ms")
column_spec = self.db_engine_spec.get_column_spec(self.type)
type_ = column_spec.sqla_type if column_spec else DateTime
if not self.expression and not time_grain and not is_epoch:
sqla_col = column(self.column_name, type_=DateTime)
sqla_col = column(self.column_name, type_=type_)
villebro marked this conversation as resolved.
Show resolved Hide resolved
return self.table.make_sqla_column_compatible(sqla_col, label)
if self.expression:
col = literal_column(self.expression)
col = literal_column(self.expression, type_=type_)
else:
col = column(self.column_name)
col = column(self.column_name, type_=type_)
time_expr = self.db_engine_spec.get_timestamp_expr(
col, pdf, time_grain, self.type
)
Expand Down Expand Up @@ -935,7 +941,7 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
columns: Optional[List[str]] = None,
groupby: Optional[List[str]] = None,
filter: Optional[ # pylint: disable=redefined-builtin
List[Dict[str, Any]]
List[QueryObjectFilterClause]
] = None,
is_timeseries: bool = True,
timeseries_limit: int = 15,
Expand Down Expand Up @@ -1056,14 +1062,15 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
# filter out the pseudo column __timestamp from columns
columns = columns or []
columns = [col for col in columns if col != utils.DTTM_ALIAS]
time_grain = extras.get("time_grain_sqla")
dttm_col = columns_by_name.get(granularity) if granularity else None
Comment on lines +1065 to +1066
Copy link
Member Author

Choose a reason for hiding this comment

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

These are moved up in the code as they are needed earlier now


if need_groupby:
# dedup columns while preserving order
columns = groupby or columns
for selected in columns:
# if groupby field/expr equals granularity field/expr
if selected == granularity:
time_grain = extras.get("time_grain_sqla")
sqla_col = columns_by_name[selected]
outer = sqla_col.get_timestamp_expression(time_grain, selected)
# if groupby field equals a selected column
Expand All @@ -1087,15 +1094,13 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
groupby_exprs_with_timestamp = OrderedDict(groupby_exprs_sans_timestamp.items())

if granularity:
if granularity not in columns_by_name:
if granularity not in columns_by_name or not dttm_col:
raise QueryObjectValidationError(
_(
'Time column "%(col)s" does not exist in dataset',
col=granularity,
)
)
dttm_col = columns_by_name[granularity]
time_grain = extras.get("time_grain_sqla")
time_filters = []

if is_timeseries:
Expand Down Expand Up @@ -1150,14 +1155,23 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
col = flt["col"]
val = flt.get("val")
op = flt["op"].upper()
col_obj = columns_by_name.get(col)
col_obj = (
dttm_col
if col == utils.DTTM_ALIAS and is_timeseries and dttm_col
else columns_by_name.get(col)
)
filter_grain = flt.get("grain")

if is_feature_enabled("ENABLE_TEMPLATE_REMOVE_FILTERS"):
if col in removed_filters:
# Skip generating SQLA filter when the jinja template handles it.
continue

if col_obj:
if filter_grain:
sqla_col = col_obj.get_timestamp_expression(filter_grain)
else:
sqla_col = col_obj.get_sqla_col()
col_spec = db_engine_spec.get_column_spec(col_obj.type)
is_list_target = op in (
utils.FilterOperator.IN.value,
Expand All @@ -1180,24 +1194,24 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
)
if None in eq:
eq = [x for x in eq if x is not None]
is_null_cond = col_obj.get_sqla_col().is_(None)
is_null_cond = sqla_col.is_(None)
if eq:
cond = or_(is_null_cond, col_obj.get_sqla_col().in_(eq))
cond = or_(is_null_cond, sqla_col.in_(eq))
else:
cond = is_null_cond
else:
cond = col_obj.get_sqla_col().in_(eq)
cond = sqla_col.in_(eq)
if op == utils.FilterOperator.NOT_IN.value:
cond = ~cond
where_clause_and.append(cond)
elif op == utils.FilterOperator.IS_NULL.value:
where_clause_and.append(col_obj.get_sqla_col().is_(None))
where_clause_and.append(sqla_col.is_(None))
elif op == utils.FilterOperator.IS_NOT_NULL.value:
where_clause_and.append(col_obj.get_sqla_col().isnot(None))
where_clause_and.append(sqla_col.isnot(None))
elif op == utils.FilterOperator.IS_TRUE.value:
where_clause_and.append(col_obj.get_sqla_col().is_(True))
where_clause_and.append(sqla_col.is_(True))
elif op == utils.FilterOperator.IS_FALSE.value:
where_clause_and.append(col_obj.get_sqla_col().is_(False))
where_clause_and.append(sqla_col.is_(False))
else:
if eq is None:
raise QueryObjectValidationError(
Expand All @@ -1207,21 +1221,21 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
)
)
if op == utils.FilterOperator.EQUALS.value:
where_clause_and.append(col_obj.get_sqla_col() == eq)
where_clause_and.append(sqla_col == eq)
elif op == utils.FilterOperator.NOT_EQUALS.value:
where_clause_and.append(col_obj.get_sqla_col() != eq)
where_clause_and.append(sqla_col != eq)
elif op == utils.FilterOperator.GREATER_THAN.value:
where_clause_and.append(col_obj.get_sqla_col() > eq)
where_clause_and.append(sqla_col > eq)
elif op == utils.FilterOperator.LESS_THAN.value:
where_clause_and.append(col_obj.get_sqla_col() < eq)
where_clause_and.append(sqla_col < eq)
elif op == utils.FilterOperator.GREATER_THAN_OR_EQUALS.value:
where_clause_and.append(col_obj.get_sqla_col() >= eq)
where_clause_and.append(sqla_col >= eq)
elif op == utils.FilterOperator.LESS_THAN_OR_EQUALS.value:
where_clause_and.append(col_obj.get_sqla_col() <= eq)
where_clause_and.append(sqla_col <= eq)
elif op == utils.FilterOperator.LIKE.value:
where_clause_and.append(col_obj.get_sqla_col().like(eq))
where_clause_and.append(sqla_col.like(eq))
elif op == utils.FilterOperator.ILIKE.value:
where_clause_and.append(col_obj.get_sqla_col().ilike(eq))
where_clause_and.append(sqla_col.ilike(eq))
else:
raise QueryObjectValidationError(
_("Invalid filter operation type: %(op)s", op=op)
Expand Down Expand Up @@ -1281,6 +1295,7 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
and timeseries_limit
and not time_groupby_inline
and groupby_exprs_sans_timestamp
and dttm_col
):
if db_engine_spec.allows_joins:
# some sql dialects require for order by expressions
Expand Down
4 changes: 2 additions & 2 deletions superset/db_engine_specs/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
from flask_babel import gettext as __, lazy_gettext as _
from marshmallow import fields, Schema
from marshmallow.validate import Range
from sqlalchemy import column, DateTime, select, types
from sqlalchemy import column, select, types
from sqlalchemy.engine.base import Engine
from sqlalchemy.engine.interfaces import Compiled, Dialect
from sqlalchemy.engine.reflection import Inspector
Expand Down Expand Up @@ -381,7 +381,7 @@ def get_timestamp_expr(
elif pdf == "epoch_ms":
time_expr = time_expr.replace("{col}", cls.epoch_ms_to_dttm())

return TimestampExpression(time_expr, col, type_=DateTime)
return TimestampExpression(time_expr, col, type_=col.type)
villebro marked this conversation as resolved.
Show resolved Hide resolved

@classmethod
def get_time_grains(cls) -> Tuple[TimeGrain, ...]:
Expand Down
2 changes: 1 addition & 1 deletion superset/typing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class AdhocMetric(TypedDict):
]
DbapiDescription = Union[List[DbapiDescriptionRow], Tuple[DbapiDescriptionRow, ...]]
DbapiResult = Sequence[Union[List[Any], Tuple[Any, ...]]]
FilterValue = Union[datetime, float, int, str]
FilterValue = Union[bool, datetime, float, int, str]
villebro marked this conversation as resolved.
Show resolved Hide resolved
FilterValues = Union[FilterValue, List[FilterValue], Tuple[FilterValue]]
FormData = Dict[str, Any]
Granularity = Union[str, Dict[str, Union[str, float]]]
Expand Down