feat(cross-filters): add support for temporal filters (#16139)

* feat(cross-filters): add support for temporal filters

* fix test

* make filter optional

* remove mocks

* fix more tests

* remove unnecessary optionality

* fix even more tests

* bump superset-ui

* add isExtra to schema

* address comments

* fix presto test

tests/integration_tests/model_tests.py

@@ -22,7 +22,6 @@ from tests.integration_tests.fixtures.birth_names_dashboard import (
     load_birth_names_dashboard_with_slices,
 )
 
-import pandas
 import pytest
 from sqlalchemy.engine.url import make_url
 
@@ -339,12 +338,19 @@ class TestDatabaseModel(SupersetTestCase):
 class TestSqlaTableModel(SupersetTestCase):
     @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
     def test_get_timestamp_expression(self):
+        col_type = (
+            "VARCHAR"
+            if get_example_database().backend == "presto"
+            else "TemporalWrapperType"
+        )
         tbl = self.get_table(name="birth_names")
         ds_col = tbl.get_column("ds")
         sqla_literal = ds_col.get_timestamp_expression(None)
         self.assertEqual(str(sqla_literal.compile()), "ds")
+        assert type(sqla_literal.type).__name__ == col_type
 
         sqla_literal = ds_col.get_timestamp_expression("P1D")
+        assert type(sqla_literal.type).__name__ == col_type
         compiled = "{}".format(sqla_literal.compile())
         if tbl.database.backend == "mysql":
             self.assertEqual(compiled, "DATE(ds)")
@@ -352,6 +358,7 @@ class TestSqlaTableModel(SupersetTestCase):
         prev_ds_expr = ds_col.expression
         ds_col.expression = "DATE_ADD(ds, 1)"
         sqla_literal = ds_col.get_timestamp_expression("P1D")
+        assert type(sqla_literal.type).__name__ == col_type
         compiled = "{}".format(sqla_literal.compile())
         if tbl.database.backend == "mysql":
             self.assertEqual(compiled, "DATE(DATE_ADD(ds, 1))")

tests/integration_tests/utils/core_tests.py

@@ -17,10 +17,10 @@
 # pylint: disable=no-self-use
 import pytest
 
-from superset.utils.core import to_adhoc
+from superset.utils.core import form_data_to_adhoc, simple_filter_to_adhoc
 
 
-def test_to_adhoc_generates_deterministic_values():
+def test_simple_filter_to_adhoc_generates_deterministic_values():
     input_1 = {
         "op": "IS NOT NULL",
         "col": "LATITUDE",
@@ -30,25 +30,56 @@ def test_to_adhoc_generates_deterministic_values():
     input_2 = {**input_1, "col": "LONGITUDE"}
 
     # The result is the same when given the same input
-    assert to_adhoc(input_1) == to_adhoc(input_1)
-    assert to_adhoc(input_1) == {
+    assert simple_filter_to_adhoc(input_1) == simple_filter_to_adhoc(input_1)
+    assert simple_filter_to_adhoc(input_1) == {
         "clause": "WHERE",
         "expressionType": "SIMPLE",
-        "isExtra": False,
         "comparator": "",
         "operator": "IS NOT NULL",
         "subject": "LATITUDE",
-        "filterOptionName": "d0908f77d950131db7a69fdc820cb739",
+        "filterOptionName": "6ac89d498115da22396f80a765cffc70",
     }
 
     # The result is different when given different input
-    assert to_adhoc(input_1) != to_adhoc(input_2)
-    assert to_adhoc(input_2) == {
+    assert simple_filter_to_adhoc(input_1) != simple_filter_to_adhoc(input_2)
+    assert simple_filter_to_adhoc(input_2) == {
         "clause": "WHERE",
         "expressionType": "SIMPLE",
-        "isExtra": False,
         "comparator": "",
         "operator": "IS NOT NULL",
         "subject": "LONGITUDE",
-        "filterOptionName": "c5f283f727d4dfc6258b351d4a8663bc",
+        "filterOptionName": "9c984bd3714883ca859948354ce26ab9",
     }
+
+
+def test_form_data_to_adhoc_generates_deterministic_values():
+    form_data = {"where": "1 = 1", "having": "count(*) > 1"}
+
+    # The result is the same when given the same input
+    assert form_data_to_adhoc(form_data, "where") == form_data_to_adhoc(
+        form_data, "where"
+    )
+    assert form_data_to_adhoc(form_data, "where") == {
+        "clause": "WHERE",
+        "expressionType": "SQL",
+        "sqlExpression": "1 = 1",
+        "filterOptionName": "99fe79985afbddea4492626dc6a87b74",
+    }
+
+    # The result is different when given different input
+    assert form_data_to_adhoc(form_data, "having") == form_data_to_adhoc(
+        form_data, "having"
+    )
+    assert form_data_to_adhoc(form_data, "having") == {
+        "clause": "HAVING",
+        "expressionType": "SQL",
+        "sqlExpression": "count(*) > 1",
+        "filterOptionName": "1da11f6b709c3190daeabb84f77fc8c2",
+    }
+
+
+def test_form_data_to_adhoc_incorrect_clause_type():
+    form_data = {"where": "1 = 1", "having": "count(*) > 1"}
+
+    with pytest.raises(ValueError):
+        form_data_to_adhoc(form_data, "foobar")

tests/integration_tests/utils_tests.py

@@ -85,19 +85,6 @@ from tests.integration_tests.fixtures.world_bank_dashboard import (
 from .fixtures.certificates import ssl_certificate
 
 
-def mock_to_adhoc(filt, expressionType="SIMPLE", clause="where"):
-    result = {"clause": clause.upper(), "expressionType": expressionType}
-
-    if expressionType == "SIMPLE":
-        result.update(
-            {"comparator": filt["val"], "operator": filt["op"], "subject": filt["col"]}
-        )
-    elif expressionType == "SQL":
-        result.update({"sqlExpression": filt[clause]})
-
-    return result
-
-
 class TestUtils(SupersetTestCase):
     def test_json_int_dttm_ser(self):
         dttm = datetime(2020, 1, 1)
@@ -137,7 +124,6 @@ class TestUtils(SupersetTestCase):
         got_str = zlib_decompress(blob)
         self.assertEqual(json_str, got_str)
 
-    @patch("superset.utils.core.to_adhoc", mock_to_adhoc)
     def test_merge_extra_filters(self):
         # does nothing if no extra filters
         form_data = {"A": 1, "B": 2, "c": "test"}
@@ -168,6 +154,8 @@ class TestUtils(SupersetTestCase):
                     "clause": "WHERE",
                     "comparator": "someval",
                     "expressionType": "SIMPLE",
+                    "filterOptionName": "90cfb3c34852eb3bc741b0cc20053b46",
+                    "isExtra": True,
                     "operator": "in",
                     "subject": "a",
                 },
@@ -175,6 +163,8 @@ class TestUtils(SupersetTestCase):
                     "clause": "WHERE",
                     "comparator": ["c1", "c2"],
                     "expressionType": "SIMPLE",
+                    "filterOptionName": "6c178d069965f1c02640661280415d96",
+                    "isExtra": True,
                     "operator": "==",
                     "subject": "B",
                 },
@@ -212,6 +202,8 @@ class TestUtils(SupersetTestCase):
                     "clause": "WHERE",
                     "comparator": "someval",
                     "expressionType": "SIMPLE",
+                    "filterOptionName": "90cfb3c34852eb3bc741b0cc20053b46",
+                    "isExtra": True,
                     "operator": "in",
                     "subject": "a",
                 },
@@ -219,6 +211,8 @@ class TestUtils(SupersetTestCase):
                     "clause": "WHERE",
                     "comparator": ["c1", "c2"],
                     "expressionType": "SIMPLE",
+                    "filterOptionName": "6c178d069965f1c02640661280415d96",
+                    "isExtra": True,
                     "operator": "==",
                     "subject": "B",
                 },
@@ -244,6 +238,8 @@ class TestUtils(SupersetTestCase):
                     "clause": "WHERE",
                     "comparator": "hello",
                     "expressionType": "SIMPLE",
+                    "filterOptionName": "e3cbdd92a2ae23ca92c6d7fca42e36a6",
+                    "isExtra": True,
                     "operator": "like",
                     "subject": "A",
                 }
