Skip to content
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

fix(explore): Explore page boolean filter is broken for Presto DB #14952

Merged
merged 11 commits into from Jun 10, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions superset-frontend/src/components/Select/NativeSelect.tsx
Expand Up @@ -20,6 +20,11 @@ import React from 'react';
import { styled } from '@superset-ui/core';
import Select, { SelectProps } from 'antd/lib/select';

export {
OptionType as NativeSelectOptionType,
SelectProps as NativeSelectProps,
} from 'antd/lib/select';

const StyledNativeSelect = styled((props: SelectProps<any>) => (
<Select getPopupContainer={(trigger: any) => trigger.parentNode} {...props} />
))`
Expand Down
Expand Up @@ -20,7 +20,7 @@ import React, { useEffect, useMemo, useState } from 'react';
import { logging, SupersetClient, t, Metric } from '@superset-ui/core';
import { ColumnMeta } from '@superset-ui/chart-controls';
import { Tooltip } from 'src/components/Tooltip';
import { OPERATORS } from 'src/explore/constants';
import { Operators, OPERATOR_MAPPING } from 'src/explore/constants';
import { OptionSortType } from 'src/explore/types';
import {
DndFilterSelectProps,
Expand Down Expand Up @@ -191,7 +191,8 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
props.datasource.type === 'druid'
? filterOptions.saved_metric_name
: getMetricExpression(filterOptions.saved_metric_name),
operator: OPERATORS['>'],
operator: OPERATOR_MAPPING[Operators.GREATER_THAN].operation,
operatorId: Operators.GREATER_THAN,
comparator: 0,
clause: CLAUSES.HAVING,
});
Expand All @@ -207,7 +208,8 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
props.datasource.type === 'druid'
? filterOptions.label
: new AdhocMetric(option).translateToSql(),
operator: OPERATORS['>'],
operator: OPERATOR_MAPPING[Operators.GREATER_THAN].operation,
operatorId: Operators.GREATER_THAN,
comparator: 0,
clause: CLAUSES.HAVING,
});
Expand All @@ -217,7 +219,8 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
return new AdhocFilter({
expressionType: EXPRESSION_TYPES.SIMPLE,
subject: filterOptions.column_name,
operator: OPERATORS['=='],
operator: OPERATOR_MAPPING[Operators.EQUALS].operation,
operatorId: Operators.EQUALS,
comparator: '',
clause: CLAUSES.WHERE,
isNew: true,
Expand Down
Expand Up @@ -20,6 +20,7 @@ import AdhocFilter, {
EXPRESSION_TYPES,
CLAUSES,
} from 'src/explore/components/controls/FilterControl/AdhocFilter';
import { Operators } from 'src/explore/constants';

describe('AdhocFilter', () => {
it('sets filterOptionName in constructor', () => {
Expand Down Expand Up @@ -188,6 +189,22 @@ describe('AdhocFilter', () => {
});
// eslint-disable-next-line no-unused-expressions
expect(adhocFilter8.isValid()).toBe(false);

const adhocFilter9 = new AdhocFilter({
expressionType: EXPRESSION_TYPES.SIMPLE,
subject: 'value',
operator: 'IS NULL',
clause: CLAUSES.WHERE,
});
expect(adhocFilter9.isValid()).toBe(true);
const adhocFilter10 = new AdhocFilter({
expressionType: EXPRESSION_TYPES.SIMPLE,
subject: 'value',
operator: 'IS NOT NULL',
clause: CLAUSES.WHERE,
});
// eslint-disable-next-line no-unused-expressions
expect(adhocFilter10.isValid()).toBe(true);
});

it('can translate from simple expressions to sql expressions', () => {
Expand All @@ -209,4 +226,26 @@ describe('AdhocFilter', () => {
});
expect(adhocFilter2.translateToSql()).toBe('SUM(value) <> 5');
});
it('sets comparator to null when operator is IS_NULL', () => {
const adhocFilter2 = new AdhocFilter({
expressionType: EXPRESSION_TYPES.SIMPLE,
subject: 'SUM(value)',
operator: 'IS NULL',
operatorId: Operators.IS_NULL,
comparator: '5',
clause: CLAUSES.HAVING,
});
expect(adhocFilter2.comparator).toBe(null);
});
it('sets comparator to null when operator is IS_NOT_NULL', () => {
const adhocFilter2 = new AdhocFilter({
expressionType: EXPRESSION_TYPES.SIMPLE,
subject: 'SUM(value)',
operator: 'IS NOT NULL',
operatorId: Operators.IS_NOT_NULL,
comparator: '5',
clause: CLAUSES.HAVING,
});
expect(adhocFilter2.comparator).toBe(null);
});
});
Expand Up @@ -16,7 +16,11 @@
* specific language governing permissions and limitations
* under the License.
*/
import { CUSTOM_OPERATORS, OPERATORS } from 'src/explore/constants';
import {
CUSTOM_OPERATORS,
Operators,
OPERATOR_MAPPING,
} from 'src/explore/constants';
import { getSimpleSQLExpression } from 'src/explore/exploreUtils';

export const EXPRESSION_TYPES = {
Expand Down Expand Up @@ -49,11 +53,16 @@ const OPERATORS_TO_SQL = {
`= '{{ presto.latest_partition('${datasource.schema}.${datasource.datasource_name}') }}'`,
};

const CUSTOM_OPERATIONS = [...CUSTOM_OPERATORS].map(
op => OPERATOR_MAPPING[op].operation,
);

function translateToSql(adhocMetric, { useSimple } = {}) {
if (adhocMetric.expressionType === EXPRESSION_TYPES.SIMPLE || useSimple) {
const { subject, comparator } = adhocMetric;
const operator =
adhocMetric.operator && CUSTOM_OPERATORS.has(adhocMetric.operator)
adhocMetric.operator &&
CUSTOM_OPERATIONS.indexOf(adhocMetric.operator) >= 0
? OPERATORS_TO_SQL[adhocMetric.operator](adhocMetric)
: OPERATORS_TO_SQL[adhocMetric.operator];
return getSimpleSQLExpression(subject, operator, comparator);
Expand All @@ -70,7 +79,22 @@ export default class AdhocFilter {
if (this.expressionType === EXPRESSION_TYPES.SIMPLE) {
this.subject = adhocFilter.subject;
this.operator = adhocFilter.operator?.toUpperCase();
this.operatorId = adhocFilter.operatorId;
this.comparator = adhocFilter.comparator;
if (
[Operators.IS_TRUE, Operators.IS_FALSE].indexOf(
adhocFilter.operatorId,
) >= 0
) {
this.comparator = adhocFilter.operatorId === Operators.IS_TRUE;
}
if (
[Operators.IS_NULL, Operators.IS_NOT_NULL].indexOf(
adhocFilter.operatorId,
) >= 0
) {
this.comparator = null;
}
this.clause = adhocFilter.clause || CLAUSES.WHERE;
this.sqlExpression = null;
} else if (this.expressionType === EXPRESSION_TYPES.SQL) {
Expand All @@ -79,9 +103,13 @@ export default class AdhocFilter {
? adhocFilter.sqlExpression
: translateToSql(adhocFilter, { useSimple: true });
this.clause = adhocFilter.clause;
if (adhocFilter.operator && CUSTOM_OPERATORS.has(adhocFilter.operator)) {
if (
adhocFilter.operator &&
CUSTOM_OPERATIONS.indexOf(adhocFilter.operator) >= 0
) {
this.subject = adhocFilter.subject;
this.operator = adhocFilter.operator;
this.operatorId = adhocFilter.operatorId;
} else {
this.subject = null;
this.operator = null;
Expand Down Expand Up @@ -112,24 +140,26 @@ export default class AdhocFilter {
adhocFilter.expressionType === this.expressionType &&
adhocFilter.sqlExpression === this.sqlExpression &&
adhocFilter.operator === this.operator &&
adhocFilter.operatorId === this.operatorId &&
adhocFilter.comparator === this.comparator &&
adhocFilter.subject === this.subject
);
}

isValid() {
const nullCheckOperators = [Operators.IS_NOT_NULL, Operators.IS_NULL].map(
op => OPERATOR_MAPPING[op].operation,
);
const truthCheckOperators = [Operators.IS_TRUE, Operators.IS_FALSE].map(
op => OPERATOR_MAPPING[op].operation,
);
if (this.expressionType === EXPRESSION_TYPES.SIMPLE) {
if (
[
OPERATORS['IS TRUE'],
OPERATORS['IS FALSE'],
OPERATORS['IS NULL'],
OPERATORS['IS NOT NULL'],
].indexOf(this.operator) >= 0
) {
if (nullCheckOperators.indexOf(this.operator) >= 0) {
return !!(this.operator && this.subject);
}

if (truthCheckOperators.indexOf(this.operator) >= 0) {
return !!(this.subject && this.comparator !== null);
}
if (this.operator && this.subject && this.clause) {
if (Array.isArray(this.comparator)) {
if (this.comparator.length > 0) {
Expand Down
Expand Up @@ -27,14 +27,14 @@ import AdhocFilter, {
CLAUSES,
} from 'src/explore/components/controls/FilterControl/AdhocFilter';
import { LabelsContainer } from 'src/explore/components/controls/OptionControls';
import { AGGREGATES, OPERATORS } from 'src/explore/constants';
import { AGGREGATES, Operators, OPERATOR_MAPPING } from 'src/explore/constants';
import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric';
import AdhocFilterControl from '.';

const simpleAdhocFilter = new AdhocFilter({
expressionType: EXPRESSION_TYPES.SIMPLE,
subject: 'value',
operator: '>',
operator: OPERATOR_MAPPING[Operators.GREATER_THAN].operation,
comparator: '10',
clause: CLAUSES.WHERE,
});
Expand Down Expand Up @@ -92,7 +92,7 @@ describe('AdhocFilterControl', () => {
new AdhocFilter({
expressionType: EXPRESSION_TYPES.SQL,
subject: savedMetric.expression,
operator: OPERATORS['>'],
operator: OPERATOR_MAPPING[Operators.GREATER_THAN].operation,
comparator: 0,
clause: CLAUSES.HAVING,
}),
Expand All @@ -111,7 +111,7 @@ describe('AdhocFilterControl', () => {
new AdhocFilter({
expressionType: EXPRESSION_TYPES.SQL,
subject: sumValueAdhocMetric.label,
operator: OPERATORS['>'],
operator: OPERATOR_MAPPING[Operators.GREATER_THAN].operation,
comparator: 0,
clause: CLAUSES.HAVING,
}),
Expand All @@ -129,16 +129,13 @@ describe('AdhocFilterControl', () => {

const newAdhocFilter = onChange.lastCall.args[0][1];
expect(newAdhocFilter instanceof AdhocFilter).toBe(true);
expect(
newAdhocFilter.equals(
new AdhocFilter({
expressionType: EXPRESSION_TYPES.SIMPLE,
subject: columns[0].column_name,
operator: OPERATORS['=='],
comparator: '',
clause: CLAUSES.WHERE,
}),
),
).toBe(true);
const newA = new AdhocFilter({
expressionType: EXPRESSION_TYPES.SIMPLE,
subject: columns[0].column_name,
operator: OPERATOR_MAPPING[Operators.EQUALS].operation,
comparator: '',
clause: CLAUSES.WHERE,
});
expect(newAdhocFilter.equals(newA)).toBe(true);
});
});
Expand Up @@ -30,7 +30,7 @@ import ControlHeader from 'src/explore/components/ControlHeader';
import adhocMetricType from 'src/explore/components/controls/MetricControl/adhocMetricType';
import savedMetricType from 'src/explore/components/controls/MetricControl/savedMetricType';
import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric';
import { OPERATORS } from 'src/explore/constants';
import { Operators, OPERATOR_MAPPING } from 'src/explore/constants';
import FilterDefinitionOption from 'src/explore/components/controls/MetricControl/FilterDefinitionOption';
import {
AddControlLabel,
Expand Down Expand Up @@ -242,7 +242,7 @@ class AdhocFilterControl extends React.Component {
this.props.datasource.type === 'druid'
? option.saved_metric_name
: this.getMetricExpression(option.saved_metric_name),
operator: OPERATORS['>'],
operator: OPERATOR_MAPPING[Operators.GREATER_THAN].operation,
comparator: 0,
clause: CLAUSES.HAVING,
});
Expand All @@ -258,7 +258,7 @@ class AdhocFilterControl extends React.Component {
this.props.datasource.type === 'druid'
? option.label
: new AdhocMetric(option).translateToSql(),
operator: OPERATORS['>'],
operator: OPERATOR_MAPPING[Operators.GREATER_THAN].operation,
comparator: 0,
clause: CLAUSES.HAVING,
});
Expand All @@ -268,7 +268,7 @@ class AdhocFilterControl extends React.Component {
return new AdhocFilter({
expressionType: EXPRESSION_TYPES.SIMPLE,
subject: option.column_name,
operator: OPERATORS['=='],
operator: OPERATOR_MAPPING[Operators.EQUALS].operation,
comparator: '',
clause: CLAUSES.WHERE,
isNew: true,
Expand Down