You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by vi...@apache.org on 2021/09/22 10:35:22 UTC

[superset] 05/17: fix: TemporalWrapperType string representation (#16614)

This is an automated email from the ASF dual-hosted git repository.

villebro pushed a commit to branch 1.3
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 72fe94842da3338d7e52ecb0e32a4562bdceee0c
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Tue Sep 7 13:50:24 2021 +0300

    fix: TemporalWrapperType string representation (#16614)
    
    * fix: TemporalWrapperType string representation
    
    * fix tests
    
    (cherry picked from commit 9de2196b7ffee3eae5ffd61d350172b0d52b40c5)
---
 superset/db_engine_specs/base.py                        |  2 +-
 superset/models/sql_types/base.py                       | 10 ++++------
 tests/integration_tests/db_engine_specs/presto_tests.py |  6 ++++--
 tests/integration_tests/model_tests.py                  | 10 ++++++++++
 4 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py
index 8f4ed76..6461612 100644
--- a/superset/db_engine_specs/base.py
+++ b/superset/db_engine_specs/base.py
@@ -1297,7 +1297,7 @@ class BaseEngineSpec:  # pylint: disable=too-many-public-methods
             # using datetimes
             if generic_type == GenericDataType.TEMPORAL:
                 column_type = literal_dttm_type_factory(
-                    type(column_type), cls, native_type or ""
+                    column_type, cls, native_type or ""
                 )
             is_dttm = generic_type == GenericDataType.TEMPORAL
             return ColumnSpec(
diff --git a/superset/models/sql_types/base.py b/superset/models/sql_types/base.py
index 0d9eda4..669181b 100644
--- a/superset/models/sql_types/base.py
+++ b/superset/models/sql_types/base.py
@@ -26,10 +26,8 @@ if TYPE_CHECKING:
 
 
 def literal_dttm_type_factory(
-    sqla_type: Type[types.TypeEngine],
-    db_engine_spec: Type["BaseEngineSpec"],
-    col_type: str,
-) -> Type[types.TypeEngine]:
+    sqla_type: types.TypeEngine, db_engine_spec: Type["BaseEngineSpec"], col_type: str,
+) -> types.TypeEngine:
     """
     Create a custom SQLAlchemy type that supports datetime literal binds.
 
@@ -39,7 +37,7 @@ def literal_dttm_type_factory(
     :return: SQLAlchemy type that supports using datetima as literal bind
     """
     # pylint: disable=too-few-public-methods
-    class TemporalWrapperType(sqla_type):  # type: ignore
+    class TemporalWrapperType(type(sqla_type)):  # type: ignore
         # pylint: disable=unused-argument
         def literal_processor(self, dialect: Dialect) -> Callable[[Any], Any]:
             def process(value: Any) -> Any:
@@ -58,4 +56,4 @@ def literal_dttm_type_factory(
 
             return process
 
-    return TemporalWrapperType
+    return TemporalWrapperType()
diff --git a/tests/integration_tests/db_engine_specs/presto_tests.py b/tests/integration_tests/db_engine_specs/presto_tests.py
index 3b48f67..3b7596b 100644
--- a/tests/integration_tests/db_engine_specs/presto_tests.py
+++ b/tests/integration_tests/db_engine_specs/presto_tests.py
@@ -561,11 +561,13 @@ class TestPrestoDbEngineSpec(TestDbEngineSpec):
         self.assertEqual(column_spec.generic_type, GenericDataType.NUMERIC)
 
         column_spec = PrestoEngineSpec.get_column_spec("time")
-        assert issubclass(column_spec.sqla_type, types.Time)
+        assert isinstance(column_spec.sqla_type, types.Time)
+        assert type(column_spec.sqla_type).__name__ == "TemporalWrapperType"
         self.assertEqual(column_spec.generic_type, GenericDataType.TEMPORAL)
 
         column_spec = PrestoEngineSpec.get_column_spec("timestamp")
-        assert issubclass(column_spec.sqla_type, types.TIMESTAMP)
+        assert isinstance(column_spec.sqla_type, types.TIMESTAMP)
+        assert type(column_spec.sqla_type).__name__ == "TemporalWrapperType"
         self.assertEqual(column_spec.generic_type, GenericDataType.TEMPORAL)
 
         sqla_type = PrestoEngineSpec.get_sqla_column_type(None)
diff --git a/tests/integration_tests/model_tests.py b/tests/integration_tests/model_tests.py
index deb7846..567fdfe 100644
--- a/tests/integration_tests/model_tests.py
+++ b/tests/integration_tests/model_tests.py
@@ -24,11 +24,14 @@ from tests.integration_tests.fixtures.birth_names_dashboard import (
 
 import pytest
 from sqlalchemy.engine.url import make_url
+from sqlalchemy.types import DateTime
 
 import tests.integration_tests.test_app
 from superset import app, db as metadata_db
+from superset.db_engine_specs.postgres import PostgresEngineSpec
 from superset.models.core import Database
 from superset.models.slice import Slice
+from superset.models.sql_types.base import literal_dttm_type_factory
 from superset.utils.core import get_example_database, QueryStatus
 
 from .base_tests import SupersetTestCase
@@ -516,3 +519,10 @@ class TestSqlaTableModel(SupersetTestCase):
         assert set(data_for_slices["verbose_map"].keys()) == set(
             ["__timestamp", "sum__num", "gender",]
         )
+
+
+def test_literal_dttm_type_factory():
+    orig_type = DateTime()
+    new_type = literal_dttm_type_factory(orig_type, PostgresEngineSpec, "TIMESTAMP")
+    assert type(new_type).__name__ == "TemporalWrapperType"
+    assert str(new_type) == str(orig_type)