fix: boolean filters in Explore (#32701)

This commit is contained in:
Beto Dealmeida
2025-03-17 14:38:00 -04:00
committed by GitHub
parent 06deaebe19
commit 41bf215367
7 changed files with 81 additions and 69 deletions

View File

@@ -225,8 +225,8 @@ describe('AdhocFilter', () => {
});
expect(adhocFilter2.translateToSql()).toBe('SUM(value) <> 5');
});
it('sets comparator to null when operator is IS_NULL', () => {
const adhocFilter2 = new AdhocFilter({
it('sets comparator to undefined when operator is IS_NULL', () => {
const adhocFilter = new AdhocFilter({
expressionType: ExpressionTypes.Simple,
subject: 'SUM(value)',
operator: 'IS NULL',
@@ -234,10 +234,10 @@ describe('AdhocFilter', () => {
comparator: '5',
clause: Clauses.Having,
});
expect(adhocFilter2.comparator).toBe(null);
expect(adhocFilter.comparator).toBe(undefined);
});
it('sets comparator to null when operator is IS_NOT_NULL', () => {
const adhocFilter2 = new AdhocFilter({
it('sets comparator to undefined when operator is IS_NOT_NULL', () => {
const adhocFilter = new AdhocFilter({
expressionType: ExpressionTypes.Simple,
subject: 'SUM(value)',
operator: 'IS NOT NULL',
@@ -245,38 +245,60 @@ describe('AdhocFilter', () => {
comparator: '5',
clause: Clauses.Having,
});
expect(adhocFilter2.comparator).toBe(null);
expect(adhocFilter.comparator).toBe(undefined);
});
it('sets comparator to undefined when operator is IS_TRUE', () => {
const adhocFilter = new AdhocFilter({
expressionType: ExpressionTypes.Simple,
subject: 'col',
operator: 'IS TRUE',
operatorId: Operators.IsTrue,
comparator: '5',
clause: Clauses.Having,
});
expect(adhocFilter.comparator).toBe(undefined);
});
it('sets comparator to undefined when operator is IS_FALSE', () => {
const adhocFilter = new AdhocFilter({
expressionType: ExpressionTypes.Simple,
subject: 'col',
operator: 'IS FALSE',
operatorId: Operators.IsFalse,
comparator: '5',
clause: Clauses.Having,
});
expect(adhocFilter.comparator).toBe(undefined);
});
it('sets the label properly if subject is a string', () => {
const adhocFilter2 = new AdhocFilter({
const adhocFilter = new AdhocFilter({
expressionType: ExpressionTypes.Simple,
subject: 'order_date',
});
expect(adhocFilter2.getDefaultLabel()).toBe('order_date');
expect(adhocFilter.getDefaultLabel()).toBe('order_date');
});
it('sets the label properly if subject is an object with the column_date property', () => {
const adhocFilter2 = new AdhocFilter({
const adhocFilter = new AdhocFilter({
expressionType: ExpressionTypes.Simple,
subject: {
column_name: 'year',
},
});
expect(adhocFilter2.getDefaultLabel()).toBe('year');
expect(adhocFilter.getDefaultLabel()).toBe('year');
});
it('sets the label to empty is there is no column_name in the object', () => {
const adhocFilter2 = new AdhocFilter({
const adhocFilter = new AdhocFilter({
expressionType: ExpressionTypes.Simple,
subject: {
unknown: 'year',
},
});
expect(adhocFilter2.getDefaultLabel()).toBe('');
expect(adhocFilter.getDefaultLabel()).toBe('');
});
it('sets the label to empty is there is no subject', () => {
const adhocFilter2 = new AdhocFilter({
const adhocFilter = new AdhocFilter({
expressionType: ExpressionTypes.Simple,
subject: undefined,
});
expect(adhocFilter2.getDefaultLabel()).toBe('');
expect(adhocFilter.getDefaultLabel()).toBe('');
});
});

View File

@@ -18,7 +18,7 @@
*/
import {
CUSTOM_OPERATORS,
Operators,
DISABLE_INPUT_OPERATORS,
OPERATOR_ENUM_TO_OPERATOR_TYPE,
} from 'src/explore/constants';
import { translateToSql } from '../utils/translateToSQL';
@@ -36,18 +36,8 @@ export default class AdhocFilter {
this.operator = adhocFilter.operator?.toUpperCase();
this.operatorId = adhocFilter.operatorId;
this.comparator = adhocFilter.comparator;
if (
[Operators.IsTrue, Operators.IsFalse].indexOf(adhocFilter.operatorId) >=
0
) {
this.comparator = adhocFilter.operatorId === Operators.IsTrue;
}
if (
[Operators.IsNull, Operators.IsNotNull].indexOf(
adhocFilter.operatorId,
) >= 0
) {
this.comparator = null;
if (DISABLE_INPUT_OPERATORS.indexOf(adhocFilter.operatorId) >= 0) {
this.comparator = undefined;
}
this.clause = adhocFilter.clause || Clauses.Where;
this.sqlExpression = null;
@@ -103,34 +93,30 @@ export default class AdhocFilter {
}
isValid() {
const nullCheckOperators = [Operators.IsNotNull, Operators.IsNull].map(
op => OPERATOR_ENUM_TO_OPERATOR_TYPE[op].operation,
);
const truthCheckOperators = [Operators.IsTrue, Operators.IsFalse].map(
op => OPERATOR_ENUM_TO_OPERATOR_TYPE[op].operation,
);
if (this.expressionType === ExpressionTypes.Simple) {
if (nullCheckOperators.indexOf(this.operator) >= 0) {
return !!(this.operator && this.subject);
}
if (truthCheckOperators.indexOf(this.operator) >= 0) {
return !!(this.subject && this.comparator !== null);
// operators where the comparator is not used
if (
DISABLE_INPUT_OPERATORS.map(
op => OPERATOR_ENUM_TO_OPERATOR_TYPE[op].operation,
).indexOf(this.operator) >= 0
) {
return !!this.subject;
}
if (this.operator && this.subject && this.clause) {
if (Array.isArray(this.comparator)) {
if (this.comparator.length > 0) {
// A non-empty array of values ('IN' or 'NOT IN' clauses)
return true;
}
} else if (this.comparator !== null) {
// A value has been selected or typed
return true;
// A non-empty array of values ('IN' or 'NOT IN' clauses)
return this.comparator.length > 0;
}
// A value has been selected or typed
return this.comparator !== null;
}
} else if (this.expressionType === ExpressionTypes.Sql) {
return !!(this.sqlExpression && this.clause);
}
return false;
return (
this.expressionType === ExpressionTypes.Sql &&
!!(this.sqlExpression && this.clause)
);
}
getDefaultLabel() {

View File

@@ -330,25 +330,25 @@ describe('AdhocFilterEditPopoverSimpleTabContent', () => {
expect(isOperatorRelevant(operator, 'value')).toBe(true);
});
});
it('sets comparator to true when operator is IS_TRUE', () => {
it('sets comparator to undefined when operator is IS_TRUE', () => {
const props = setup();
const { onOperatorChange } = useSimpleTabFilterProps(props);
onOperatorChange(Operators.IsTrue);
expect(props.onChange.calledOnce).toBe(true);
expect(props.onChange.lastCall.args[0].operatorId).toBe(Operators.IsTrue);
expect(props.onChange.lastCall.args[0].operator).toBe('==');
expect(props.onChange.lastCall.args[0].comparator).toBe(true);
expect(props.onChange.lastCall.args[0].operator).toBe('IS TRUE');
expect(props.onChange.lastCall.args[0].comparator).toBe(undefined);
});
it('sets comparator to false when operator is IS_FALSE', () => {
it('sets comparator to undefined when operator is IS_FALSE', () => {
const props = setup();
const { onOperatorChange } = useSimpleTabFilterProps(props);
onOperatorChange(Operators.IsFalse);
expect(props.onChange.calledOnce).toBe(true);
expect(props.onChange.lastCall.args[0].operatorId).toBe(Operators.IsFalse);
expect(props.onChange.lastCall.args[0].operator).toBe('==');
expect(props.onChange.lastCall.args[0].comparator).toBe(false);
expect(props.onChange.lastCall.args[0].operator).toBe('IS FALSE');
expect(props.onChange.lastCall.args[0].comparator).toBe(undefined);
});
it('sets comparator to null when operator is IS_NULL or IS_NOT_NULL', () => {
it('sets comparator to undefined when operator is IS_NULL or IS_NOT_NULL', () => {
const props = setup();
const { onOperatorChange } = useSimpleTabFilterProps(props);
[Operators.IsNull, Operators.IsNotNull].forEach(op => {
@@ -358,7 +358,7 @@ describe('AdhocFilterEditPopoverSimpleTabContent', () => {
expect(props.onChange.lastCall.args[0].operator).toBe(
OPERATOR_ENUM_TO_OPERATOR_TYPE[op].operation,
);
expect(props.onChange.lastCall.args[0].comparator).toBe(null);
expect(props.onChange.lastCall.args[0].comparator).toBe(undefined);
});
});
});

View File

@@ -209,9 +209,6 @@ export const useSimpleTabFilterProps = (props: Props) => {
? currentComparator[0]
: currentComparator;
}
if (operatorId === Operators.IsTrue || operatorId === Operators.IsFalse) {
newComparator = Operators.IsTrue === operatorId;
}
if (operatorId && CUSTOM_OPERATORS.has(operatorId)) {
props.onChange(
props.adhocFilter.duplicateWith({

View File

@@ -83,8 +83,8 @@ export const OPERATOR_ENUM_TO_OPERATOR_TYPE: {
display: t('use latest_partition template'),
operation: 'LATEST PARTITION',
},
[Operators.IsTrue]: { display: t('Is true'), operation: '==' },
[Operators.IsFalse]: { display: t('Is false'), operation: '==' },
[Operators.IsTrue]: { display: t('Is true'), operation: 'IS TRUE' },
[Operators.IsFalse]: { display: t('Is false'), operation: 'IS FALSE' },
[Operators.TemporalRange]: {
display: t('TEMPORAL_RANGE'),
operation: 'TEMPORAL_RANGE',

View File

@@ -32,6 +32,7 @@ import { safeStringify } from 'src/utils/safeStringify';
import { optionLabel } from 'src/utils/common';
import { URL_PARAMS } from 'src/constants';
import {
DISABLE_INPUT_OPERATORS,
MULTI_OPERATORS,
OPERATOR_ENUM_TO_OPERATOR_TYPE,
UNSAVED_CHART_ID,
@@ -300,6 +301,10 @@ export const getSimpleSQLExpression = (subject, operator, comparator) => {
[...MULTI_OPERATORS]
.map(op => OPERATOR_ENUM_TO_OPERATOR_TYPE[op].operation)
.indexOf(operator) >= 0;
const showComparator =
DISABLE_INPUT_OPERATORS.map(
op => OPERATOR_ENUM_TO_OPERATOR_TYPE[op].operation,
).indexOf(operator) === -1;
// If returned value is an object after changing dataset
let expression =
typeof subject === 'object'
@@ -314,13 +319,13 @@ export const getSimpleSQLExpression = (subject, operator, comparator) => {
firstValue !== undefined && Number.isNaN(Number(firstValue));
const quote = isString ? "'" : '';
const [prefix, suffix] = isMulti ? ['(', ')'] : ['', ''];
const formattedComparators = comparatorArray
.map(val => optionLabel(val))
.map(
val =>
`${quote}${isString ? String(val).replace(/'/g, "''") : val}${quote}`,
);
if (comparatorArray.length > 0) {
if (comparatorArray.length > 0 && showComparator) {
const formattedComparators = comparatorArray
.map(val => optionLabel(val))
.map(
val =>
`${quote}${isString ? String(val).replace(/'/g, "''") : val}${quote}`,
);
expression += ` ${prefix}${formattedComparators.join(', ')}${suffix}`;
}
}

View File

@@ -722,6 +722,8 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
}
fetch_values_predicate = None
normalize_columns = False
@property
def type(self) -> str:
raise NotImplementedError()
@@ -1976,7 +1978,7 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
self.make_orderby_compatible(select_exprs, orderby_exprs)
for col, (orig_col, ascending) in zip(orderby_exprs, orderby, strict=False): # noqa: B007
for col, (_orig_col, ascending) in zip(orderby_exprs, orderby, strict=False): # noqa: B007
if not db_engine_spec.allows_alias_in_orderby and isinstance(col, Label):
# if engine does not allow using SELECT alias in ORDER BY
# revert to the underlying column