This is an automated email from the ASF dual-hosted git repository.
beto pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 41bf215367 fix: boolean filters in Explore (#32701)
41bf215367 is described below
commit 41bf2153672964d58080966abee464b7fb847c6b
Author: Beto Dealmeida <[email protected]>
AuthorDate: Mon Mar 17 14:38:00 2025 -0400
fix: boolean filters in Explore (#32701)
---
.../FilterControl/AdhocFilter/AdhocFilter.test.js | 50 ++++++++++++++------
.../controls/FilterControl/AdhocFilter/index.js | 54 ++++++++--------------
...AdhocFilterEditPopoverSimpleTabContent.test.tsx | 16 +++----
.../index.tsx | 3 --
superset-frontend/src/explore/constants.ts | 4 +-
.../src/explore/exploreUtils/index.js | 19 +++++---
superset/models/helpers.py | 4 +-
7 files changed, 81 insertions(+), 69 deletions(-)
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 ed25e78c12..7ac2a79e4b 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 92cb69eebc..7c13236573 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 bbae865763..f7614deaf8 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 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..ed4d03c6a2 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 1cad46ca62..33c7c17d17 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
