villebro commented on a change in pull request #13082:
URL: https://github.com/apache/superset/pull/13082#discussion_r582631339
##########
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:
I'm wondering if we couldn't just use the existing `convert_dttm` here,
as we should already have type casting logic in place for these types of
conversions?
https://github.com/apache/superset/blob/8ab45c90ccc04f51a0e7db12b622fb63a29ec27c/superset/db_engine_specs/presto.py#L597-L604
Something like `query = query.where(Column(col_name) ==
cls.convert_dttm(column_type_by_name.get(col_name), value)` (not tested!
thinking out loud)
----------------------------------------------------------------
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]
