kekwan commented on a change in pull request #13082:
URL: https://github.com/apache/superset/pull/13082#discussion_r583138335



##########
File path: superset/db_engine_specs/presto.py
##########
@@ -911,10 +913,18 @@ def where_latest_partition(  # pylint: 
disable=too-many-arguments
         if values is None:
             return None
 
-        column_names = {column.get("name") for column in columns or []}
+        column_type_by_name = {
+            column.get("name"): column.get("type") for column in columns or []
+        }
+
         for col_name, value in zip(col_names, values):
-            if col_name in column_names:
-                query = query.where(Column(col_name) == value)
+            if col_name in column_type_by_name:
+                if column_type_by_name.get(col_name) == "TIMESTAMP":
+                    query = query.where(Column(col_name, TimeStamp()) == value)
+                elif column_type_by_name.get(col_name) == "DATE":
+                    query = query.where(Column(col_name, Date()) == value)
+                else:
+                    query = query.where(Column(col_name) == value)

Review comment:
       Certainly possible but the drawbacks I see to re-using this existing 
method is that: 
   
   1. We have to explicitly cast the string `value` to a python `datetime` to 
call the method only to get it converted back to a string with `.isoformat`.
   2. `from_iso8601_x` doesn't seem to be a recommended function anymore in the 
latest versions of Presto/Trino 
https://trino.io/docs/current/language/types.html#date-and-time
   3. This method only works if your date/timestamps are in ISO 8601 format. 
   4. In the future, if more Trino types are to be supported in previewing data 
with partitions, we will have to create new types like this anyways. Also, I 
think this is more the sqlalchemy friendly way. 




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to