-
Notifications
You must be signed in to change notification settings - Fork 16.6k
Specify the metric to order by for Series Limit #1351
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -867,8 +867,11 @@ def query( # sqla | |
| from_dttm, to_dttm, | ||
| filter=None, # noqa | ||
| is_timeseries=True, | ||
| timeseries_limit=15, row_limit=None, | ||
| inner_from_dttm=None, inner_to_dttm=None, | ||
| timeseries_limit=15, | ||
| timeseries_limit_metric=None, | ||
| row_limit=None, | ||
| inner_from_dttm=None, | ||
| inner_to_dttm=None, | ||
| orderby=None, | ||
| extras=None, | ||
| columns=None): | ||
|
|
@@ -887,7 +890,11 @@ def query( # sqla | |
| "and is required by this type of chart")) | ||
|
|
||
| metrics_exprs = [metrics_dict.get(m).sqla_col for m in metrics] | ||
|
|
||
| timeseries_limit_metric = metrics_dict.get(timeseries_limit_metric) | ||
| timeseries_limit_metric_expr = None | ||
| if timeseries_limit_metric: | ||
| timeseries_limit_metric_expr = \ | ||
| timeseries_limit_metric.sqla_col | ||
|
||
| if metrics: | ||
| main_metric_expr = metrics_exprs[0] | ||
| else: | ||
|
|
@@ -1023,7 +1030,10 @@ def visit_column(element, compiler, **kw): | |
| subq = subq.select_from(tbl) | ||
| subq = subq.where(and_(*(where_clause_and + inner_time_filter))) | ||
| subq = subq.group_by(*inner_groupby_exprs) | ||
| subq = subq.order_by(desc(main_metric_expr)) | ||
| ob = main_metric_expr | ||
| if timeseries_limit_metric_expr is not None: | ||
| ob = timeseries_limit_metric_expr | ||
|
||
| subq = subq.order_by(desc(ob)) | ||
| subq = subq.limit(timeseries_limit) | ||
| on_clause = [] | ||
| for i, gb in enumerate(groupby): | ||
|
|
@@ -1689,6 +1699,7 @@ def query( # druid | |
| filter=None, # noqa | ||
| is_timeseries=True, | ||
| timeseries_limit=None, | ||
| timeseries_limit_metric=None, | ||
|
||
| row_limit=None, | ||
| inner_from_dttm=None, inner_to_dttm=None, | ||
| orderby=None, | ||
|
|
@@ -1794,6 +1805,9 @@ def recursive_get_fields(_conf): | |
| client = self.cluster.get_pydruid_client() | ||
| orig_filters = filters | ||
| if timeseries_limit and is_timeseries: | ||
| order_by = metrics[0] if metrics else self.metrics[0] | ||
| if timeseries_limit_metric: | ||
| order_by = timeseries_limit_metric | ||
| # Limit on the number of timeseries, doing a two-phases query | ||
| pre_qry = deepcopy(qry) | ||
| pre_qry['granularity'] = "all" | ||
|
|
@@ -1804,7 +1818,7 @@ def recursive_get_fields(_conf): | |
| inner_from_dttm.isoformat() + '/' + | ||
| inner_to_dttm.isoformat()), | ||
| "columns": [{ | ||
| "dimension": metrics[0] if metrics else self.metrics[0], | ||
| "dimension": order_by, | ||
| "direction": "descending", | ||
| }], | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just curious, what's
\at the end? (am new to python)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
python's way of terminating a line on another line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