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): #10098 boolean filter not working #14567

Expand Up @@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { CUSTOM_OPERATORS } from 'src/explore/constants';
import { CUSTOM_OPERATORS, OPERATORS } from 'src/explore/constants';
import { getSimpleSQLExpression } from 'src/explore/exploreUtils';

export const EXPRESSION_TYPES = {
Expand All @@ -42,6 +42,8 @@ const OPERATORS_TO_SQL = {
REGEX: 'REGEX',
'IS NOT NULL': 'IS NOT NULL',
'IS NULL': 'IS NULL',
'IS TRUE': 'IS TRUE',
'IS FALSE': 'IS FALSE',
'LATEST PARTITION': ({ datasource }) =>
`= '{{ presto.latest_partition('${datasource.schema}.${datasource.datasource_name}') }}'`,
};
Expand Down Expand Up @@ -116,7 +118,14 @@ export default class AdhocFilter {

isValid() {
if (this.expressionType === EXPRESSION_TYPES.SIMPLE) {
if (this.operator === 'IS NOT NULL' || this.operator === 'IS NULL') {
if (
[
OPERATORS['IS TRUE'],
OPERATORS['IS FALSE'],
OPERATORS['IS NULL'],
OPERATORS['IS NOT NULL'],
].indexOf(this.operator) >= 0
) {
return !!(this.operator && this.subject);
}

Expand Down
Expand Up @@ -197,4 +197,82 @@ describe('AdhocFilterEditPopoverSimpleTabContent', () => {
}),
);
});
it('will display boolean operators only when column type is boolean', () => {
const { wrapper } = setup({
datasource: {
type: 'table',
datasource_name: 'table1',
schema: 'schema',
columns: [{ column_name: 'value', type: 'BOOL' }],
},
adhocFilter: simpleAdhocFilter,
});
const booleanOnlyOperators = [
'IS TRUE',
'IS FALSE',
'IS NULL',
'IS NOT NULL',
];
booleanOnlyOperators.forEach(operator => {
expect(wrapper.instance().isOperatorRelevant(operator, 'value')).toBe(
true,
);
});
});
it('will display boolean operators when column type is number', () => {
const { wrapper } = setup({
datasource: {
type: 'table',
datasource_name: 'table1',
schema: 'schema',
columns: [{ column_name: 'value', type: 'INT' }],
},
adhocFilter: simpleAdhocFilter,
});
const booleanOnlyOperators = ['IS TRUE', 'IS FALSE'];
booleanOnlyOperators.forEach(operator => {
expect(wrapper.instance().isOperatorRelevant(operator, 'value')).toBe(
true,
);
});
});
it('will not display boolean operators when column type is string', () => {
const { wrapper } = setup({
datasource: {
type: 'table',
datasource_name: 'table1',
schema: 'schema',
columns: [{ column_name: 'value', type: 'STRING' }],
},
adhocFilter: simpleAdhocFilter,
});
const booleanOnlyOperators = ['IS TRUE', 'IS FALSE'];
booleanOnlyOperators.forEach(operator => {
expect(wrapper.instance().isOperatorRelevant(operator, 'value')).toBe(
false,
);
});
});
it('will display boolean operators when column is an expression', () => {
const { wrapper } = setup({
datasource: {
type: 'table',
datasource_name: 'table1',
schema: 'schema',
columns: [
{
column_name: 'value',
expression: 'case when value is 0 then "NO"',
},
],
},
adhocFilter: simpleAdhocFilter,
});
const booleanOnlyOperators = ['IS TRUE', 'IS FALSE'];
booleanOnlyOperators.forEach(operator => {
expect(wrapper.instance().isOperatorRelevant(operator, 'value')).toBe(
true,
);
});
});
});
Expand Up @@ -236,11 +236,30 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon
}

