fix(explore): require a value for simple adhoc filters before allowing save

The Save button in the adhoc filter popover stayed enabled when a
comparator-taking operator had no value, because `AdhocFilter.isValid()`
only rejected a `null` comparator. An unset comparator is `undefined`, not
`null`: selecting a subject resets it (and falls back to the `IN` operator),
and the value Select's clear affordance emits `undefined` as well.

This was most visible on boolean columns, whose operator list is restricted
to unary operators, so a freshly picked boolean column lands on `IN` with no
value and the popover looks complete. Saving sent a filter with no `val` to
the query API, which tripped a bare `assert isinstance(eq, (tuple, list))`
in the query builder and surfaced as a generic error instead of inline
client-side validation.

Extend the existing check to treat `undefined` like `null`, matching the
empty-array guard already applied to `IN`/`NOT IN` comparators. Unary
operators are unaffected: they short-circuit earlier via
DISABLE_INPUT_OPERATORS.

.github/labeler.yml

@@ -29,7 +29,7 @@
 "dependencies:python":
 - changed-files:
   - any-glob-to-any-file:
-    - 'superset/requirements/**'
+    - 'requirements/**'
     - 'superset/translations/requirements.txt'
     - 'RELEASING/requirements.txt'
 

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

@@ -207,6 +207,38 @@ describe('AdhocFilter', () => {
     expect(adhocFilter10.isValid()).toBe(true);
   });
 
