fix(presto/trino): Add TIME/TIMESTAMP WITH TIME ZONE (#19263)

Co-authored-by: John Bodley <[email protected]>

superset/db_engine_specs/base.py

@@ -812,7 +812,7 @@ def convert_dttm(  # pylint: disable=unused-argument
         cls, target_type: str, dttm: datetime, db_extra: Optional[Dict[str, Any]] = None
     ) -> Optional[str]:
         """
-        Convert Python datetime object to a SQL expression
+        Convert a Python `datetime` object to a SQL expression.
 
         :param target_type: The target type of expression
         :param dttm: The datetime object

superset/db_engine_specs/presto.py

@@ -725,10 +725,24 @@ def adjust_database_uri(
     def convert_dttm(
         cls, target_type: str, dttm: datetime, db_extra: Optional[Dict[str, Any]] = None
     ) -> Optional[str]:
+        """
+        Convert a Python `datetime` object to a SQL expression.
+
+        :param target_type: The target type of expression
+        :param dttm: The datetime object
+        :param db_extra: The database extra object
+        :return: The SQL expression
+
+        Superset only defines time zone naive `datetime` objects, though this method
+        handles both time zone naive and aware conversions.
+        """
         tt = target_type.upper()
         if tt == utils.TemporalType.DATE:
             return f"""from_iso8601_date('{dttm.date().isoformat()}')"""
-        if tt == utils.TemporalType.TIMESTAMP:
+        if tt in (
+            utils.TemporalType.TIMESTAMP,
+            utils.TemporalType.TIMESTAMP_WITH_TIME_ZONE,
+        ):
             return f"""from_iso8601_timestamp('{dttm.isoformat(timespec="microseconds")}')"""  # pylint: disable=line-too-long,useless-suppression
         return None
 

superset/db_engine_specs/trino.py

@@ -57,13 +57,25 @@ class TrinoEngineSpec(BaseEngineSpec):
     def convert_dttm(
         cls, target_type: str, dttm: datetime, db_extra: Optional[Dict[str, Any]] = None
     ) -> Optional[str]:
+        """
+        Convert a Python `datetime` object to a SQL expression.
+
+        :param target_type: The target type of expression
+        :param dttm: The datetime object
+        :param db_extra: The database extra object
+        :return: The SQL expression
+
+        Superset only defines time zone naive `datetime` objects, though this method
+        handles both time zone naive and aware conversions.
+        """
         tt = target_type.upper()
         if tt == utils.TemporalType.DATE:
-            value = dttm.date().isoformat()
-            return f"from_iso8601_date('{value}')"
-        if tt == utils.TemporalType.TIMESTAMP:
-            value = dttm.isoformat(timespec="microseconds")
-            return f"from_iso8601_timestamp('{value}')"
+            return f"from_iso8601_date('{dttm.date().isoformat()}')"
+        if tt in (
+            utils.TemporalType.TIMESTAMP,
+            utils.TemporalType.TIMESTAMP_WITH_TIME_ZONE,
+        ):
+            return f"""from_iso8601_timestamp('{dttm.isoformat(timespec="microseconds")}')"""  # pylint: disable=line-too-long,useless-suppression
         return None
 
     @classmethod

superset/utils/core.py

@@ -325,7 +325,9 @@ class TemporalType(str, Enum):
     SMALLDATETIME = "SMALLDATETIME"
     TEXT = "TEXT"
     TIME = "TIME"
+    TIME_WITH_TIME_ZONE = "TIME WITH TIME ZONE"
     TIMESTAMP = "TIMESTAMP"
+    TIMESTAMP_WITH_TIME_ZONE = "TIMESTAMP WITH TIME ZONE"
 
 
 class ColumnTypeSource(Enum):

tests/unit_tests/db_engine_specs/test_presto.py

@@ -0,0 +1,53 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from datetime import datetime
+from typing import Optional
+
+import pytest
+import pytz
+from flask.ctx import AppContext
+
+
+@pytest.mark.parametrize(
+    "target_type,dttm,result",
+    [
+        ("VARCHAR", datetime(2022, 1, 1), None),
+        ("DATE", datetime(2022, 1, 1), "from_iso8601_date('2022-01-01')"),
+        (
+            "TIMESTAMP",
+            datetime(2022, 1, 1, 1, 23, 45, 600000),
+            "from_iso8601_timestamp('2022-01-01T01:23:45.600000')",
+        ),
+        (
+            "TIMESTAMP WITH TIME ZONE",
+            datetime(2022, 1, 1, 1, 23, 45, 600000),
+            "from_iso8601_timestamp('2022-01-01T01:23:45.600000')",
+        ),
+        (
+            "TIMESTAMP WITH TIME ZONE",
+            datetime(2022, 1, 1, 1, 23, 45, 600000, tzinfo=pytz.UTC),
+            "from_iso8601_timestamp('2022-01-01T01:23:45.600000+00:00')",
+        ),
+    ],
+)
+def test_convert_dttm(
+    app_context: AppContext, target_type: str, dttm: datetime, result: Optional[str],
+) -> None:
+    from superset.db_engine_specs.presto import PrestoEngineSpec
+
+    for case in (str.lower, str.upper):
+        assert PrestoEngineSpec.convert_dttm(case(target_type), dttm) == result

tests/unit_tests/db_engine_specs/test_trino.py

@@ -0,0 +1,53 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from datetime import datetime
+from typing import Optional
+
+import pytest
+import pytz
+from flask.ctx import AppContext
+
+
+@pytest.mark.parametrize(
+    "target_type,dttm,result",
+    [
+        ("VARCHAR", datetime(2022, 1, 1), None),
+        ("DATE", datetime(2022, 1, 1), "from_iso8601_date('2022-01-01')"),
+        (
+            "TIMESTAMP",
+            datetime(2022, 1, 1, 1, 23, 45, 600000),
+            "from_iso8601_timestamp('2022-01-01T01:23:45.600000')",
+        ),
+        (
+            "TIMESTAMP WITH TIME ZONE",
+            datetime(2022, 1, 1, 1, 23, 45, 600000),
+            "from_iso8601_timestamp('2022-01-01T01:23:45.600000')",
+        ),
+        (
+            "TIMESTAMP WITH TIME ZONE",
+            datetime(2022, 1, 1, 1, 23, 45, 600000, tzinfo=pytz.UTC),
+            "from_iso8601_timestamp('2022-01-01T01:23:45.600000+00:00')",
+        ),
+    ],
+)
+def test_convert_dttm(
+    app_context: AppContext, target_type: str, dttm: datetime, result: Optional[str],
+) -> None:
+    from superset.db_engine_specs.trino import TrinoEngineSpec
+
+    for case in (str.lower, str.upper):
+        assert TrinoEngineSpec.convert_dttm(case(target_type), dttm) == result