@@ -264,7 +260,6 @@ class TestUtils(SupersetTestCase):
         merge_extra_filters(form_data)
         self.assertEqual(form_data, expected)
 
-    @patch("superset.utils.core.to_adhoc", mock_to_adhoc)
     def test_merge_extra_filters_ignores_empty_filters(self):
         form_data = {
             "extra_filters": [
@@ -276,7 +271,6 @@ class TestUtils(SupersetTestCase):
         merge_extra_filters(form_data)
         self.assertEqual(form_data, expected)
 
-    @patch("superset.utils.core.to_adhoc", mock_to_adhoc)
     def test_merge_extra_filters_ignores_nones(self):
         form_data = {
             "adhoc_filters": [
@@ -305,7 +299,6 @@ class TestUtils(SupersetTestCase):
         merge_extra_filters(form_data)
         self.assertEqual(form_data, expected)
 
-    @patch("superset.utils.core.to_adhoc", mock_to_adhoc)
     def test_merge_extra_filters_ignores_equal_filters(self):
         form_data = {
             "extra_filters": [
@@ -366,7 +359,6 @@ class TestUtils(SupersetTestCase):
         merge_extra_filters(form_data)
         self.assertEqual(form_data, expected)
 
-    @patch("superset.utils.core.to_adhoc", mock_to_adhoc)
     def test_merge_extra_filters_merges_different_val_types(self):
         form_data = {
             "extra_filters": [
@@ -410,6 +402,8 @@ class TestUtils(SupersetTestCase):
                     "clause": "WHERE",
                     "comparator": ["g1", "g2"],
                     "expressionType": "SIMPLE",
+                    "filterOptionName": "c11969c994b40a83a4ae7d48ff1ea28e",
+                    "isExtra": True,
                     "operator": "in",
                     "subject": "a",
                 },
@@ -460,6 +454,8 @@ class TestUtils(SupersetTestCase):
                     "clause": "WHERE",
                     "comparator": "someval",
                     "expressionType": "SIMPLE",
+                    "filterOptionName": "90cfb3c34852eb3bc741b0cc20053b46",
+                    "isExtra": True,
                     "operator": "in",
                     "subject": "a",
                 },
@@ -469,7 +465,6 @@ class TestUtils(SupersetTestCase):
         merge_extra_filters(form_data)
         self.assertEqual(form_data, expected)
 
-    @patch("superset.utils.core.to_adhoc", mock_to_adhoc)
     def test_merge_extra_filters_adds_unequal_lists(self):
         form_data = {
             "extra_filters": [
@@ -513,6 +508,8 @@ class TestUtils(SupersetTestCase):
                     "clause": "WHERE",
                     "comparator": ["g1", "g2", "g3"],
                     "expressionType": "SIMPLE",
+                    "filterOptionName": "21cbb68af7b17e62b3b2f75e2190bfd7",
+                    "isExtra": True,
                     "operator": "in",
                     "subject": "a",
                 },
@@ -520,6 +517,8 @@ class TestUtils(SupersetTestCase):
                     "clause": "WHERE",
                     "comparator": ["c1", "c2", "c3"],
                     "expressionType": "SIMPLE",
+                    "filterOptionName": "0a8dcb928f1f4bba97643c6e68d672f1",
+                    "isExtra": True,
                     "operator": "==",
                     "subject": "B",
                 },
@@ -580,18 +579,21 @@ class TestUtils(SupersetTestCase):
         with self.assertRaises(SupersetException):
             validate_json(invalid)
 
-    @patch("superset.utils.core.to_adhoc", mock_to_adhoc)
     def test_convert_legacy_filters_into_adhoc_where(self):
         form_data = {"where": "a = 1"}
         expected = {
             "adhoc_filters": [
-                {"clause": "WHERE", "expressionType": "SQL", "sqlExpression": "a = 1"}
+                {
+                    "clause": "WHERE",
+                    "expressionType": "SQL",
+                    "filterOptionName": "46fb6d7891e23596e42ae38da94a57e0",
+                    "sqlExpression": "a = 1",
+                }
             ]
         }
         convert_legacy_filters_into_adhoc(form_data)
         self.assertEqual(form_data, expected)
 
-    @patch("superset.utils.core.to_adhoc", mock_to_adhoc)
     def test_convert_legacy_filters_into_adhoc_filters(self):
         form_data = {"filters": [{"col": "a", "op": "in", "val": "someval"}]}
         expected = {
@@ -600,6 +602,7 @@ class TestUtils(SupersetTestCase):
                     "clause": "WHERE",
                     "comparator": "someval",
                     "expressionType": "SIMPLE",
+                    "filterOptionName": "135c7ee246666b840a3d7a9c3a30cf38",
                     "operator": "in",
                     "subject": "a",
                 }
@@ -608,7 +611,6 @@ class TestUtils(SupersetTestCase):
         convert_legacy_filters_into_adhoc(form_data)
         self.assertEqual(form_data, expected)
 
-    @patch("superset.utils.core.to_adhoc", mock_to_adhoc)
     def test_convert_legacy_filters_into_adhoc_having(self):
         form_data = {"having": "COUNT(1) = 1"}
         expected = {
@@ -616,6 +618,7 @@ class TestUtils(SupersetTestCase):
                 {
                     "clause": "HAVING",
                     "expressionType": "SQL",
+                    "filterOptionName": "683f1c26466ab912f75a00842e0f2f7b",
                     "sqlExpression": "COUNT(1) = 1",
                 }
             ]
@@ -623,7 +626,6 @@ class TestUtils(SupersetTestCase):
         convert_legacy_filters_into_adhoc(form_data)
         self.assertEqual(form_data, expected)
 
-    @patch("superset.utils.core.to_adhoc", mock_to_adhoc)
     def test_convert_legacy_filters_into_adhoc_having_filters(self):
         form_data = {"having_filters": [{"col": "COUNT(1)", "op": "==", "val": 1}]}
         expected = {
@@ -632,6 +634,7 @@ class TestUtils(SupersetTestCase):
                     "clause": "HAVING",
                     "comparator": 1,
                     "expressionType": "SIMPLE",
+                    "filterOptionName": "967d0fb409f6d9c7a6c03a46cf933c9c",
                     "operator": "==",
                     "subject": "COUNT(1)",
                 }
@@ -640,18 +643,21 @@ class TestUtils(SupersetTestCase):
         convert_legacy_filters_into_adhoc(form_data)
         self.assertEqual(form_data, expected)
 
-    @patch("superset.utils.core.to_adhoc", mock_to_adhoc)
     def test_convert_legacy_filters_into_adhoc_present_and_empty(self):
         form_data = {"adhoc_filters": [], "where": "a = 1"}
         expected = {
             "adhoc_filters": [
-                {"clause": "WHERE", "expressionType": "SQL", "sqlExpression": "a = 1"}
+                {
+                    "clause": "WHERE",
+                    "expressionType": "SQL",
+                    "filterOptionName": "46fb6d7891e23596e42ae38da94a57e0",
+                    "sqlExpression": "a = 1",
+                }
             ]
         }
         convert_legacy_filters_into_adhoc(form_data)
         self.assertEqual(form_data, expected)
 
-    @patch("superset.utils.core.to_adhoc", mock_to_adhoc)
     def test_convert_legacy_filters_into_adhoc_present_and_nonempty(self):
         form_data = {
             "adhoc_filters": [

tests/integration_tests/viz_tests.py

@@ -1196,42 +1196,38 @@ class TestBaseDeckGLViz(SupersetTestCase):
                 {
                     "clause": "WHERE",
                     "expressionType": "SIMPLE",
-                    "filterOptionName": "bfa3a42a6f3de3c781b7d4f8e8d6613d",
+                    "filterOptionName": "c7f171cf3204bcbf456acfeac5cd9afd",
                     "comparator": "",
                     "operator": "IS NOT NULL",
                     "subject": "lat",
-                    "isExtra": False,
                 },
                 {
                     "clause": "WHERE",
                     "expressionType": "SIMPLE",
-                    "filterOptionName": "2d35d87b57c6f1a5ae139f1a6b0cbd0a",
+                    "filterOptionName": "52634073fbb8ae0a3aa59ad48abac55e",
                     "comparator": "",
                     "operator": "IS NOT NULL",
                     "subject": "lon",
-                    "isExtra": False,
                 },
             ],
             "delimited_key": [
                 {
                     "clause": "WHERE",
                     "expressionType": "SIMPLE",
-                    "filterOptionName": "89cc0fafe39a4eabc5df2cd52e4d6514",
+                    "filterOptionName": "cae5c925c140593743da08499e6fb207",
                     "comparator": "",
                     "operator": "IS NOT NULL",
                     "subject": "lonlat",
-                    "isExtra": False,
                 }
             ],
             "geohash_key": [
                 {
                     "clause": "WHERE",
                     "expressionType": "SIMPLE",
-                    "filterOptionName": "fa734d9a7bab254a53b41540d46cdb6c",
+                    "filterOptionName": "d84f55222d8e414e888fa5f990b341d2",
                     "comparator": "",
                     "operator": "IS NOT NULL",
                     "subject": "geo",
-                    "isExtra": False,
                 }
             ],
         }