Re: [I] BUG) Superset get wrong value of TIMESTAMP WITH TIME ZONE type field on Trino client [superset]

2024-05-24 Thread via GitHub


okayhooni closed issue #28685: BUG) Superset get wrong value of TIMESTAMP WITH 
TIME ZONE type field on Trino client
URL: https://github.com/apache/superset/issues/28685


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



Re: [I] BUG) Superset get wrong value of TIMESTAMP WITH TIME ZONE type field on Trino client [superset]

2024-05-24 Thread via GitHub


okayhooni commented on issue #28685:
URL: https://github.com/apache/superset/issues/28685#issuecomment-2128744546

   I found this issue was related to the bug on `trino-python-client` itself.
   
   this issue was fixed on `v0.324` of that library, so I solved this issue by 
updating this dependency from `v0.321` to `0.328`
   
   - related MR: https://github.com/trinodb/trino-python-client/pull/368


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



Re: [I] BUG) Superset get wrong value of TIMESTAMP WITH TIME ZONE type field on Trino client [superset]

2024-05-23 Thread via GitHub


dosubot[bot] commented on issue #28685:
URL: https://github.com/apache/superset/issues/28685#issuecomment-2128457789

   
   Hey @okayhooni! I'm here to help you with any bugs, questions, or 
contributions you have for the project. Let's squash those bugs together! 🐞
   
   
   The issue you're encountering with `TIMESTAMP WITH TIME ZONE` values from 
Trino in Apache Superset version 3.1.3 has been addressed in several pull 
requests, but the changes were later removed. Here are the key points and 
solutions:
   
   1. **PR 19263**: This pull request introduced changes to handle `TIMESTAMP 
WITH TIME ZONE` values correctly. It added a method to convert these values to 
a SQL expression in the `db_engine_specs/presto.py` file.
  
   2. **PR 21737**: This pull request also aimed to fix issues related to 
`TIMESTAMP WITH TIME ZONE` values from Trino, but specific changes related to 
this were not detailed in the provided context.
   
   3. **PR 22871**: This pull request removed the changes introduced in the 
previous pull requests, which might have reintroduced the issue.
   
   To fix the issue, you can manually reapply the changes from PR 19263 to your 
local `db_engine_specs/presto.py` file. Here is the relevant code snippet:
   
   ```python
   class PrestoBaseEngineSpec(BaseEngineSpec, metaclass=ABCMeta):
   """
   A base class that share common functions between Presto and Trino
   """
   
   supports_dynamic_schema = True
   supports_catalog = supports_dynamic_catalog = True
   
   column_type_mappings = (
   # ... other mappings ...
   (
   re.compile(r"^timestamp with time zone.*", re.IGNORECASE),
   types.TIMESTAMP(timezone=True),
   GenericDataType.TEMPORAL,
   ),
   # ... other mappings ...
   )
   
   @classmethod
   def convert_dttm(
   cls, target_type: str, dttm: datetime, db_extra: dict[str, Any] | 
None = None
   ) -> str | None:
   """
   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.
   """
   sqla_type = cls.get_sqla_column_type(target_type)
   
   if isinstance(sqla_type, types.Date):
   return f"DATE '{dttm.date().isoformat()}'"
   if isinstance(sqla_type, types.TIMESTAMP):
   if dttm.tzinfo is not None:
   return f"""TIMESTAMP WITH TIME ZONE 
'{dttm.isoformat(timespec="microseconds", sep=" ")}'"""
   return f"""TIMESTAMP '{dttm.isoformat(timespec="microseconds", 
sep=" ")}'"""
   
   return None
   ```
   
   Additionally, ensure that both the Apache Superset backend and the database 
engine are configured to use the same timezone, preferably UTC, to avoid 
inconsistencies 
[[1]](https://github.com/apache/superset/blob/master/superset/db_engine_specs/presto.py)[[2]](https://github.com/apache/superset/blob/master/docs/docs/configuration/timezones.mdx)[[3]](https://github.com/apache/superset/blob/master/superset/result_set.py).
   
   For further details, you can refer to the pull requests:
   - [PR 19263](https://github.com/apache/superset/pull/19263)
   - [PR 21737](https://github.com/apache/superset/pull/21737)
   - [PR 22871](https://github.com/apache/superset/pull/22871)
   
   
   
   
   
   
    Details
   
   
   **About Dosu**
   This response is meant to be useful and save you time. It is **not meant to 
be a precise solution**, but rather a starting point for your own research.
   
   Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is 
irrelevant.
   
   
   *To continue the conversation, mention @dosu.*
   


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



[I] BUG) Superset get wrong value of TIMESTAMP WITH TIME ZONE type field on Trino client [superset]

2024-05-23 Thread via GitHub


okayhooni opened a new issue, #28685:
URL: https://github.com/apache/superset/issues/28685

   ### Bug description
   
   when get value with `TIMESTAMP WITH TIME ZONE` type from Trino, 
`db_engine_specs/presto.py` code of Superset cannot handle that value properly. 
(please see the screenshot below.)
   
   I found some issues and MR to fix this issue.
   - https://github.com/apache/superset/pull/19263
   - https://github.com/apache/superset/pull/21737
   
   But the modified codes with PR above, is removed on the PR below
   - https://github.com/apache/superset/pull/22871
   
   ### How to reproduce the bug
   
   just submit a query like below, to Trino.
   ```
   SELECT
   now() as server_time,
   now() at time zone 'UTC' as utc_time,
   now() at time zone 'Asia/Seoul' as seoul_time, -- wrong
   CAST(now() at time zone 'Asia/Seoul' as TIMESTAMP WITH TIME ZONE) as 
seoul_time_2, -- wrong
   CAST(now() at time zone 'Asia/Seoul' as TIMESTAMP) as seoul_time_3 -- valid
   ```
   
   ### Screenshots/recordings
   
   
![image](https://github.com/apache/superset/assets/81631424/b6508975-8be9-45b2-98a0-dcfe9de818a2)
   
   
   ### Superset version
   
   3.1.3
   
   ### Python version
   
   3.9
   
   ### Node version
   
   16
   
   ### Browser
   
   Chrome
   
   ### Additional context
   
   https://superset.apache.org/docs/configuration/timezones/
   
   ### Checklist
   
   - [X] I have searched Superset docs and Slack and didn't find a solution to 
my problem.
   - [X] I have searched the GitHub issue tracker and didn't find a similar bug 
report.
   - [X] I have checked Superset's logs for errors and if I found a relevant 
Python stacktrace, I included it here as text in the "additional context" 
section.


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