From 6f05b48385d80bc6adca3e0aa3b545d34243acfa Mon Sep 17 00:00:00 2001 From: Michelle Thomas Date: Wed, 23 May 2018 18:16:27 -0700 Subject: [PATCH 1/3] Adding the MetricsControl to the timeseries_limit_metric field --- superset/assets/src/explore/controls.jsx | 6 ++++-- superset/connectors/druid/models.py | 9 +++++++-- superset/connectors/sqla/models.py | 11 +++++++++-- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/superset/assets/src/explore/controls.jsx b/superset/assets/src/explore/controls.jsx index b7327ddd58d..ff719c320c0 100644 --- a/superset/assets/src/explore/controls.jsx +++ b/superset/assets/src/explore/controls.jsx @@ -959,12 +959,14 @@ export const controls = { }, timeseries_limit_metric: { - type: 'SelectControl', + type: 'MetricsControl', label: t('Sort By'), default: null, description: t('Metric used to define the top series'), mapStateToProps: state => ({ - choices: (state.datasource) ? state.datasource.metrics_combo : [], + columns: state.datasource ? state.datasource.columns : [], + savedMetrics: state.datasource ? state.datasource.metrics : [], + datasourceType: state.datasource && state.datasource.type, }), }, diff --git a/superset/connectors/druid/models.py b/superset/connectors/druid/models.py index 79b0dad6077..480e8307ef4 100644 --- a/superset/connectors/druid/models.py +++ b/superset/connectors/druid/models.py @@ -1148,6 +1148,11 @@ class DruidDatasource(Model, BaseDatasource): metric['column']['type'].upper() == 'FLOAT' ): metric['column']['type'] = 'DOUBLE' + if ( + utils.is_adhoc_metric(timeseries_limit_metric) and + timeseries_limit_metric['column']['type'].upper() == 'FLOAT' + ): + timeseries_limit_metric['column']['type'] = 'DOUBLE' aggregations, post_aggs = DruidDatasource.metrics_and_post_aggs( metrics, @@ -1203,7 +1208,7 @@ class DruidDatasource(Model, BaseDatasource): logging.info('Running two-phase topn query for dimension [{}]'.format(dim)) pre_qry = deepcopy(qry) if timeseries_limit_metric: - order_by = timeseries_limit_metric + order_by = utils.get_metric_name(timeseries_limit_metric) aggs_dict, post_aggs_dict = DruidDatasource.metrics_and_post_aggs( [timeseries_limit_metric], metrics_dict) @@ -1272,7 +1277,7 @@ class DruidDatasource(Model, BaseDatasource): order_by = pre_qry_dims[0] if timeseries_limit_metric: - order_by = timeseries_limit_metric + order_by = utils.get_metric_name(timeseries_limit_metric) aggs_dict, post_aggs_dict = DruidDatasource.metrics_and_post_aggs( [timeseries_limit_metric], metrics_dict) diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 875707f55cc..8b024423586 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -675,8 +675,15 @@ class SqlaTable(Model, BaseDatasource): ob = inner_main_metric_expr if timeseries_limit_metric: - timeseries_limit_metric = metrics_dict.get(timeseries_limit_metric) - ob = timeseries_limit_metric.sqla_col + if utils.is_adhoc_metric(timeseries_limit_metric): + ob = self.adhoc_metric_to_sa(timeseries_limit_metric, cols) + elif timeseries_limit_metric in metrics_dict: + timeseries_limit_metric = metrics_dict.get( + timeseries_limit_metric, + ) + ob = timeseries_limit_metric.sqla_col + else: + raise Exception(_("Metric '{}' is not valid".format(m))) direction = desc if order_desc else asc subq = subq.order_by(direction(ob)) subq = subq.limit(timeseries_limit) From 47768284d01453968e9b2e5aef8024110b6dc33e Mon Sep 17 00:00:00 2001 From: Michelle Thomas Date: Thu, 31 May 2018 23:48:12 -0700 Subject: [PATCH 2/3] Adding tests for adhoc metric as timeseries_limit_metric --- superset/data/__init__.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/superset/data/__init__.py b/superset/data/__init__.py index 4f79be842a9..27b3166bb09 100644 --- a/superset/data/__init__.py +++ b/superset/data/__init__.py @@ -760,6 +760,35 @@ def load_birth_names(): }, viz_type="big_number_total", granularity_sqla="ds")), + Slice( + slice_name='Top 10 California Names Timeseries', + viz_type='line', + datasource_type='table', + datasource_id=tbl.id, + params=get_slice_json( + defaults, + metrics=[{ + 'expressionType': 'SIMPLE', + 'column': { + 'column_name': 'num_california', + 'expression': "CASE WHEN state = 'CA' THEN num ELSE 0 END", + }, + 'aggregate': 'SUM', + 'label': 'SUM(num_california)', + }], + viz_type='line', + granularity_sqla='ds', + groupby=['name'], + timeseries_limit_metric={ + 'expressionType': 'SIMPLE', + 'column': { + 'column_name': 'num_california', + 'expression': "CASE WHEN state = 'CA' THEN num ELSE 0 END", + }, + 'aggregate': 'SUM', + 'label': 'SUM(num_california)', + }, + limit='10')), ] for slc in slices: merge_slice(slc) From b380a57c91f9918e7dbc8f858df840e3ccb8d24d Mon Sep 17 00:00:00 2001 From: Michelle Thomas Date: Tue, 5 Jun 2018 11:14:36 -0700 Subject: [PATCH 3/3] Fixing sortby adhoc metrics for table viz --- superset/assets/src/visualizations/table.js | 6 +++--- superset/connectors/druid/models.py | 24 ++++++++++++--------- superset/connectors/sqla/models.py | 2 ++ superset/data/__init__.py | 21 +++++++++++++++++- superset/viz.py | 3 ++- 5 files changed, 41 insertions(+), 15 deletions(-) diff --git a/superset/assets/src/visualizations/table.js b/superset/assets/src/visualizations/table.js index 6b8deec2d86..72a326ac65b 100644 --- a/superset/assets/src/visualizations/table.js +++ b/superset/assets/src/visualizations/table.js @@ -17,7 +17,7 @@ function tableVis(slice, payload) { const data = payload.data; const fd = slice.formData; - let metrics = fd.metrics || []; + let metrics = fd.metrics.map(m => m.label || m); // Add percent metrics metrics = metrics.concat((fd.percent_metrics || []).map(m => '%' + m)); // Removing metrics (aggregates) that are strings @@ -187,7 +187,7 @@ function tableVis(slice, payload) { let sortBy; if (fd.timeseries_limit_metric) { // Sort by as specified - sortBy = fd.timeseries_limit_metric; + sortBy = fd.timeseries_limit_metric.label || fd.timeseries_limit_metric; } else if (metrics.length > 0) { // If not specified, use the first metric from the list sortBy = metrics[0]; @@ -195,7 +195,7 @@ function tableVis(slice, payload) { if (sortBy) { datatable.column(data.columns.indexOf(sortBy)).order(fd.order_desc ? 'desc' : 'asc'); } - if (fd.timeseries_limit_metric && metrics.indexOf(fd.timeseries_limit_metric) < 0) { + if (sortBy && metrics.indexOf(sortBy) < 0) { // Hiding the sortBy column if not in the metrics list datatable.column(data.columns.indexOf(sortBy)).visible(false); } diff --git a/superset/connectors/druid/models.py b/superset/connectors/druid/models.py index 480e8307ef4..e23537fe608 100644 --- a/superset/connectors/druid/models.py +++ b/superset/connectors/druid/models.py @@ -1100,6 +1100,18 @@ class DruidDatasource(Model, BaseDatasource): return values + @staticmethod + def sanitize_metric_object(metric): + """ + Update a metric with the correct type if necessary. + :param dict metric: The metric to sanitize + """ + if ( + utils.is_adhoc_metric(metric) and + metric['column']['type'].upper() == 'FLOAT' + ): + metric['column']['type'] = 'DOUBLE' + def run_query( # noqa / druid self, groupby, metrics, @@ -1143,16 +1155,8 @@ class DruidDatasource(Model, BaseDatasource): LooseVersion(self.cluster.get_druid_version()) < LooseVersion('0.11.0') ): for metric in metrics: - if ( - utils.is_adhoc_metric(metric) and - metric['column']['type'].upper() == 'FLOAT' - ): - metric['column']['type'] = 'DOUBLE' - if ( - utils.is_adhoc_metric(timeseries_limit_metric) and - timeseries_limit_metric['column']['type'].upper() == 'FLOAT' - ): - timeseries_limit_metric['column']['type'] = 'DOUBLE' + self.sanitize_metric_object(metric) + self.sanitize_metric_object(timeseries_limit_metric) aggregations, post_aggs = DruidDatasource.metrics_and_post_aggs( metrics, diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py index 8b024423586..e08053ccd1e 100644 --- a/superset/connectors/sqla/models.py +++ b/superset/connectors/sqla/models.py @@ -651,6 +651,8 @@ class SqlaTable(Model, BaseDatasource): for col, ascending in orderby: direction = asc if ascending else desc + if utils.is_adhoc_metric(col): + col = self.adhoc_metric_to_sa(col, cols) qry = qry.order_by(direction(col)) if row_limit: diff --git a/superset/data/__init__.py b/superset/data/__init__.py index 27b3166bb09..dc681f4f409 100644 --- a/superset/data/__init__.py +++ b/superset/data/__init__.py @@ -626,7 +626,8 @@ def load_birth_names(): 'op': 'in', 'val': ['girl'], }], - row_limit=50)), + row_limit=50, + timeseries_limit_metric='sum__num')), Slice( slice_name="Boys", viz_type='table', @@ -789,6 +790,24 @@ def load_birth_names(): 'label': 'SUM(num_california)', }, limit='10')), + Slice( + slice_name="Names Sorted by Num in California", + viz_type='table', + datasource_type='table', + datasource_id=tbl.id, + params=get_slice_json( + defaults, + groupby=['name'], + row_limit=50, + timeseries_limit_metric={ + 'expressionType': 'SIMPLE', + 'column': { + 'column_name': 'num_california', + 'expression': "CASE WHEN state = 'CA' THEN num ELSE 0 END", + }, + 'aggregate': 'SUM', + 'label': 'SUM(num_california)', + })), ] for slc in slices: merge_slice(slc) diff --git a/superset/viz.py b/superset/viz.py index f3db31c2470..e7165d317d3 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -510,7 +510,8 @@ class TableViz(BaseViz): order_by_cols = fd.get('order_by_cols') or [] d['orderby'] = [json.loads(t) for t in order_by_cols] elif sort_by: - if sort_by not in d['metrics']: + sort_by_label = utils.get_metric_name(sort_by) + if sort_by_label not in utils.get_metric_names(d['metrics']): d['metrics'] += [sort_by] d['orderby'] = [(sort_by, not fd.get('order_desc', True))]