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

[okayhooni]

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.

• fix(presto/trino): Add TIME/TIMESTAMP WITH TIME ZONE #19263
• fix(trino): Fix Trino timestamp conversion #21737

But the modified codes with PR above, is removed on the PR below

• chore(db_engine_specs): clean up column spec logic and add tests #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

you can see the wrong converted results with a margin of error of 32 minutes.

(this is not the issue on Trino side, because this issue is not reproduced when I submit a query to same Trino cluster from other tools.)

Screenshots/recordings

Superset version

3.1.3

Python version

3.9

Node version

16

Browser

Chrome

Additional context

https://superset.apache.org/docs/configuration/timezones/

Checklist

• I have searched Superset docs and Slack and didn't find a solution to my problem.
• I have searched the GitHub issue tracker and didn't find a similar bug report.
• 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.

[dosubot]

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:

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][2][3].

For further details, you can refer to the pull requests:

• PR 19263
• PR 21737
• PR 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.

[okayhooni]

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: Address time zone localization issue trinodb/trino-python-client#368