Skip to content

Commit

Permalink
[druid] fixing the having clause in the explore view (#2648)
Browse files Browse the repository at this point in the history
* [druid] fixing the having clause in the explore view

* Backend

* Lint

* Fixing tests
  • Loading branch information
mistercrunch authored Apr 24, 2017
1 parent f10ee13 commit 2978082
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,12 @@ export default class DisplayQueryButton extends React.PureComponent {
language: data.language,
query: data.query,
isLoading: false,
error: null,
});
},
error: (data) => {
this.setState({
error: data.error,
error: data.responseJSON ? data.responseJSON.error : 'Error...',
isLoading: false,
});
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ const $ = window.$ = require('jquery');
const operatorsArr = [
{ val: 'in', type: 'array', useSelect: true, multi: true },
{ val: 'not in', type: 'array', useSelect: true, multi: true },
{ val: '==', type: 'string', useSelect: true, multi: false },
{ val: '!=', type: 'string', useSelect: true, multi: false },
{ val: '>=', type: 'string' },
{ val: '<=', type: 'string' },
{ val: '>', type: 'string' },
{ val: '<', type: 'string' },
{ val: '==', type: 'string', useSelect: true, multi: false, havingOnly: true },
{ val: '!=', type: 'string', useSelect: true, multi: false, havingOnly: true },
{ val: '>=', type: 'string', havingOnly: true },
{ val: '<=', type: 'string', havingOnly: true },
{ val: '>', type: 'string', havingOnly: true },
{ val: '<', type: 'string', havingOnly: true },
{ val: 'regex', type: 'string', datasourceTypes: ['druid'] },
{ val: 'LIKE', type: 'string', datasourceTypes: ['table'] },
];
Expand All @@ -27,12 +27,14 @@ const propTypes = {
removeFilter: PropTypes.func,
filter: PropTypes.object.isRequired,
datasource: PropTypes.object,
having: PropTypes.bool,
};

const defaultProps = {
changeFilter: () => {},
removeFilter: () => {},
datasource: null,
having: false,
};

export default class Filter extends React.Component {
Expand All @@ -47,7 +49,7 @@ export default class Filter extends React.Component {
}
fetchFilterValues(col) {
const datasource = this.props.datasource;
if (col && this.props.datasource && this.props.datasource.filter_select) {
if (col && this.props.datasource && this.props.datasource.filter_select && !this.props.having) {
this.setState({ valuesLoading: true });
$.ajax({
type: 'GET',
Expand Down Expand Up @@ -90,7 +92,7 @@ export default class Filter extends React.Component {
}
renderFilterFormControl(filter) {
const operator = operators[filter.op];
if (operator.useSelect) {
if (operator.useSelect && !this.props.having) {
return (
<SelectControl
multi={operator.multi}
Expand All @@ -117,18 +119,28 @@ export default class Filter extends React.Component {
const datasource = this.props.datasource;
const filter = this.props.filter;
const opsChoices = operatorsArr
.filter(o => !o.datasourceTypes || o.datasourceTypes.indexOf(datasource.type) >= 0)
.filter((o) => {
if (this.props.having) {
return !!o.havingOnly;
}
return (!o.datasourceTypes || o.datasourceTypes.indexOf(datasource.type) >= 0);
})
.map(o => ({ value: o.val, label: o.val }));
const colChoices = datasource ?
datasource.filterable_cols.map(c => ({ value: c[0], label: c[1] })) :
null;
let colChoices;
if (datasource) {
if (this.props.having) {
colChoices = datasource.metrics_combo.map(c => ({ value: c[0], label: c[1] }));
} else {
colChoices = datasource.filterable_cols.map(c => ({ value: c[0], label: c[1] }));
}
}
return (
<div>
<Row className="space-1">
<Col md={12}>
<Select
id="select-col"
placeholder="Select column"
placeholder={this.props.having ? 'Select metric' : 'Select column'}
clearable={false}
options={colChoices}
value={filter.col}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ const defaultProps = {
type: 'qtable',
filter_select: false,
filterable_cols: ['col1', 'col2'],
metrics_combo: [
['m1', 'v1'],
['m2', 'v2'],
],
},
};

Expand All @@ -46,7 +50,7 @@ describe('Filter', () => {
});

it('renders five op choices for table datasource', () => {
const props = defaultProps;
const props = Object.assign({}, defaultProps);
props.datasource = {
id: 1,
type: 'druid',
Expand All @@ -58,10 +62,10 @@ describe('Filter', () => {
});

it('renders six op choices for having filter', () => {
const props = defaultProps;
const props = Object.assign({}, defaultProps);
props.having = true;
const havingWrapper = shallow(<Filter {...props} />);
expect(havingWrapper.find('#select-op').prop('options')).to.have.lengthOf(9);
expect(havingWrapper.find('#select-op').prop('options')).to.have.lengthOf(6);
});

it('calls changeFilter when select is changed', () => {
Expand All @@ -75,7 +79,7 @@ describe('Filter', () => {
});

it('renders input for regex filters', () => {
const props = defaultProps;
const props = Object.assign({}, defaultProps);
props.filter = {
col: null,
op: 'regex',
Expand Down
2 changes: 1 addition & 1 deletion superset/connectors/druid/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ def recursive_get_fields(_conf):
qry['having'] = having_filters

orig_filters = filters
if len(groupby) == 0:
if len(groupby) == 0 and not having_filters:
del qry['dimensions']
client.timeseries(**qry)
if not having_filters and len(groupby) == 1:
Expand Down
2 changes: 1 addition & 1 deletion superset/views/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def get_error_msg():


def json_error_response(msg, status=None, stacktrace=None):
data = {'error': msg}
data = {'error': str(msg)}
if stacktrace:
data['stacktrace'] = stacktrace
status = status if status else 500
Expand Down

0 comments on commit 2978082

Please sign in to comment.