Skip to content

Commit

Permalink
fix(presto/trino): Add TIME/TIMESTAMP WITH TIME ZONE (#19263)
Browse files Browse the repository at this point in the history
Co-authored-by: John Bodley <john.bodley@airbnb.com>
(cherry picked from commit 82a6811)
  • Loading branch information
john-bodley authored and villebro committed Apr 3, 2022
1 parent dc01531 commit 81a1abf
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 7 deletions.
2 changes: 1 addition & 1 deletion superset/db_engine_specs/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,7 @@ def convert_dttm( # pylint: disable=unused-argument
cls, target_type: str, dttm: datetime, db_extra: Optional[Dict[str, Any]] = None
) -> Optional[str]:
"""
Convert Python datetime object to a SQL expression
Convert a Python `datetime` object to a SQL expression.
:param target_type: The target type of expression
:param dttm: The datetime object
Expand Down
16 changes: 15 additions & 1 deletion superset/db_engine_specs/presto.py
Original file line number Diff line number Diff line change
Expand Up @@ -725,10 +725,24 @@ def adjust_database_uri(
def convert_dttm(
cls, target_type: str, dttm: datetime, db_extra: Optional[Dict[str, Any]] = None
) -> Optional[str]:
"""
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.
"""
tt = target_type.upper()
if tt == utils.TemporalType.DATE:
return f"""from_iso8601_date('{dttm.date().isoformat()}')"""
if tt == utils.TemporalType.TIMESTAMP:
if tt in (
utils.TemporalType.TIMESTAMP,
utils.TemporalType.TIMESTAMP_WITH_TIME_ZONE,
):
return f"""from_iso8601_timestamp('{dttm.isoformat(timespec="microseconds")}')""" # pylint: disable=line-too-long,useless-suppression
return None

Expand Down
22 changes: 17 additions & 5 deletions superset/db_engine_specs/trino.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,25 @@ class TrinoEngineSpec(BaseEngineSpec):
def convert_dttm(
cls, target_type: str, dttm: datetime, db_extra: Optional[Dict[str, Any]] = None
) -> Optional[str]:
"""
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.
"""
tt = target_type.upper()
if tt == utils.TemporalType.DATE:
value = dttm.date().isoformat()
return f"from_iso8601_date('{value}')"
if tt == utils.TemporalType.TIMESTAMP:
value = dttm.isoformat(timespec="microseconds")
return f"from_iso8601_timestamp('{value}')"
return f"from_iso8601_date('{dttm.date().isoformat()}')"
if tt in (
utils.TemporalType.TIMESTAMP,
utils.TemporalType.TIMESTAMP_WITH_TIME_ZONE,
):
return f"""from_iso8601_timestamp('{dttm.isoformat(timespec="microseconds")}')""" # pylint: disable=line-too-long,useless-suppression
return None

@classmethod
Expand Down
2 changes: 2 additions & 0 deletions superset/utils/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,9 @@ class TemporalType(str, Enum):
SMALLDATETIME = "SMALLDATETIME"
TEXT = "TEXT"
TIME = "TIME"
TIME_WITH_TIME_ZONE = "TIME WITH TIME ZONE"
TIMESTAMP = "TIMESTAMP"
TIMESTAMP_WITH_TIME_ZONE = "TIMESTAMP WITH TIME ZONE"


class ColumnTypeSource(Enum):
Expand Down
53 changes: 53 additions & 0 deletions tests/unit_tests/db_engine_specs/test_presto.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from datetime import datetime
from typing import Optional

import pytest
import pytz
from flask.ctx import AppContext


@pytest.mark.parametrize(
"target_type,dttm,result",
[
("VARCHAR", datetime(2022, 1, 1), None),
("DATE", datetime(2022, 1, 1), "from_iso8601_date('2022-01-01')"),
(
"TIMESTAMP",
datetime(2022, 1, 1, 1, 23, 45, 600000),
"from_iso8601_timestamp('2022-01-01T01:23:45.600000')",
),
(
"TIMESTAMP WITH TIME ZONE",
datetime(2022, 1, 1, 1, 23, 45, 600000),
"from_iso8601_timestamp('2022-01-01T01:23:45.600000')",
),
(
"TIMESTAMP WITH TIME ZONE",
datetime(2022, 1, 1, 1, 23, 45, 600000, tzinfo=pytz.UTC),
"from_iso8601_timestamp('2022-01-01T01:23:45.600000+00:00')",
),
],
)
def test_convert_dttm(
app_context: AppContext, target_type: str, dttm: datetime, result: Optional[str],
) -> None:
from superset.db_engine_specs.presto import PrestoEngineSpec

for case in (str.lower, str.upper):
assert PrestoEngineSpec.convert_dttm(case(target_type), dttm) == result
53 changes: 53 additions & 0 deletions tests/unit_tests/db_engine_specs/test_trino.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from datetime import datetime
from typing import Optional

import pytest
import pytz
from flask.ctx import AppContext


@pytest.mark.parametrize(
"target_type,dttm,result",
[
("VARCHAR", datetime(2022, 1, 1), None),
("DATE", datetime(2022, 1, 1), "from_iso8601_date('2022-01-01')"),
(
"TIMESTAMP",
datetime(2022, 1, 1, 1, 23, 45, 600000),
"from_iso8601_timestamp('2022-01-01T01:23:45.600000')",
),
(
"TIMESTAMP WITH TIME ZONE",
datetime(2022, 1, 1, 1, 23, 45, 600000),
"from_iso8601_timestamp('2022-01-01T01:23:45.600000')",
),
(
"TIMESTAMP WITH TIME ZONE",
datetime(2022, 1, 1, 1, 23, 45, 600000, tzinfo=pytz.UTC),
"from_iso8601_timestamp('2022-01-01T01:23:45.600000+00:00')",
),
],
)
def test_convert_dttm(
app_context: AppContext, target_type: str, dttm: datetime, result: Optional[str],
) -> None:
from superset.db_engine_specs.trino import TrinoEngineSpec

for case in (str.lower, str.upper):
assert TrinoEngineSpec.convert_dttm(case(target_type), dttm) == result

0 comments on commit 81a1abf

Please sign in to comment.