diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/AdhocFilter.test.js b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/AdhocFilter.test.js index ed25e78c12f..7ac2a79e4b9 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/AdhocFilter.test.js +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/AdhocFilter.test.js @@ -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(''); }); }); diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js index 92cb69eebca..7c132365737 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js @@ -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() { diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx index bbae8657630..f7614deaf89 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/AdhocFilterEditPopoverSimpleTabContent.test.tsx @@ -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); }); }); }); diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx index 904c005c14e..bfd2b409a8e 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx @@ -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({ diff --git a/superset-frontend/src/explore/constants.ts b/superset-frontend/src/explore/constants.ts index cb56e31393a..6b927b4198a 100644 --- a/superset-frontend/src/explore/constants.ts +++ b/superset-frontend/src/explore/constants.ts @@ -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', diff --git a/superset-frontend/src/explore/exploreUtils/index.js b/superset-frontend/src/explore/exploreUtils/index.js index 374445c9794..ed4d03c6a24 100644 --- a/superset-frontend/src/explore/exploreUtils/index.js +++ b/superset-frontend/src/explore/exploreUtils/index.js @@ -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}`; } } diff --git a/superset/models/helpers.py b/superset/models/helpers.py index 1cad46ca62a..33c7c17d170 100644 --- a/superset/models/helpers.py +++ b/superset/models/helpers.py @@ -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