This is an automated email from the ASF dual-hosted git repository. beto pushed a commit to branch fix-boolean-filter in repository https://gitbox.apache.org/repos/asf/superset.git
commit 7a8d397134c36b158bf0ebc8cb81382614c513d2 Author: Beto Dealmeida <[email protected]> AuthorDate: Mon Mar 17 12:03:08 2025 -0400 WIP --- .../controls/FilterControl/AdhocFilter/index.js | 52 ++++++++-------------- .../index.tsx | 3 -- superset-frontend/src/explore/constants.ts | 4 +- .../src/explore/exploreUtils/index.js | 20 ++++++--- superset/models/helpers.py | 10 ++++- 5 files changed, 42 insertions(+), 47 deletions(-) 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 92cb69eebc..a58e3d15b3 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,17 +36,7 @@ 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 - ) { + if (DISABLE_INPUT_OPERATORS.indexOf(adhocFilter.operatorId) >= 0) { this.comparator = null; } this.clause = adhocFilter.clause || Clauses.Where; @@ -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/index.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx index 904c005c14..bfd2b409a8 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 cb56e31393..6b927b4198 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 374445c979..a03fe97171 100644 --- a/superset-frontend/src/explore/exploreUtils/index.js +++ b/superset-frontend/src/explore/exploreUtils/index.js @@ -32,7 +32,9 @@ import { safeStringify } from 'src/utils/safeStringify'; import { optionLabel } from 'src/utils/common'; import { URL_PARAMS } from 'src/constants'; import { + DISABLE_INPUT_OPERATORS, MULTI_OPERATORS, + Operators, OPERATOR_ENUM_TO_OPERATOR_TYPE, UNSAVED_CHART_ID, } from 'src/explore/constants'; @@ -300,6 +302,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 +320,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 1cad46ca62..f049603234 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() @@ -817,7 +819,9 @@ class ExploreMixin: # pylint: disable=too-many-public-methods def get_sqla_row_level_filters( self, - template_processor: Optional[BaseTemplateProcessor] = None, # pylint: disable=unused-argument + template_processor: Optional[ + BaseTemplateProcessor + ] = None, # pylint: disable=unused-argument ) -> list[TextClause]: # TODO: We should refactor this mixin and remove this method # as it exists in the BaseDatasource and is not applicable @@ -1976,7 +1980,9 @@ 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
