fix(explore): Explore page boolean filter is broken for Presto DB (ap…

…ache#14952)

* Front end update - modify OPERATORS, to have SQL operation and display value

* Updated tests

* More tests

* Remove OPERATOR imports

* Fix break tests

* PR comments

* fix issue with comparator loading

* rename a variable

* Linting

superset-frontend/src/components/Select/NativeSelect.tsx

@@ -20,6 +20,11 @@ import React from 'react';
 import { styled } from '@superset-ui/core';
 import Select, { SelectProps } from 'antd/lib/select';
 
+export {
+  OptionType as NativeSelectOptionType,
+  SelectProps as NativeSelectProps,
+} from 'antd/lib/select';
+
 const StyledNativeSelect = styled((props: SelectProps<any>) => (
   <Select getPopupContainer={(trigger: any) => trigger.parentNode} {...props} />
 ))`

superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx

@@ -20,7 +20,10 @@ import React, { useEffect, useMemo, useState } from 'react';
 import { logging, SupersetClient, t, Metric } from '@superset-ui/core';
 import { ColumnMeta } from '@superset-ui/chart-controls';
 import { Tooltip } from 'src/components/Tooltip';
-import { OPERATORS } from 'src/explore/constants';
+import {
+  Operators,
+  OPERATOR_ENUM_TO_OPERATOR_TYPE,
+} from 'src/explore/constants';
 import { OptionSortType } from 'src/explore/types';
 import {
   DndFilterSelectProps,
@@ -191,7 +194,9 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
           props.datasource.type === 'druid'
             ? filterOptions.saved_metric_name
             : getMetricExpression(filterOptions.saved_metric_name),
-        operator: OPERATORS['>'],
+        operator:
+          OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.GREATER_THAN].operation,
+        operatorId: Operators.GREATER_THAN,
         comparator: 0,
         clause: CLAUSES.HAVING,
       });
@@ -207,7 +212,9 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
           props.datasource.type === 'druid'
             ? filterOptions.label
             : new AdhocMetric(option).translateToSql(),
-        operator: OPERATORS['>'],
+        operator:
+          OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.GREATER_THAN].operation,
+        operatorId: Operators.GREATER_THAN,
         comparator: 0,
         clause: CLAUSES.HAVING,
       });
@@ -217,7 +224,8 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => {
       return new AdhocFilter({
         expressionType: EXPRESSION_TYPES.SIMPLE,
         subject: filterOptions.column_name,
-        operator: OPERATORS['=='],
+        operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.EQUALS].operation,
+        operatorId: Operators.EQUALS,
         comparator: '',
         clause: CLAUSES.WHERE,
         isNew: true,

superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/AdhocFilter.test.js

@@ -20,6 +20,7 @@ import AdhocFilter, {
   EXPRESSION_TYPES,
   CLAUSES,
 } from 'src/explore/components/controls/FilterControl/AdhocFilter';
+import { Operators } from 'src/explore/constants';
 
 describe('AdhocFilter', () => {
   it('sets filterOptionName in constructor', () => {
@@ -188,6 +189,22 @@ describe('AdhocFilter', () => {
     });
     // eslint-disable-next-line no-unused-expressions
     expect(adhocFilter8.isValid()).toBe(false);
+
+    const adhocFilter9 = new AdhocFilter({
+      expressionType: EXPRESSION_TYPES.SIMPLE,
+      subject: 'value',
+      operator: 'IS NULL',
+      clause: CLAUSES.WHERE,
+    });
+    expect(adhocFilter9.isValid()).toBe(true);
+    const adhocFilter10 = new AdhocFilter({
+      expressionType: EXPRESSION_TYPES.SIMPLE,
+      subject: 'value',
+      operator: 'IS NOT NULL',
+      clause: CLAUSES.WHERE,
+    });
+    // eslint-disable-next-line no-unused-expressions
+    expect(adhocFilter10.isValid()).toBe(true);
   });
 
   it('can translate from simple expressions to sql expressions', () => {
@@ -209,4 +226,26 @@ describe('AdhocFilter', () => {
     });
     expect(adhocFilter2.translateToSql()).toBe('SUM(value) <> 5');
   });
+  it('sets comparator to null when operator is IS_NULL', () => {
+    const adhocFilter2 = new AdhocFilter({
+      expressionType: EXPRESSION_TYPES.SIMPLE,
+      subject: 'SUM(value)',
+      operator: 'IS NULL',
+      operatorId: Operators.IS_NULL,
+      comparator: '5',
+      clause: CLAUSES.HAVING,
+    });
+    expect(adhocFilter2.comparator).toBe(null);
+  });
+  it('sets comparator to null when operator is IS_NOT_NULL', () => {
+    const adhocFilter2 = new AdhocFilter({
+      expressionType: EXPRESSION_TYPES.SIMPLE,
+      subject: 'SUM(value)',
+      operator: 'IS NOT NULL',
+      operatorId: Operators.IS_NOT_NULL,
+      comparator: '5',
+      clause: CLAUSES.HAVING,
+    });
+    expect(adhocFilter2.comparator).toBe(null);
+  });
 });

superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js

@@ -16,7 +16,11 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { CUSTOM_OPERATORS, OPERATORS } from 'src/explore/constants';
+import {
+  CUSTOM_OPERATORS,
+  Operators,
+  OPERATOR_ENUM_TO_OPERATOR_TYPE,
+} from 'src/explore/constants';
 import { getSimpleSQLExpression } from 'src/explore/exploreUtils';
 
 export const EXPRESSION_TYPES = {
@@ -49,11 +53,16 @@ const OPERATORS_TO_SQL = {
     `= '{{ presto.latest_partition('${datasource.schema}.${datasource.datasource_name}') }}'`,
 };
 
+const CUSTOM_OPERATIONS = [...CUSTOM_OPERATORS].map(
+  op => OPERATOR_ENUM_TO_OPERATOR_TYPE[op].operation,
+);
+
 function translateToSql(adhocMetric, { useSimple } = {}) {
   if (adhocMetric.expressionType === EXPRESSION_TYPES.SIMPLE || useSimple) {
     const { subject, comparator } = adhocMetric;
     const operator =
-      adhocMetric.operator && CUSTOM_OPERATORS.has(adhocMetric.operator)
+      adhocMetric.operator &&
+      CUSTOM_OPERATIONS.indexOf(adhocMetric.operator) >= 0
         ? OPERATORS_TO_SQL[adhocMetric.operator](adhocMetric)
         : OPERATORS_TO_SQL[adhocMetric.operator];
     return getSimpleSQLExpression(subject, operator, comparator);
@@ -70,7 +79,22 @@ export default class AdhocFilter {
     if (this.expressionType === EXPRESSION_TYPES.SIMPLE) {
       this.subject = adhocFilter.subject;
       this.operator = adhocFilter.operator?.toUpperCase();
+      this.operatorId = adhocFilter.operatorId;
       this.comparator = adhocFilter.comparator;
+      if (
+        [Operators.IS_TRUE, Operators.IS_FALSE].indexOf(
+          adhocFilter.operatorId,
+        ) >= 0
+      ) {
+        this.comparator = adhocFilter.operatorId === Operators.IS_TRUE;
+      }
+      if (
+        [Operators.IS_NULL, Operators.IS_NOT_NULL].indexOf(
+          adhocFilter.operatorId,
+        ) >= 0
+      ) {
+        this.comparator = null;
+      }
       this.clause = adhocFilter.clause || CLAUSES.WHERE;
       this.sqlExpression = null;
     } else if (this.expressionType === EXPRESSION_TYPES.SQL) {
@@ -79,9 +103,13 @@ export default class AdhocFilter {
           ? adhocFilter.sqlExpression
           : translateToSql(adhocFilter, { useSimple: true });
       this.clause = adhocFilter.clause;
-      if (adhocFilter.operator && CUSTOM_OPERATORS.has(adhocFilter.operator)) {
+      if (
+        adhocFilter.operator &&
+        CUSTOM_OPERATIONS.indexOf(adhocFilter.operator) >= 0
+      ) {
         this.subject = adhocFilter.subject;
         this.operator = adhocFilter.operator;
+        this.operatorId = adhocFilter.operatorId;
       } else {
         this.subject = null;
         this.operator = null;
@@ -112,24 +140,26 @@ export default class AdhocFilter {
       adhocFilter.expressionType === this.expressionType &&
       adhocFilter.sqlExpression === this.sqlExpression &&
       adhocFilter.operator === this.operator &&
+      adhocFilter.operatorId === this.operatorId &&
       adhocFilter.comparator === this.comparator &&
       adhocFilter.subject === this.subject
     );
   }
 
   isValid() {
+    const nullCheckOperators = [Operators.IS_NOT_NULL, Operators.IS_NULL].map(
+      op => OPERATOR_ENUM_TO_OPERATOR_TYPE[op].operation,
+    );
+    const truthCheckOperators = [Operators.IS_TRUE, Operators.IS_FALSE].map(
+      op => OPERATOR_ENUM_TO_OPERATOR_TYPE[op].operation,
+    );
     if (this.expressionType === EXPRESSION_TYPES.SIMPLE) {
-      if (
-        [
-          OPERATORS['IS TRUE'],
-          OPERATORS['IS FALSE'],
-          OPERATORS['IS NULL'],
-          OPERATORS['IS NOT NULL'],
-        ].indexOf(this.operator) >= 0
-      ) {
+      if (nullCheckOperators.indexOf(this.operator) >= 0) {
         return !!(this.operator && this.subject);
       }
-
+      if (truthCheckOperators.indexOf(this.operator) >= 0) {
+        return !!(this.subject && this.comparator !== null);
+      }
       if (this.operator && this.subject && this.clause) {
         if (Array.isArray(this.comparator)) {
           if (this.comparator.length > 0) {

superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/AdhocFilterControl.test.jsx

@@ -27,14 +27,18 @@ import AdhocFilter, {
   CLAUSES,
 } from 'src/explore/components/controls/FilterControl/AdhocFilter';
 import { LabelsContainer } from 'src/explore/components/controls/OptionControls';
-import { AGGREGATES, OPERATORS } from 'src/explore/constants';
+import {
+  AGGREGATES,
+  Operators,
+  OPERATOR_ENUM_TO_OPERATOR_TYPE,
+} from 'src/explore/constants';
 import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric';
 import AdhocFilterControl from '.';
 
 const simpleAdhocFilter = new AdhocFilter({
   expressionType: EXPRESSION_TYPES.SIMPLE,
   subject: 'value',
-  operator: '>',
+  operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.GREATER_THAN].operation,
   comparator: '10',
   clause: CLAUSES.WHERE,
 });
@@ -92,7 +96,8 @@ describe('AdhocFilterControl', () => {
         new AdhocFilter({
           expressionType: EXPRESSION_TYPES.SQL,
           subject: savedMetric.expression,
-          operator: OPERATORS['>'],
+          operator:
+            OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.GREATER_THAN].operation,
           comparator: 0,
           clause: CLAUSES.HAVING,
         }),
@@ -111,7 +116,8 @@ describe('AdhocFilterControl', () => {
         new AdhocFilter({
           expressionType: EXPRESSION_TYPES.SQL,
           subject: sumValueAdhocMetric.label,
-          operator: OPERATORS['>'],
+          operator:
+            OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.GREATER_THAN].operation,
           comparator: 0,
           clause: CLAUSES.HAVING,
         }),
@@ -134,7 +140,7 @@ describe('AdhocFilterControl', () => {
         new AdhocFilter({
           expressionType: EXPRESSION_TYPES.SIMPLE,
           subject: columns[0].column_name,
-          operator: OPERATORS['=='],
+          operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.EQUALS].operation,
           comparator: '',
           clause: CLAUSES.WHERE,
         }),

superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.jsx

@@ -30,7 +30,10 @@ import ControlHeader from 'src/explore/components/ControlHeader';
 import adhocMetricType from 'src/explore/components/controls/MetricControl/adhocMetricType';
 import savedMetricType from 'src/explore/components/controls/MetricControl/savedMetricType';
 import AdhocMetric from 'src/explore/components/controls/MetricControl/AdhocMetric';
-import { OPERATORS } from 'src/explore/constants';
+import {
+  Operators,
+  OPERATOR_ENUM_TO_OPERATOR_TYPE,
+} from 'src/explore/constants';
 import FilterDefinitionOption from 'src/explore/components/controls/MetricControl/FilterDefinitionOption';
 import {
   AddControlLabel,
@@ -242,7 +245,8 @@ class AdhocFilterControl extends React.Component {
           this.props.datasource.type === 'druid'
             ? option.saved_metric_name
             : this.getMetricExpression(option.saved_metric_name),
-        operator: OPERATORS['>'],
+        operator:
+          OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.GREATER_THAN].operation,
         comparator: 0,
         clause: CLAUSES.HAVING,
       });
@@ -258,7 +262,8 @@ class AdhocFilterControl extends React.Component {
           this.props.datasource.type === 'druid'
             ? option.label
             : new AdhocMetric(option).translateToSql(),
-        operator: OPERATORS['>'],
+        operator:
+          OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.GREATER_THAN].operation,
         comparator: 0,
         clause: CLAUSES.HAVING,
       });
@@ -268,7 +273,7 @@ class AdhocFilterControl extends React.Component {
       return new AdhocFilter({
         expressionType: EXPRESSION_TYPES.SIMPLE,
         subject: option.column_name,
-        operator: OPERATORS['=='],
+        operator: OPERATOR_ENUM_TO_OPERATOR_TYPE[Operators.EQUALS].operation,
         comparator: '',
         clause: CLAUSES.WHERE,
         isNew: true,