From 578f9a6a04f5587f3f8a2fd830633d40755491ed Mon Sep 17 00:00:00 2001
From: Ville Brofeldt <ville.v.brofeldt@gmail.com>
Date: Mon, 26 Oct 2020 17:47:31 +0200
Subject: [PATCH] fix: is_temporal should be overridden by is_dttm value

---
 .../src/datasource/DatasourceEditor.jsx            |  2 +-
 superset/connectors/sqla/models.py                 | 14 ++++++++++++++
 tests/sqla_models_tests.py                         | 12 ++++++++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/superset-frontend/src/datasource/DatasourceEditor.jsx b/superset-frontend/src/datasource/DatasourceEditor.jsx
index 3cfba9411a139..70abc4693c975 100644
--- a/superset-frontend/src/datasource/DatasourceEditor.jsx
+++ b/superset-frontend/src/datasource/DatasourceEditor.jsx
@@ -180,7 +180,7 @@ function ColumnCollectionTable({
               control={
                 <TextControl
                   controlId="python_date_format"
-                  placeholder="%y/%m/%d"
+                  placeholder="%Y/%m/%d"
                 />
               }
             />
diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index 955dbb473724e..b93f5cddad2ca 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -181,6 +181,9 @@ class TableColumn(Model, BaseColumn):
 
     @property
     def is_numeric(self) -> bool:
+        """
+        Check if the column has a numeric datatype.
+        """
         db_engine_spec = self.table.database.db_engine_spec
         return db_engine_spec.is_db_column_type_match(
             self.type, utils.DbColumnType.NUMERIC
@@ -188,6 +191,9 @@ def is_numeric(self) -> bool:
 
     @property
     def is_string(self) -> bool:
+        """
+        Check if the column has a string datatype.
+        """
         db_engine_spec = self.table.database.db_engine_spec
         return db_engine_spec.is_db_column_type_match(
             self.type, utils.DbColumnType.STRING
@@ -195,6 +201,14 @@ def is_string(self) -> bool:
 
     @property
     def is_temporal(self) -> bool:
+        """
+        Check if the column has a temporal datatype. If column has been set as
+        temporal/non-temporal (`is_dttm` is True or False respectively), return that
+        value. This usually happens during initial metadata fetching or when a column
+        is manually set as temporal (for this `python_date_format` needs to be set).
+        """
+        if self.is_dttm is not None:
+            return self.is_dttm
         db_engine_spec = self.table.database.db_engine_spec
         return db_engine_spec.is_db_column_type_match(
             self.type, utils.DbColumnType.TEMPORAL
diff --git a/tests/sqla_models_tests.py b/tests/sqla_models_tests.py
index 9cdea16efb2fe..1f573f52fa9ac 100644
--- a/tests/sqla_models_tests.py
+++ b/tests/sqla_models_tests.py
@@ -44,6 +44,18 @@ def test_is_time_druid_time_col(self):
         col = TableColumn(column_name="__not_time", type="INTEGER", table=tbl)
         self.assertEqual(col.is_temporal, False)
 
+    def test_temporal_varchar(self):
+        """Ensure a column with is_dttm set to true evaluates to is_temporal == True"""
+
+        database = get_example_database()
+        tbl = SqlaTable(table_name="test_tbl", database=database)
+        col = TableColumn(column_name="ds", type="VARCHAR", table=tbl)
+        # by default, VARCHAR should not be assumed to be temporal
+        assert col.is_temporal is False
+        # changing to `is_dttm = True`, calling `is_temporal` should return True
+        col.is_dttm = True
+        assert col.is_temporal is True
+
     def test_db_column_types(self):
         test_cases: Dict[str, DbColumnType] = {
             # string