[GitHub] [superset] mdesmet commented on a diff in pull request #21737: fix(trino): Fix Trino timestamp conversion

[GitBox <gi...@apache.org>]

mdesmet commented on code in PR #21737:
URL: https://github.com/apache/superset/pull/21737#discussion_r1042776802


##########
superset/db_engine_specs/trino.py:
##########
@@ -47,6 +49,29 @@ class TrinoEngineSpec(PrestoBaseEngineSpec):
     engine = "trino"
     engine_name = "Trino"
 
+    @classmethod
+    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"DATE '{dttm.date().isoformat()}'"
+        if re.sub(r"\(\d\)", "", tt) in (

Review Comment:
   Trino exposes the precision in the temporal types, this is not covered in the Presto code (this is what's the bug was about)



##########
superset/db_engine_specs/trino.py:
##########
@@ -47,6 +49,29 @@ class TrinoEngineSpec(PrestoBaseEngineSpec):
     engine = "trino"
     engine_name = "Trino"
 
+    @classmethod
+    def convert_dttm(

Review Comment:
   Makes sense. I'll take care of that.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org