isOperatorRelevant(operator, subject) {
const column = this.props.datasource.columns?.find(
col => col.column_name === subject,
);
const isColumnBoolean =
!!column && (column.type === 'BOOL' || column.type === 'BOOLEAN');
const isColumnNumber = !!column && column.type === 'INT';
const isColumnFunction = !!column && !!column.expression;

if (operator && CUSTOM_OPERATORS.has(operator)) {
const { partitionColumn } = this.props;
return partitionColumn && subject && subject === partitionColumn;
}

if (
operator === OPERATORS['IS TRUE'] ||
operator === OPERATORS['IS FALSE']
) {
return isColumnBoolean || isColumnNumber || isColumnFunction;
}
if (isColumnBoolean) {
return (
operator === OPERATORS['IS NULL'] ||
operator === OPERATORS['IS NOT NULL']
);
}
return !(
(this.props.datasource.type === 'druid' &&
TABLE_ONLY_OPERATORS.indexOf(operator) >= 0) ||
Expand Down Expand Up @@ -319,7 +338,7 @@ export default class AdhocFilterEditPopoverSimpleTabContent extends React.Compon
OPERATORS_OPTIONS.filter(op => this.isOperatorRelevant(op, subject))
.length,
),
// like AGGREGTES_OPTIONS, operator options are string
// like AGGREGATES_OPTIONS, operator options are string
value: operator,
onChange: this.onOperatorChange,
filterOption: (input, option) =>
Expand Down
5 changes: 5 additions & 0 deletions superset-frontend/src/explore/constants.ts
Expand Up @@ -42,7 +42,10 @@ export const OPERATORS = {
'IS NOT NULL': 'IS NOT NULL',
'IS NULL': 'IS NULL',
'LATEST PARTITION': 'LATEST PARTITION',
'IS TRUE': 'IS TRUE',
'IS FALSE': 'IS FALSE',
};

export const OPERATORS_OPTIONS = Object.values(OPERATORS);

export const TABLE_ONLY_OPERATORS = [OPERATORS.LIKE];
Expand All @@ -65,6 +68,8 @@ export const DISABLE_INPUT_OPERATORS = [
OPERATORS['IS NOT NULL'],
OPERATORS['IS NULL'],
OPERATORS['LATEST PARTITION'],
OPERATORS['IS TRUE'],
OPERATORS['IS FALSE'],
];

export const sqlaAutoGeneratedMetricNameRegex = /^(sum|min|max|avg|count|count_distinct)__.*$/i;
Expand Down
4 changes: 4 additions & 0 deletions superset/connectors/sqla/models.py
Expand Up @@ -1198,6 +1198,10 @@ def get_sqla_query( # pylint: disable=too-many-arguments,too-many-locals,too-ma
where_clause_and.append(col_obj.get_sqla_col().is_(None))
elif op == utils.FilterOperator.IS_NOT_NULL.value:
where_clause_and.append(col_obj.get_sqla_col().isnot(None))
elif op == utils.FilterOperator.IS_TRUE.value:
where_clause_and.append(col_obj.get_sqla_col().is_(True))
elif op == utils.FilterOperator.IS_FALSE.value:
where_clause_and.append(col_obj.get_sqla_col().is_(False))
else:
if eq is None:
raise QueryObjectValidationError(
Expand Down
2 changes: 2 additions & 0 deletions superset/utils/core.py
Expand Up @@ -211,6 +211,8 @@ class FilterOperator(str, Enum):
IN = "IN" # pylint: disable=invalid-name
NOT_IN = "NOT IN"
REGEX = "REGEX"
IS_TRUE = "IS TRUE"
IS_FALSE = "IS FALSE"


class PostProcessingBoxplotWhiskerType(str, Enum):
Expand Down
12 changes: 10 additions & 2 deletions tests/sqla_models_tests.py
Expand Up @@ -169,11 +169,14 @@ def test_where_operators(self):
class FilterTestCase(NamedTuple):
operator: str
value: Union[float, int, List[Any], str]
expected: str
expected: Union[str, List[str]]

filters: Tuple[FilterTestCase, ...] = (
FilterTestCase(FilterOperator.IS_NULL, "", "IS NULL"),
FilterTestCase(FilterOperator.IS_NOT_NULL, "", "IS NOT NULL"),
# Some db backends translate true/false to 1/0
FilterTestCase(FilterOperator.IS_TRUE, "", ["IS 1", "IS true"]),
FilterTestCase(FilterOperator.IS_FALSE, "", ["IS 0", "IS false"]),
FilterTestCase(FilterOperator.GREATER_THAN, 0, "> 0"),
FilterTestCase(FilterOperator.GREATER_THAN_OR_EQUALS, 0, ">= 0"),
FilterTestCase(FilterOperator.LESS_THAN, 0, "< 0"),
Expand All @@ -199,7 +202,12 @@ class FilterTestCase(NamedTuple):
}
sqla_query = table.get_sqla_query(**query_obj)
sql = table.database.compile_sqla_query(sqla_query.sqla_query)
self.assertIn(filter_.expected, sql)
if isinstance(filter_.expected, list):
self.assertTrue(
any([candidate in sql for candidate in filter_.expected])
)
else:
self.assertIn(filter_.expected, sql)

def test_incorrect_jinja_syntax_raises_correct_exception(self):
query_obj = {
Expand Down