giftig commented on code in PR #24054:
URL: https://github.com/apache/superset/pull/24054#discussion_r1193969232
##########
superset/db_engine_specs/presto.py:
##########
@@ -509,13 +507,13 @@ def where_latest_partition( # pylint:
disable=too-many-arguments
}
for col_name, value in zip(col_names, values):
- 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)
+ col_type = column_type_by_name.get(col_name)
+
+ if isinstance(col_type, (types.DATE, types.TIMESTAMP)):
+ value = text(f"{col_type} '{value}'")
Review Comment:
Actually when I went looking for the dialect, I found `presto_sql_types`
which seem to be the correct types to use to get the `DATE '...'`
functionality, so replacing it with this perhaps does the job better (I have
just tested this, though I still need to fix my unit test to deal with
tokenisation for it to work there):
```python
if isinstance(col_type, types.DATE):
col_type = Date() # from
superset.models.sql_types.presto_sql_types
elif isinstance(col_type, types.TIMESTAMP):
col_type = TimeStamp()
query = query.where(Column(col_name, col_type) == value)
```
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
