superset-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ccwilli...@apache.org
Subject [incubator-superset] branch master updated: [Explore] Adding custom expressions to adhoc metrics (#4736)
Date Fri, 13 Apr 2018 18:20:55 GMT
This is an automated email from the ASF dual-hosted git repository.

ccwilliams pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 8669874  [Explore] Adding custom expressions to adhoc metrics (#4736)
8669874 is described below

commit 8669874ec63437c774ea848042b7e866d297c451
Author: Gabe Lyons <gabe.lyons@airbnb.com>
AuthorDate: Fri Apr 13 11:20:53 2018 -0700

    [Explore] Adding custom expressions to adhoc metrics (#4736)
    
    * adding custom expressions to adhoc metrics
    
    * adjusted transitions and made the box expandable
---
 .../SqlLab/components/AceEditorWrapper.jsx         |   5 +-
 superset/assets/javascripts/explore/AdhocMetric.js |  71 ++++++++-
 .../explore/components/AdhocMetricEditPopover.jsx  | 164 +++++++++++++++++----
 .../explore/components/AdhocMetricOption.jsx       |   7 +
 .../explore/components/MetricDefinitionValue.jsx   |   1 -
 .../explore/components/controls/MetricsControl.jsx |  11 +-
 superset/assets/javascripts/explore/constants.js   |   3 +
 superset/assets/javascripts/explore/main.css       |  23 +++
 .../explore/propTypes/adhocMetricType.js           |  19 ++-
 .../spec/javascripts/explore/AdhocMetric_spec.js   | 106 ++++++++++++-
 .../components/AdhocMetricEditPopover_spec.jsx     |  27 +++-
 .../explore/components/MetricsControl_spec.jsx     |   7 +-
 superset/connectors/sqla/models.py                 |  16 +-
 superset/utils.py                                  |  23 ++-
 tests/druid_func_tests.py                          |   2 +
 15 files changed, 430 insertions(+), 55 deletions(-)

diff --git a/superset/assets/javascripts/SqlLab/components/AceEditorWrapper.jsx b/superset/assets/javascripts/SqlLab/components/AceEditorWrapper.jsx
index 9a9b037..6aef34c 100644
--- a/superset/assets/javascripts/SqlLab/components/AceEditorWrapper.jsx
+++ b/superset/assets/javascripts/SqlLab/components/AceEditorWrapper.jsx
@@ -12,7 +12,8 @@ const langTools = ace.acequire('ace/ext/language_tools');
 const keywords = (
   'SELECT|INSERT|UPDATE|DELETE|FROM|WHERE|AND|OR|GROUP|BY|ORDER|LIMIT|OFFSET|HAVING|AS|CASE|'
+
   'WHEN|ELSE|END|TYPE|LEFT|RIGHT|JOIN|ON|OUTER|DESC|ASC|UNION|CREATE|TABLE|PRIMARY|KEY|IF|'
+
-  'FOREIGN|NOT|REFERENCES|DEFAULT|NULL|INNER|CROSS|NATURAL|DATABASE|DROP|GRANT'
+  'FOREIGN|NOT|REFERENCES|DEFAULT|NULL|INNER|CROSS|NATURAL|DATABASE|DROP|GRANT|SUM|MAX|MIN|COUNT|'
+
+  'AVG|DISTINCT'
 );
 
 const dataTypes = (
@@ -21,7 +22,7 @@ const dataTypes = (
 );
 
 const sqlKeywords = [].concat(keywords.split('|'), dataTypes.split('|'));
-const sqlWords = sqlKeywords.map(s => ({
+export const sqlWords = sqlKeywords.map(s => ({
   name: s, value: s, score: 60, meta: 'sql',
 }));
 
diff --git a/superset/assets/javascripts/explore/AdhocMetric.js b/superset/assets/javascripts/explore/AdhocMetric.js
index e123521..5c62f05 100644
--- a/superset/assets/javascripts/explore/AdhocMetric.js
+++ b/superset/assets/javascripts/explore/AdhocMetric.js
@@ -1,7 +1,46 @@
+import { sqlaAutoGeneratedMetricRegex } from './constants';
+
+export const EXPRESSION_TYPES = {
+  SIMPLE: 'SIMPLE',
+  SQL: 'SQL',
+};
+
+function inferSqlExpressionColumn(adhocMetric) {
+  if (adhocMetric.sqlExpression && sqlaAutoGeneratedMetricRegex.test(adhocMetric.sqlExpression))
{
+    const indexFirstCloseParen = adhocMetric.sqlExpression.indexOf(')');
+    const indexPairedOpenParen =
+      adhocMetric.sqlExpression.substring(0, indexFirstCloseParen).lastIndexOf('(');
+    if (indexFirstCloseParen > 0 && indexPairedOpenParen > 0) {
+      return adhocMetric.sqlExpression.substring(indexPairedOpenParen + 1, indexFirstCloseParen);
+    }
+  }
+  return null;
+}
+
+function inferSqlExpressionAggregate(adhocMetric) {
+  if (adhocMetric.sqlExpression && sqlaAutoGeneratedMetricRegex.test(adhocMetric.sqlExpression))
{
+    const indexFirstOpenParen = adhocMetric.sqlExpression.indexOf('(');
+    if (indexFirstOpenParen > 0) {
+      return adhocMetric.sqlExpression.substring(0, indexFirstOpenParen);
+    }
+  }
+  return null;
+}
+
 export default class AdhocMetric {
   constructor(adhocMetric) {
-    this.column = adhocMetric.column;
-    this.aggregate = adhocMetric.aggregate;
+    this.expressionType = adhocMetric.expressionType || EXPRESSION_TYPES.SIMPLE;
+    if (this.expressionType === EXPRESSION_TYPES.SIMPLE) {
+      // try to be clever in the case of transitioning from Sql expression back to simple
expression
+      const inferredColumn = inferSqlExpressionColumn(adhocMetric);
+      this.column = adhocMetric.column || (inferredColumn && { column_name: inferredColumn
});
+      this.aggregate = adhocMetric.aggregate || inferSqlExpressionAggregate(adhocMetric);
+      this.sqlExpression = null;
+    } else if (this.expressionType === EXPRESSION_TYPES.SQL) {
+      this.sqlExpression = adhocMetric.sqlExpression;
+      this.column = null;
+      this.aggregate = null;
+    }
     this.hasCustomLabel = !!(adhocMetric.hasCustomLabel && adhocMetric.label);
     this.fromFormData = !!adhocMetric.optionName;
     this.label = this.hasCustomLabel ? adhocMetric.label : this.getDefaultLabel();
@@ -11,7 +50,14 @@ export default class AdhocMetric {
   }
 
   getDefaultLabel() {
-    return `${this.aggregate || ''}(${(this.column && this.column.column_name) ||
''})`;
+    if (this.expressionType === EXPRESSION_TYPES.SIMPLE) {
+      return `${this.aggregate || ''}(${(this.column && this.column.column_name)
|| ''})`;
+    } else if (this.expressionType === EXPRESSION_TYPES.SQL) {
+      return this.sqlExpression.length < 43 ?
+        this.sqlExpression :
+        this.sqlExpression.substring(0, 40) + '...';
+    }
+    return 'malformatted metric';
   }
 
   duplicateWith(nextFields) {
@@ -23,10 +69,29 @@ export default class AdhocMetric {
 
   equals(adhocMetric) {
     return adhocMetric.label === this.label &&
+      adhocMetric.expressionType === this.expressionType &&
+      adhocMetric.sqlExpression === this.sqlExpression &&
       adhocMetric.aggregate === this.aggregate &&
       (
         (adhocMetric.column && adhocMetric.column.column_name) ===
         (this.column && this.column.column_name)
       );
   }
+
+  isValid() {
+    if (this.expressionType === EXPRESSION_TYPES.SIMPLE) {
+      return !!(this.column && this.aggregate);
+    } else if (this.expressionType === EXPRESSION_TYPES.SQL) {
+      return !!(this.sqlExpression);
+    }
+    return false;
+  }
+
+  inferSqlExpressionAggregate() {
+    return inferSqlExpressionAggregate(this);
+  }
+
+  inferSqlExpressionColumn() {
+    return inferSqlExpressionColumn(this);
+  }
 }
diff --git a/superset/assets/javascripts/explore/components/AdhocMetricEditPopover.jsx b/superset/assets/javascripts/explore/components/AdhocMetricEditPopover.jsx
index 0964a51..4fb8032 100644
--- a/superset/assets/javascripts/explore/components/AdhocMetricEditPopover.jsx
+++ b/superset/assets/javascripts/explore/components/AdhocMetricEditPopover.jsx
@@ -1,7 +1,11 @@
 import React from 'react';
 import PropTypes from 'prop-types';
-import { Button, ControlLabel, FormGroup, Popover } from 'react-bootstrap';
+import { Button, ControlLabel, FormGroup, Popover, Tab, Tabs } from 'react-bootstrap';
 import VirtualizedSelect from 'react-virtualized-select';
+import AceEditor from 'react-ace';
+import 'brace/mode/sql';
+import 'brace/theme/github';
+import 'brace/ext/language_tools';
 
 import { AGGREGATES } from '../constants';
 import { t } from '../../locales';
@@ -9,13 +13,17 @@ import VirtualizedRendererWrap from '../../components/VirtualizedRendererWrap';
 import OnPasteSelect from '../../components/OnPasteSelect';
 import AdhocMetricEditPopoverTitle from './AdhocMetricEditPopoverTitle';
 import columnType from '../propTypes/columnType';
-import AdhocMetric from '../AdhocMetric';
+import AdhocMetric, { EXPRESSION_TYPES } from '../AdhocMetric';
 import ColumnOption from '../../components/ColumnOption';
+import { sqlWords } from '../../SqlLab/components/AceEditorWrapper';
+
+const langTools = ace.acequire('ace/ext/language_tools');
 
 const propTypes = {
   adhocMetric: PropTypes.instanceOf(AdhocMetric).isRequired,
   onChange: PropTypes.func.isRequired,
   onClose: PropTypes.func.isRequired,
+  onResize: PropTypes.func.isRequired,
   columns: PropTypes.arrayOf(columnType),
   datasourceType: PropTypes.string,
 };
@@ -24,14 +32,25 @@ const defaultProps = {
   columns: [],
 };
 
+const startingWidth = 300;
+const startingHeight = 180;
+
 export default class AdhocMetricEditPopover extends React.Component {
   constructor(props) {
     super(props);
     this.onSave = this.onSave.bind(this);
     this.onColumnChange = this.onColumnChange.bind(this);
     this.onAggregateChange = this.onAggregateChange.bind(this);
+    this.onSqlExpressionChange = this.onSqlExpressionChange.bind(this);
     this.onLabelChange = this.onLabelChange.bind(this);
-    this.state = { adhocMetric: this.props.adhocMetric };
+    this.onDragDown = this.onDragDown.bind(this);
+    this.onMouseMove = this.onMouseMove.bind(this);
+    this.onMouseUp = this.onMouseUp.bind(this);
+    this.state = {
+      adhocMetric: this.props.adhocMetric,
+      width: startingWidth,
+      height: startingHeight,
+    };
     this.selectProps = {
       multi: false,
       name: 'select-column',
@@ -40,6 +59,23 @@ export default class AdhocMetricEditPopover extends React.Component {
       clearable: true,
       selectWrap: VirtualizedSelect,
     };
+    if (langTools) {
+      const words = sqlWords.concat(this.props.columns.map(column => (
+        { name: column.column_name, value: column.column_name, score: 50, meta: 'column'
}
+      )));
+      const completer = {
+        getCompletions: (aceEditor, session, pos, prefix, callback) => {
+          callback(null, words);
+        },
+      };
+      langTools.setCompleters([completer]);
+    }
+    document.addEventListener('mouseup', this.onMouseUp);
+  }
+
+  componentWillUnmount() {
+    document.removeEventListener('mouseup', this.onMouseUp);
+    document.removeEventListener('mousemove', this.onMouseMove);
   }
 
   onSave() {
@@ -48,7 +84,10 @@ export default class AdhocMetricEditPopover extends React.Component {
   }
 
   onColumnChange(column) {
-    this.setState({ adhocMetric: this.state.adhocMetric.duplicateWith({ column }) });
+    this.setState({ adhocMetric: this.state.adhocMetric.duplicateWith({
+      column,
+      expressionType: EXPRESSION_TYPES.SIMPLE,
+    }) });
   }
 
   onAggregateChange(aggregate) {
@@ -56,6 +95,16 @@ export default class AdhocMetricEditPopover extends React.Component {
     this.setState({
       adhocMetric: this.state.adhocMetric.duplicateWith({
         aggregate: aggregate && aggregate.aggregate,
+        expressionType: EXPRESSION_TYPES.SIMPLE,
+      }),
+    });
+  }
+
+  onSqlExpressionChange(sqlExpression) {
+    this.setState({
+      adhocMetric: this.state.adhocMetric.duplicateWith({
+        sqlExpression,
+        expressionType: EXPRESSION_TYPES.SQL,
       }),
     });
   }
@@ -68,13 +117,44 @@ export default class AdhocMetricEditPopover extends React.Component {
     });
   }
 
+  onDragDown(e) {
+    this.dragStartX = e.clientX;
+    this.dragStartY = e.clientY;
+    this.dragStartWidth = this.state.width;
+    this.dragStartHeight = this.state.height;
+    document.addEventListener('mousemove', this.onMouseMove);
+  }
+
+  onMouseMove(e) {
+    this.props.onResize();
+    this.setState({
+      width: Math.max(this.dragStartWidth + (e.clientX - this.dragStartX), startingWidth),
+      height: Math.max(this.dragStartHeight + (e.clientY - this.dragStartY) * 2, startingHeight),
+    });
+  }
+
+  onMouseUp() {
+    document.removeEventListener('mousemove', this.onMouseMove);
+  }
+
   render() {
-    const { adhocMetric, columns, onChange, onClose, datasourceType, ...popoverProps } =
this.props;
+    const {
+      adhocMetric: propsAdhocMetric,
+      columns,
+      onChange,
+      onClose,
+      onResize,
+      datasourceType,
+      ...popoverProps
+    } = this.props;
+
+    const { adhocMetric } = this.state;
 
     const columnSelectProps = {
       placeholder: t('%s column(s)', columns.length),
       options: columns,
-      value: this.state.adhocMetric.column && this.state.adhocMetric.column.column_name,
+      value: (adhocMetric.column && adhocMetric.column.column_name) ||
+        adhocMetric.inferSqlExpressionColumn(),
       onChange: this.onColumnChange,
       optionRenderer: VirtualizedRendererWrap(option => (
         <ColumnOption column={option} showType />
@@ -86,7 +166,7 @@ export default class AdhocMetricEditPopover extends React.Component {
     const aggregateSelectProps = {
       placeholder: t('%s aggregates(s)', Object.keys(AGGREGATES).length),
       options: Object.keys(AGGREGATES).map(aggregate => ({ aggregate })),
-      value: this.state.adhocMetric.aggregate,
+      value: adhocMetric.aggregate || adhocMetric.inferSqlExpressionAggregate(),
       onChange: this.onAggregateChange,
       optionRenderer: VirtualizedRendererWrap(aggregate => aggregate.aggregate),
       valueRenderer: aggregate => aggregate.aggregate,
@@ -101,13 +181,13 @@ export default class AdhocMetricEditPopover extends React.Component
{
 
     const popoverTitle = (
       <AdhocMetricEditPopoverTitle
-        adhocMetric={this.state.adhocMetric}
+        adhocMetric={adhocMetric}
         onChange={this.onLabelChange}
       />
     );
 
-    const stateIsValid = this.state.adhocMetric.column && this.state.adhocMetric.aggregate;
-    const hasUnsavedChanges = this.state.adhocMetric.equals(this.props.adhocMetric);
+    const stateIsValid = adhocMetric.isValid();
+    const hasUnsavedChanges = !adhocMetric.equals(propsAdhocMetric);
 
     return (
       <Popover
@@ -115,24 +195,54 @@ export default class AdhocMetricEditPopover extends React.Component
{
         title={popoverTitle}
         {...popoverProps}
       >
-        <FormGroup>
-          <ControlLabel><strong>column</strong></ControlLabel>
-          <OnPasteSelect {...this.selectProps} {...columnSelectProps} />
-        </FormGroup>
-        <FormGroup>
-          <ControlLabel><strong>aggregate</strong></ControlLabel>
-          <OnPasteSelect {...this.selectProps} {...aggregateSelectProps} />
-        </FormGroup>
-        <Button
-          disabled={!stateIsValid}
-          bsStyle={(hasUnsavedChanges || !stateIsValid) ? 'default' : 'primary'}
-          bsSize="small"
-          className="m-r-5"
-          onClick={this.onSave}
+        <Tabs
+          id="adhoc-metric-edit-tabs"
+          defaultActiveKey={adhocMetric.expressionType}
+          className="adhoc-metric-edit-tabs"
+          style={{ height: this.state.height, width: this.state.width }}
         >
-          Save
-        </Button>
-        <Button bsSize="small" onClick={this.props.onClose}>Close</Button>
+          <Tab className="adhoc-metric-edit-tab" eventKey={EXPRESSION_TYPES.SIMPLE} title="Simple">
+            <FormGroup>
+              <ControlLabel><strong>column</strong></ControlLabel>
+              <OnPasteSelect {...this.selectProps} {...columnSelectProps} />
+            </FormGroup>
+            <FormGroup>
+              <ControlLabel><strong>aggregate</strong></ControlLabel>
+              <OnPasteSelect {...this.selectProps} {...aggregateSelectProps} />
+            </FormGroup>
+          </Tab>
+          {
+            this.props.datasourceType !== 'druid' &&
+            <Tab className="adhoc-metric-edit-tab" eventKey={EXPRESSION_TYPES.SQL} title="Custom
SQL">
+              <FormGroup>
+                <AceEditor
+                  mode="sql"
+                  theme="github"
+                  height={(this.state.height - 40) + 'px'}
+                  onChange={this.onSqlExpressionChange}
+                  width="100%"
+                  showGutter={false}
+                  value={adhocMetric.sqlExpression || adhocMetric.getDefaultLabel()}
+                  editorProps={{ $blockScrolling: true }}
+                  enableLiveAutocompletion
+                />
+              </FormGroup>
+            </Tab>
+          }
+        </Tabs>
+        <div>
+          <Button
+            disabled={!stateIsValid}
+            bsStyle={(hasUnsavedChanges && stateIsValid) ? 'primary' : 'default'}
+            bsSize="small"
+            className="m-r-5"
+            onClick={this.onSave}
+          >
+            Save
+          </Button>
+          <Button bsSize="small" onClick={this.props.onClose}>Close</Button>
+          <i onMouseDown={this.onDragDown} className="glyphicon glyphicon-resize-full
edit-popover-resize" />
+        </div>
       </Popover>
     );
   }
diff --git a/superset/assets/javascripts/explore/components/AdhocMetricOption.jsx b/superset/assets/javascripts/explore/components/AdhocMetricOption.jsx
index 88dd0d7..e7b270e 100644
--- a/superset/assets/javascripts/explore/components/AdhocMetricOption.jsx
+++ b/superset/assets/javascripts/explore/components/AdhocMetricOption.jsx
@@ -18,6 +18,11 @@ export default class AdhocMetricOption extends React.PureComponent {
   constructor(props) {
     super(props);
     this.closeMetricEditOverlay = this.closeMetricEditOverlay.bind(this);
+    this.onPopoverResize = this.onPopoverResize.bind(this);
+  }
+
+  onPopoverResize() {
+    this.forceUpdate();
   }
 
   closeMetricEditOverlay() {
@@ -28,6 +33,7 @@ export default class AdhocMetricOption extends React.PureComponent {
     const { adhocMetric } = this.props;
     const overlay = (
       <AdhocMetricEditPopover
+        onResize={this.onPopoverResize}
         adhocMetric={adhocMetric}
         onChange={this.props.onMetricEdit}
         onClose={this.closeMetricEditOverlay}
@@ -44,6 +50,7 @@ export default class AdhocMetricOption extends React.PureComponent {
         disabled
         overlay={overlay}
         rootClose
+        shouldUpdatePosition
         defaultOverlayShown={!adhocMetric.fromFormData}
       >
         <Label style={{ margin: this.props.multi ? 0 : 3, cursor: 'pointer' }}>
diff --git a/superset/assets/javascripts/explore/components/MetricDefinitionValue.jsx b/superset/assets/javascripts/explore/components/MetricDefinitionValue.jsx
index 4997dce..5b57a7b 100644
--- a/superset/assets/javascripts/explore/components/MetricDefinitionValue.jsx
+++ b/superset/assets/javascripts/explore/components/MetricDefinitionValue.jsx
@@ -41,7 +41,6 @@ export default function MetricDefinitionValue({
       />
     );
   }
-  notify.error('You must supply either a saved metric or adhoc metric to MetricDefinitionValue');
   return null;
 }
 MetricDefinitionValue.propTypes = propTypes;
diff --git a/superset/assets/javascripts/explore/components/controls/MetricsControl.jsx b/superset/assets/javascripts/explore/components/controls/MetricsControl.jsx
index ffc2fe2..b7ed1f2 100644
--- a/superset/assets/javascripts/explore/components/controls/MetricsControl.jsx
+++ b/superset/assets/javascripts/explore/components/controls/MetricsControl.jsx
@@ -11,7 +11,11 @@ import AdhocMetric from '../../AdhocMetric';
 import columnType from '../../propTypes/columnType';
 import savedMetricType from '../../propTypes/savedMetricType';
 import adhocMetricType from '../../propTypes/adhocMetricType';
-import { AGGREGATES } from '../../constants';
+import {
+  AGGREGATES,
+  sqlaAutoGeneratedMetricRegex,
+  druidAutoGeneratedMetricRegex,
+} from '../../constants';
 
 const propTypes = {
   name: PropTypes.string.isRequired,
@@ -31,7 +35,7 @@ const defaultProps = {
 };
 
 function isDictionaryForAdhocMetric(value) {
-  return value && !(value instanceof AdhocMetric) && value.column &&
value.aggregate && value.label;
+  return value && !(value instanceof AdhocMetric) && value.expressionType;
 }
 
 // adhoc metrics are stored as dictionaries in URL params. We convert them back into the
@@ -74,9 +78,6 @@ function getDefaultAggregateForColumn(column) {
   return null;
 }
 
-const sqlaAutoGeneratedMetricRegex = /^(LONG|DOUBLE|FLOAT)?(SUM|AVG|MAX|MIN|COUNT)\([A-Z_][A-Z0-9_]*\)$/i;
-const druidAutoGeneratedMetricRegex = /^(LONG|DOUBLE|FLOAT)?(SUM|MAX|MIN|COUNT)\([A-Z_][A-Z0-9_]*\)$/i;
-
 export default class MetricsControl extends React.PureComponent {
   constructor(props) {
     super(props);
diff --git a/superset/assets/javascripts/explore/constants.js b/superset/assets/javascripts/explore/constants.js
index 330841f..0a92dfd 100644
--- a/superset/assets/javascripts/explore/constants.js
+++ b/superset/assets/javascripts/explore/constants.js
@@ -7,3 +7,6 @@ export const AGGREGATES = {
   SUM: 'SUM',
 };
 
+export const sqlaAutoGeneratedMetricRegex = /^(LONG|DOUBLE|FLOAT)?(SUM|AVG|MAX|MIN|COUNT)\([A-Z_][A-Z0-9_]*\)$/i;
+export const druidAutoGeneratedMetricRegex = /^(LONG|DOUBLE|FLOAT)?(SUM|MAX|MIN|COUNT)\([A-Z_][A-Z0-9_]*\)$/i;
+
diff --git a/superset/assets/javascripts/explore/main.css b/superset/assets/javascripts/explore/main.css
index c6fede9..946f219 100644
--- a/superset/assets/javascripts/explore/main.css
+++ b/superset/assets/javascripts/explore/main.css
@@ -138,3 +138,26 @@
 .datasource-container {
   overflow: auto;
 }
+
+.adhoc-metric-edit-tabs > .nav-tabs {
+  margin-bottom: 6px;
+}
+
+.adhoc-metric-edit-tabs > .nav-tabs > li > a {
+  padding: 4px 4px 4px 4px;
+}
+
+.edit-popover-resize {
+  transform: scaleX(-1);
+  -moz-transform: scaleX(-1);
+  -webkit-transform: scaleX(-1);
+  -ms-transform: scaleX(-1);
+  float: right;
+  margin-top: 18px;
+  margin-right: -10px;
+  cursor: nwse-resize;
+}
+
+#metrics-edit-popover {
+  max-width: none;
+}
diff --git a/superset/assets/javascripts/explore/propTypes/adhocMetricType.js b/superset/assets/javascripts/explore/propTypes/adhocMetricType.js
index b11126e..3887d04 100644
--- a/superset/assets/javascripts/explore/propTypes/adhocMetricType.js
+++ b/superset/assets/javascripts/explore/propTypes/adhocMetricType.js
@@ -2,9 +2,18 @@ import PropTypes from 'prop-types';
 
 import { AGGREGATES } from '../constants';
 import columnType from './columnType';
+import { EXPRESSION_TYPES }  from '../AdhocMetric';
 
-export default PropTypes.shape({
-  column: columnType.isRequired,
-  aggregate: PropTypes.oneOf(Object.keys(AGGREGATES)).isRequired,
-  label: PropTypes.string.isRequired,
-});
+export default PropTypes.oneOfType([
+  PropTypes.shape({
+    expressionType: PropTypes.oneOf([EXPRESSION_TYPES.SIMPLE]).isRequired,
+    column: columnType.isRequired,
+    aggregate: PropTypes.oneOf(Object.keys(AGGREGATES)).isRequired,
+    label: PropTypes.string.isRequired,
+  }),
+  PropTypes.shape({
+    expressionType: PropTypes.oneOf([EXPRESSION_TYPES.SQL]).isRequired,
+    sqlExpression: PropTypes.string.isRequired,
+    label: PropTypes.string.isRequired,
+  }),
+]);
diff --git a/superset/assets/spec/javascripts/explore/AdhocMetric_spec.js b/superset/assets/spec/javascripts/explore/AdhocMetric_spec.js
index ad9ab47..5fa837a 100644
--- a/superset/assets/spec/javascripts/explore/AdhocMetric_spec.js
+++ b/superset/assets/spec/javascripts/explore/AdhocMetric_spec.js
@@ -1,7 +1,7 @@
 import { expect } from 'chai';
 import { describe, it } from 'mocha';
 
-import AdhocMetric from '../../../javascripts/explore/AdhocMetric';
+import AdhocMetric, { EXPRESSION_TYPES } from '../../../javascripts/explore/AdhocMetric';
 import { AGGREGATES } from '../../../javascripts/explore/constants';
 
 const valueColumn = { type: 'DOUBLE', column_name: 'value' };
@@ -14,12 +14,14 @@ describe('AdhocMetric', () => {
     });
     expect(adhocMetric.optionName.length).to.be.above(10);
     expect(adhocMetric).to.deep.equal({
+      expressionType: EXPRESSION_TYPES.SIMPLE,
       column: valueColumn,
       aggregate: AGGREGATES.SUM,
       fromFormData: false,
       label: 'SUM(value)',
       hasCustomLabel: false,
       optionName: adhocMetric.optionName,
+      sqlExpression: null,
     });
   });
 
@@ -59,6 +61,17 @@ describe('AdhocMetric', () => {
 
     // eslint-disable-next-line no-unused-expressions
     expect(adhocMetric1.equals(adhocMetric2)).to.be.false;
+
+    const adhocMetric3 = new AdhocMetric({
+      expressionType: EXPRESSION_TYPES.SQL,
+      sqlExpression: 'COUNT(*)',
+      label: 'old label',
+      hasCustomLabel: true,
+    });
+    const adhocMetric4 = adhocMetric3.duplicateWith({ sqlExpression: 'COUNT(1)' });
+
+    // eslint-disable-next-line no-unused-expressions
+    expect(adhocMetric3.equals(adhocMetric4)).to.be.false;
   });
 
   it('updates label if hasCustomLabel is false', () => {
@@ -82,4 +95,95 @@ describe('AdhocMetric', () => {
 
     expect(adhocMetric2.label).to.equal('label1');
   });
+
+  it('can determine if it is valid', () => {
+    const adhocMetric1 = new AdhocMetric({
+      expressionType: EXPRESSION_TYPES.SIMPLE,
+      column: valueColumn,
+      aggregate: AGGREGATES.SUM,
+      hasCustomLabel: true,
+      label: 'label1',
+    });
+    // eslint-disable-next-line no-unused-expressions
+    expect(adhocMetric1.isValid()).to.be.true;
+
+    const adhocMetric2 = new AdhocMetric({
+      expressionType: EXPRESSION_TYPES.SIMPLE,
+      column: valueColumn,
+      aggregate: null,
+      hasCustomLabel: true,
+      label: 'label1',
+    });
+    // eslint-disable-next-line no-unused-expressions
+    expect(adhocMetric2.isValid()).to.be.false;
+
+    const adhocMetric3 = new AdhocMetric({
+      expressionType: EXPRESSION_TYPES.SQL,
+      sqlExpression: 'COUNT(*)',
+      hasCustomLabel: true,
+      label: 'label1',
+    });
+    // eslint-disable-next-line no-unused-expressions
+    expect(adhocMetric3.isValid()).to.be.true;
+
+    const adhocMetric4 = new AdhocMetric({
+      expressionType: EXPRESSION_TYPES.SQL,
+      column: valueColumn,
+      aggregate: AGGREGATES.SUM,
+      hasCustomLabel: true,
+      label: 'label1',
+    });
+    // eslint-disable-next-line no-unused-expressions
+    expect(adhocMetric4.isValid()).to.be.false;
+
+    const adhocMetric5 = new AdhocMetric({
+      expressionType: EXPRESSION_TYPES.SQL,
+      hasCustomLabel: true,
+      label: 'label1',
+    });
+    // eslint-disable-next-line no-unused-expressions
+    expect(adhocMetric5.isValid()).to.be.false;
+  });
+
+  it('can translate back from sql expressions to simple expressions when possible', () =>
{
+    const adhocMetric = new AdhocMetric({
+      expressionType: EXPRESSION_TYPES.SQL,
+      sqlExpression: 'AVG(my_column)',
+      hasCustomLabel: true,
+      label: 'label1',
+    });
+    expect(adhocMetric.inferSqlExpressionColumn()).to.equal('my_column');
+    expect(adhocMetric.inferSqlExpressionAggregate()).to.equal('AVG');
+
+    const adhocMetric2 = new AdhocMetric({
+      expressionType: EXPRESSION_TYPES.SQL,
+      sqlExpression: 'AVG(SUM(my_column)) / MAX(other_column)',
+      hasCustomLabel: true,
+      label: 'label1',
+    });
+    expect(adhocMetric2.inferSqlExpressionColumn()).to.equal(null);
+    expect(adhocMetric2.inferSqlExpressionAggregate()).to.equal(null);
+  });
+
+  it('will infer columns and aggregates when converting to a simple expression', () =>
{
+    const adhocMetric = new AdhocMetric({
+      expressionType: EXPRESSION_TYPES.SQL,
+      sqlExpression: 'AVG(my_column)',
+      hasCustomLabel: true,
+      label: 'label1',
+    });
+    const adhocMetric2 = adhocMetric.duplicateWith({
+      expressionType: EXPRESSION_TYPES.SIMPLE,
+      aggregate: AGGREGATES.SUM,
+    });
+    expect(adhocMetric2.aggregate).to.equal(AGGREGATES.SUM);
+    expect(adhocMetric2.column.column_name).to.equal('my_column');
+
+    const adhocMetric3 = adhocMetric.duplicateWith({
+      expressionType: EXPRESSION_TYPES.SIMPLE,
+      column: valueColumn,
+    });
+    expect(adhocMetric3.aggregate).to.equal(AGGREGATES.AVG);
+    expect(adhocMetric3.column.column_name).to.equal('value');
+  });
 });
diff --git a/superset/assets/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx
b/superset/assets/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx
index 0ba69fb..8a79e0c 100644
--- a/superset/assets/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx
+++ b/superset/assets/spec/javascripts/explore/components/AdhocMetricEditPopover_spec.jsx
@@ -6,7 +6,7 @@ import { describe, it } from 'mocha';
 import { shallow } from 'enzyme';
 import { Button, FormGroup, Popover } from 'react-bootstrap';
 
-import AdhocMetric from '../../../../javascripts/explore/AdhocMetric';
+import AdhocMetric, { EXPRESSION_TYPES } from '../../../../javascripts/explore/AdhocMetric';
 import AdhocMetricEditPopover from '../../../../javascripts/explore/components/AdhocMetricEditPopover';
 import { AGGREGATES } from '../../../../javascripts/explore/constants';
 
@@ -17,10 +17,16 @@ const columns = [
 ];
 
 const sumValueAdhocMetric = new AdhocMetric({
+  expressionType: EXPRESSION_TYPES.SIMPLE,
   column: columns[2],
   aggregate: AGGREGATES.SUM,
 });
 
+const sqlExpressionAdhocMetric = new AdhocMetric({
+  expressionType: EXPRESSION_TYPES.SQL,
+  sqlExpression: 'COUNT(*)',
+});
+
 function setup(overrides) {
   const onChange = sinon.spy();
   const onClose = sinon.spy();
@@ -39,7 +45,7 @@ describe('AdhocMetricEditPopover', () => {
   it('renders a popover with edit metric form contents', () => {
     const { wrapper } = setup();
     expect(wrapper.find(Popover)).to.have.lengthOf(1);
-    expect(wrapper.find(FormGroup)).to.have.lengthOf(2);
+    expect(wrapper.find(FormGroup)).to.have.lengthOf(3);
     expect(wrapper.find(Button)).to.have.lengthOf(2);
   });
 
@@ -55,6 +61,12 @@ describe('AdhocMetricEditPopover', () => {
     expect(wrapper.state('adhocMetric')).to.deep.equal(sumValueAdhocMetric.duplicateWith({
aggregate: AGGREGATES.AVG }));
   });
 
+  it('overwrites the adhocMetric in state with onSqlExpressionChange', () => {
+    const { wrapper } = setup({ adhocMetric: sqlExpressionAdhocMetric });
+    wrapper.instance().onSqlExpressionChange('COUNT(1)');
+    expect(wrapper.state('adhocMetric')).to.deep.equal(sqlExpressionAdhocMetric.duplicateWith({
sqlExpression: 'COUNT(1)' }));
+  });
+
   it('overwrites the adhocMetric in state with onLabelChange', () => {
     const { wrapper } = setup();
     wrapper.instance().onLabelChange({ target: { value: 'new label' } });
@@ -87,4 +99,15 @@ describe('AdhocMetricEditPopover', () => {
     wrapper.instance().onColumnChange({ column: columns[1] });
     expect(wrapper.find(Button).find({ bsStyle: 'primary' })).to.have.lengthOf(1);
   });
+
+  it('will initiate a drag when clicked', () => {
+    const { wrapper } = setup();
+    wrapper.instance().onDragDown = sinon.spy();
+    wrapper.instance().forceUpdate();
+
+    expect(wrapper.find('i.glyphicon-resize-full')).to.have.lengthOf(1);
+    expect(wrapper.instance().onDragDown.calledOnce).to.be.false;
+    wrapper.find('i.glyphicon-resize-full').simulate('mouseDown');
+    expect(wrapper.instance().onDragDown.calledOnce).to.be.true;
+  });
 });
diff --git a/superset/assets/spec/javascripts/explore/components/MetricsControl_spec.jsx b/superset/assets/spec/javascripts/explore/components/MetricsControl_spec.jsx
index 23c8776..285f2b2 100644
--- a/superset/assets/spec/javascripts/explore/components/MetricsControl_spec.jsx
+++ b/superset/assets/spec/javascripts/explore/components/MetricsControl_spec.jsx
@@ -8,7 +8,7 @@ import { shallow } from 'enzyme';
 import MetricsControl from '../../../../javascripts/explore/components/controls/MetricsControl';
 import { AGGREGATES } from '../../../../javascripts/explore/constants';
 import OnPasteSelect from '../../../../javascripts/components/OnPasteSelect';
-import AdhocMetric from '../../../../javascripts/explore/AdhocMetric';
+import AdhocMetric, { EXPRESSION_TYPES } from '../../../../javascripts/explore/AdhocMetric';
 
 const defaultProps = {
   name: 'metrics',
@@ -73,6 +73,7 @@ describe('MetricsControl', () => {
       const { wrapper } = setup({
         value: [
           {
+            expressionType: EXPRESSION_TYPES.SIMPLE,
             column: { type: 'double', column_name: 'value' },
             aggregate: AGGREGATES.SUM,
             label: 'SUM(value)',
@@ -87,12 +88,14 @@ describe('MetricsControl', () => {
       expect(adhocMetric.optionName.length).to.be.above(10);
       expect(wrapper.state('value')).to.deep.equal([
         {
+          expressionType: EXPRESSION_TYPES.SIMPLE,
           column: { type: 'double', column_name: 'value' },
           aggregate: AGGREGATES.SUM,
           fromFormData: true,
           label: 'SUM(value)',
           hasCustomLabel: false,
           optionName: 'blahblahblah',
+          sqlExpression: null,
         },
         'avg__value',
       ]);
@@ -117,12 +120,14 @@ describe('MetricsControl', () => {
       const adhocMetric = onChange.lastCall.args[0][0];
       expect(adhocMetric instanceof AdhocMetric).to.be.true;
       expect(onChange.lastCall.args).to.deep.equal([[{
+        expressionType: EXPRESSION_TYPES.SIMPLE,
         column: valueColumn,
         aggregate: AGGREGATES.SUM,
         label: 'SUM(value)',
         fromFormData: false,
         hasCustomLabel: false,
         optionName: adhocMetric.optionName,
+        sqlExpression: null,
       }]]);
     });
 
diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index 8ed7080..7fbb7ae 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -447,10 +447,18 @@ class SqlaTable(Model, BaseDatasource):
         return self.get_sqla_table()
 
     def adhoc_metric_to_sa(self, metric):
-        column_name = metric.get('column').get('column_name')
-        sa_metric = self.sqla_aggregations[metric.get('aggregate')](column(column_name))
-        sa_metric = sa_metric.label(metric.get('label'))
-        return sa_metric
+        expressionType = metric.get('expressionType')
+        if expressionType == utils.ADHOC_METRIC_EXPRESSION_TYPES['SIMPLE']:
+            sa_column = column(metric.get('column').get('column_name'))
+            sa_metric = self.sqla_aggregations[metric.get('aggregate')](sa_column)
+            sa_metric = sa_metric.label(metric.get('label'))
+            return sa_metric
+        elif expressionType == utils.ADHOC_METRIC_EXPRESSION_TYPES['SQL']:
+            sa_metric = literal_column(metric.get('sqlExpression'))
+            sa_metric = sa_metric.label(metric.get('label'))
+            return sa_metric
+        else:
+            return None
 
     def get_sqla_query(  # sqla
             self,
diff --git a/superset/utils.py b/superset/utils.py
index 8e99f69..d465bb0 100644
--- a/superset/utils.py
+++ b/superset/utils.py
@@ -47,6 +47,10 @@ logging.getLogger('MARKDOWN').setLevel(logging.INFO)
 PY3K = sys.version_info >= (3, 0)
 EPOCH = datetime(1970, 1, 1)
 DTTM_ALIAS = '__timestamp'
+ADHOC_METRIC_EXPRESSION_TYPES = {
+    'SIMPLE': 'SIMPLE',
+    'SQL': 'SQL',
+}
 
 JS_MAX_INTEGER = 9007199254740991   # Largest int Java Script can handle 2^53-1
 
@@ -804,10 +808,21 @@ def get_or_create_main_db():
 
 
 def is_adhoc_metric(metric):
-    return (isinstance(metric, dict) and
-            metric['column'] and
-            metric['aggregate'] and
-            metric['label'])
+    return (
+        isinstance(metric, dict) and
+        (
+            (
+                metric['expressionType'] == ADHOC_METRIC_EXPRESSION_TYPES['SIMPLE'] and
+                metric['column'] and
+                metric['aggregate']
+            ) or
+            (
+                metric['expressionType'] == ADHOC_METRIC_EXPRESSION_TYPES['SQL'] and
+                metric['sqlExpression']
+            )
+        ) and
+        metric['label']
+    )
 
 
 def get_metric_names(metrics):
diff --git a/tests/druid_func_tests.py b/tests/druid_func_tests.py
index c478494..3082bec 100644
--- a/tests/druid_func_tests.py
+++ b/tests/druid_func_tests.py
@@ -203,6 +203,7 @@ class DruidFuncTestCase(unittest.TestCase):
         ds._metrics_and_post_aggs = Mock(return_value=(all_metrics, post_aggs))
         groupby = []
         metrics = [{
+            'expressionType': 'SIMPLE',
             'column': {'type': 'DOUBLE', 'column_name': 'col1'},
             'aggregate': 'SUM',
             'label': 'My Adhoc Metric',
@@ -588,6 +589,7 @@ class DruidFuncTestCase(unittest.TestCase):
         }
 
         adhoc_metric = {
+            'expressionType': 'SIMPLE',
             'column': {'type': 'DOUBLE', 'column_name': 'value'},
             'aggregate': 'SUM',
             'label': 'My Adhoc Metric',

-- 
To stop receiving notification emails like this one, please contact
ccwilliams@apache.org.

Mime
View raw message