You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@superset.apache.org by GitBox <gi...@apache.org> on 2018/04/13 18:20:55 UTC
[GitHub] williaster closed pull request #4736: [Explore] Adding custom
expressions to adhoc metrics
williaster closed pull request #4736: [Explore] Adding custom expressions to adhoc metrics
URL: https://github.com/apache/incubator-superset/pull/4736
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/superset/assets/javascripts/SqlLab/components/AceEditorWrapper.jsx b/superset/assets/javascripts/SqlLab/components/AceEditorWrapper.jsx
index 9a9b037992..6aef34cbcf 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 e123521b4c..5c62f0544f 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 0964a51c5d..4fb8032089 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 88dd0d7ee1..e7b270e806 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 4997dcee3b..5b57a7ba87 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 ffc2fe26d6..b7ed1f205d 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 330841f4ac..0a92dfd63b 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 c6fede9074..946f21915d 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 b11126e8fe..3887d046db 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 ad9ab47254..5fa837af0f 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 0ba69fba0e..8a79e0cd22 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 23c8776322..285f2b26b0 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 c8ee40269c..eaa201bd1c 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -446,10 +446,18 @@ def get_from_clause(self, template_processor=None, db_engine_spec=None):
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 828111f6b9..fd77701258 100644
--- a/superset/utils.py
+++ b/superset/utils.py
@@ -47,6 +47,10 @@
PY3K = sys.version_info >= (3, 0)
EPOCH = datetime(1970, 1, 1)
DTTM_ALIAS = '__timestamp'
+ADHOC_METRIC_EXPRESSION_TYPES = {
+ 'SIMPLE': 'SIMPLE',
+ 'SQL': 'SQL',
+}
def flasher(msg, severity=None):
@@ -802,10 +806,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 22c1f38dc9..daca5cd3f1 100644
--- a/tests/druid_func_tests.py
+++ b/tests/druid_func_tests.py
@@ -202,6 +202,7 @@ def test_run_query_with_adhoc_metric(self):
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',
@@ -587,6 +588,7 @@ def test_metrics_and_post_aggs(self):
}
adhoc_metric = {
+ 'expressionType': 'SIMPLE',
'column': {'type': 'DOUBLE', 'column_name': 'value'},
'aggregate': 'SUM',
'label': 'My Adhoc Metric',
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services