+  test('is invalid when a comparator-taking operator has no comparator', () => {
+    // A comparator that was never set, or that was cleared through the value
+    // Select's clear affordance, is `undefined` rather than `null` or `[]`.
+    const adhocFilter1 = new AdhocFilter({
+      expressionType: ExpressionTypes.Simple,
+      subject: 'is_intro',
+      operator: 'IN',
+      comparator: undefined,
+      clause: Clauses.Where,
+    });
+    expect(adhocFilter1.isValid()).toBe(false);
+
+    const adhocFilter2 = new AdhocFilter({
+      expressionType: ExpressionTypes.Simple,
+      subject: 'is_intro',
+      operator: '==',
+      comparator: undefined,
+      clause: Clauses.Where,
+    });
+    expect(adhocFilter2.isValid()).toBe(false);
+
+    // `false` is a legitimate boolean comparator, not a missing value
+    const adhocFilter3 = new AdhocFilter({
+      expressionType: ExpressionTypes.Simple,
+      subject: 'is_intro',
+      operator: '==',
+      comparator: false,
+      clause: Clauses.Where,
+    });
+    expect(adhocFilter3.isValid()).toBe(true);
+  });
+
   test('can translate from simple expressions to sql expressions', () => {
     const adhocFilter1 = new AdhocFilter({
       expressionType: ExpressionTypes.Simple,

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

@@ -163,8 +163,10 @@ export default class AdhocFilter {
           // 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;
+        // A value has been selected or typed. An unset comparator is
+        // `undefined` rather than `null`: picking a new subject resets it, and
+        // the value Select's clear affordance emits `undefined` too.
+        return this.comparator != null;
       }
     }
 

superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/AdhocFilterEditPopover.test.tsx

@@ -181,6 +181,29 @@ describe('AdhocFilterEditPopover', () => {
     expect(saveButton).toBeDisabled();
   });
 
+  test('disables save button when a boolean column has no value selected', async () => {
+    const booleanColumn = { type: 'BOOL', column_name: 'is_intro' };
+    renderPopover({
+      adhocFilter: new AdhocFilter({
+        expressionType: ExpressionTypes.Simple,
+        clause: Clauses.Where,
+      }),
+      options: [booleanColumn],
+      datasource: { columns: [booleanColumn], filter_select: false },
+    });
+
+    // Picking the subject resets the comparator to `undefined`; the value
+    // control is then left untouched, mirroring the reported repro.
+    await userEvent.click(screen.getByTestId('select-element'));
+    await userEvent.click(
+      await screen.findByRole('option', { name: /is_intro/ }),
+    );
+
+    expect(
+      screen.getByTestId('adhoc-filter-edit-popover-save-button'),
+    ).toBeDisabled();
+  });
+
   test('initiates resize when resize handle is dragged', async () => {
     const onResize = jest.fn();
     renderPopover({ onResize });

superset/db_engine_specs/README.md

@@ -83,7 +83,7 @@ The tables below (generated via `python superset/db_engine_specs/lib.py`) summar
 | Databricks (legacy) | 70 | Supported | Partial | Supported | Partial | Partial | Not supported |
 | StarRocks | 69 | Supported | Partial | Supported | Partial | Partial | Partial |
 | SingleStore | 68 | Supported | Partial | Supported | Not supported | Partial | Not supported |
-| ClickHouse Connect (Superset) | 61 | Supported | Partial | Partial | Partial | Partial | Not supported |
+| ClickHouse Connect (Superset) | 62 | Supported | Partial | Supported | Partial | Partial | Not supported |
 | Google Sheets | 61 | Supported | Partial | Supported | Supported | Partial | Partial |
 | Aurora MySQL (Data API) | 59 | Supported | Partial | Supported | Partial | Partial | Not supported |
 | MariaDB | 59 | Supported | Partial | Supported | Partial | Partial | Not supported |
@@ -91,7 +91,7 @@ The tables below (generated via `python superset/db_engine_specs/lib.py`) summar
 | OceanBase | 59 | Supported | Partial | Supported | Partial | Partial | Not supported |
 | MotherDuck | 58 | Supported | Partial | Supported | Not supported | Partial | Not supported |
 | KustoSQL | 54 | Supported | Partial | Supported | Partial | Partial | Not supported |
-| ClickHouse | 51 | Supported | Partial | Partial | Partial | Partial | Not supported |
+| ClickHouse | 52 | Supported | Partial | Supported | Partial | Partial | Not supported |
 | Databend | 51 | Supported | Partial | Supported | Partial | Partial | Not supported |
 | Apache Drill | 50 | Supported | Partial | Supported | Partial | Partial | Partial |
 | Apache Druid | 47 | Partial | Partial | Supported | Partial | Partial | Not supported |
@@ -293,8 +293,8 @@ The tables below (generated via `python superset/db_engine_specs/lib.py`) summar
 | Aurora MySQL (Data API) | True | True | True | True | True | True | True | True |
 | Aurora PostgreSQL (Data API) | True | True | True | True | True | True | True | True |
 | Azure Synapse | True | True | True | True | True | True | True | True |
-| ClickHouse | False | True | True | True | True | True | True | True |
-| ClickHouse Connect (Superset) | False | True | True | True | True | True | True | True |
+| ClickHouse | True | True | True | True | True | True | True | True |
+| ClickHouse Connect (Superset) | True | True | True | True | True | True | True | True |
 | CockroachDB | True | True | True | True | True | True | True | True |
 | Couchbase | True | True | True | True | False | True | True | True |
 | CrateDB | True | True | True | True | True | True | True | True |

superset/db_engine_specs/clickhouse.py

@@ -112,6 +112,7 @@ class ClickHouseBaseEngineSpec(BaseEngineSpec):
 
     _time_grain_expressions = {
         None: "{col}",
+        "PT1S": "toStartOfSecond(toDateTime64({col}, 3))",
         "PT1M": "toStartOfMinute(toDateTime({col}))",
         "PT5M": "toDateTime(intDiv(toUInt32(toDateTime({col})), 300)*300)",
         "PT10M": "toDateTime(intDiv(toUInt32(toDateTime({col})), 600)*600)",

tests/unit_tests/db_engine_specs/test_clickhouse.py

@@ -62,6 +62,20 @@ def test_convert_dttm(
     assert_convert_dttm(spec, target_type, expected_result, dttm)
 
 
+@pytest.mark.parametrize(
+    "time_grain,expected",
+    [
+        (None, "{col}"),
+        ("PT1S", "toStartOfSecond(toDateTime64({col}, 3))"),
+        ("PT1M", "toStartOfMinute(toDateTime({col}))"),
+    ],
+)
+def test_time_grain_expressions(time_grain: Optional[str], expected: str) -> None:
+    from superset.db_engine_specs.clickhouse import ClickHouseBaseEngineSpec
+
+    assert ClickHouseBaseEngineSpec._time_grain_expressions[time_grain] == expected
+
+
 def test_convert_dttm_normalizes_aware_datetime_to_utc() -> None:
     from superset.db_engine_specs.clickhouse import (
         ClickHouseEngineSpec as spec,  # noqa: N813