From c4d70e42fc8901ae490dbdbe0754456876724095 Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Wed, 4 Oct 2023 15:27:23 +0800
Subject: [PATCH 01/19] fix: change the validation logic for python_date_format

---
 superset/datasets/schemas.py | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py
index 0b137e96a7da9..4bbe523289952 100644
--- a/superset/datasets/schemas.py
+++ b/superset/datasets/schemas.py
@@ -15,7 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 import json
-import re
+from datetime import datetime
 from typing import Any
 
 from flask_babel import lazy_gettext as _
@@ -43,18 +43,12 @@
 
 
 def validate_python_date_format(value: str) -> None:
-    regex = re.compile(
-        r"""
-        ^(
-            epoch_s|epoch_ms|
-            (?P<date>%Y([-/]%m([-/]%d)?)?)([\sT](?P<time>%H(:%M(:%S(\.%f)?)?)?))?
-        )$
-        """,
-        re.VERBOSE,
-    )
-    match = regex.match(value or "")
-    if not match:
-        raise ValidationError([_("Invalid date/timestamp format")])
+    if value in ("epoch_s", "epoch_ms"):
+        return
+    try:
+        datetime.now().strftime(value)
+    except ValueError as ex:
+        raise ValidationError([_("Invalid date/timestamp format")]) from ex
 
 
 class DatasetColumnsPutSchema(Schema):

From 01084564406ee597cc795ae0aae418f4754dea7b Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Thu, 5 Oct 2023 19:38:15 +0800
Subject: [PATCH 02/19] fix: strftime may receive None.

---
 superset/datasets/schemas.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py
index 4bbe523289952..f60e6d185fd51 100644
--- a/superset/datasets/schemas.py
+++ b/superset/datasets/schemas.py
@@ -46,7 +46,7 @@ def validate_python_date_format(value: str) -> None:
     if value in ("epoch_s", "epoch_ms"):
         return
     try:
-        datetime.now().strftime(value)
+        datetime.now().strftime(value or "")
     except ValueError as ex:
         raise ValidationError([_("Invalid date/timestamp format")]) from ex
 

From 0cb09cced32235d5fe63cd9feb9c710735edded7 Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Thu, 5 Oct 2023 19:39:36 +0800
Subject: [PATCH 03/19] add unit tests

---
 tests/unit_tests/datasets/schema_tests.py | 74 +++++++++++++++++++++++
 1 file changed, 74 insertions(+)
 create mode 100644 tests/unit_tests/datasets/schema_tests.py

diff --git a/tests/unit_tests/datasets/schema_tests.py b/tests/unit_tests/datasets/schema_tests.py
new file mode 100644
index 0000000000000..e04765ba0b794
--- /dev/null
+++ b/tests/unit_tests/datasets/schema_tests.py
@@ -0,0 +1,74 @@
+# 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.
+
+# pylint: disable=import-outside-toplevel, invalid-name, unused-argument, redefined-outer-name
+from marshmallow import ValidationError
+import pytest
+
+from superset.datasets.schemas import DatasetColumnsPutSchema
+
+# pylint: disable=too-few-public-methods
+
+@pytest.mark.parametrize("payload", [
+    (
+        {
+            "column_name": "insert_time",
+            "filterable": True,
+            "groupby": True,
+            "python_date_format": None,
+        }
+    ),
+    (
+        {
+            "column_name": "insert_time",
+            "filterable": True,
+            "groupby": True,
+            "python_date_format": "epoch_ms",
+        }
+    ),
+    (
+        {
+            "column_name": "insert_time",
+            "filterable": True,
+            "groupby": True,
+            "python_date_format": "epoch_s",
+        }
+    ),
+    (
+        {
+            "column_name": "insert_time",
+            "filterable": True,
+            "groupby": True,
+            "python_date_format": "%Y/%m/%dT%H:%M:%S.%f",
+        }
+    ),
+    (
+        {
+            "column_name": "insert_time",
+            "filterable": True,
+            "groupby": True,
+            "python_date_format": "%Y%m%d",
+        }
+    ),
+])
+def test_datasets_schema_update_column_datetime_format(payload) -> None:
+    schema = DatasetColumnsPutSchema()
+
+    try:
+        schema.load(payload)
+    except ValidationError as err:
+        assert False, err.messages
\ No newline at end of file

From 7c7210f8dc86d7deaf378484946cbe5e545ef3f4 Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Thu, 5 Oct 2023 20:01:16 +0800
Subject: [PATCH 04/19] re-run pre-commit

---
 tests/unit_tests/datasets/schema_tests.py | 95 ++++++++++++-----------
 1 file changed, 49 insertions(+), 46 deletions(-)

diff --git a/tests/unit_tests/datasets/schema_tests.py b/tests/unit_tests/datasets/schema_tests.py
index e04765ba0b794..58e3f9fb0c110 100644
--- a/tests/unit_tests/datasets/schema_tests.py
+++ b/tests/unit_tests/datasets/schema_tests.py
@@ -16,59 +16,62 @@
 # under the License.
 
 # pylint: disable=import-outside-toplevel, invalid-name, unused-argument, redefined-outer-name
-from marshmallow import ValidationError
 import pytest
+from marshmallow import ValidationError
 
-from superset.datasets.schemas import DatasetColumnsPutSchema
 
 # pylint: disable=too-few-public-methods
-
-@pytest.mark.parametrize("payload", [
-    (
-        {
-            "column_name": "insert_time",
-            "filterable": True,
-            "groupby": True,
-            "python_date_format": None,
-        }
-    ),
-    (
-        {
-            "column_name": "insert_time",
-            "filterable": True,
-            "groupby": True,
-            "python_date_format": "epoch_ms",
-        }
-    ),
-    (
-        {
-            "column_name": "insert_time",
-            "filterable": True,
-            "groupby": True,
-            "python_date_format": "epoch_s",
-        }
-    ),
-    (
-        {
-            "column_name": "insert_time",
-            "filterable": True,
-            "groupby": True,
-            "python_date_format": "%Y/%m/%dT%H:%M:%S.%f",
-        }
-    ),
-    (
-        {
-            "column_name": "insert_time",
-            "filterable": True,
-            "groupby": True,
-            "python_date_format": "%Y%m%d",
-        }
-    ),
-])
+@pytest.mark.parametrize(
+    "payload",
+    [
+        (
+            {
+                "column_name": "insert_time",
+                "filterable": True,
+                "groupby": True,
+                "python_date_format": None,
+            }
+        ),
+        (
+            {
+                "column_name": "insert_time",
+                "filterable": True,
+                "groupby": True,
+                "python_date_format": "epoch_ms",
+            }
+        ),
+        (
+            {
+                "column_name": "insert_time",
+                "filterable": True,
+                "groupby": True,
+                "python_date_format": "epoch_s",
+            }
+        ),
+        (
+            {
+                "column_name": "insert_time",
+                "filterable": True,
+                "groupby": True,
+                "python_date_format": "%Y/%m/%dT%H:%M:%S.%f",
+            }
+        ),
+        (
+            {
+                "column_name": "insert_time",
+                "filterable": True,
+                "groupby": True,
+                "python_date_format": "%Y%m%d",
+            }
+        ),
+    ],
+)
 def test_datasets_schema_update_column_datetime_format(payload) -> None:
+    from superset.datasets.schemas import DatasetColumnsPutSchema
+
     schema = DatasetColumnsPutSchema()
 
     try:
         schema.load(payload)
     except ValidationError as err:
-        assert False, err.messages
\ No newline at end of file
+        assert False, err.messages

From 2f05b7f118e835cc434d10160fcd265aca2c306e Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Fri, 17 Nov 2023 16:08:03 +0800
Subject: [PATCH 05/19] rewrite the unit test

---
 tests/unit_tests/datasets/schema_tests.py | 66 +++++++----------------
 1 file changed, 19 insertions(+), 47 deletions(-)

diff --git a/tests/unit_tests/datasets/schema_tests.py b/tests/unit_tests/datasets/schema_tests.py
index 58e3f9fb0c110..973b7236f1061 100644
--- a/tests/unit_tests/datasets/schema_tests.py
+++ b/tests/unit_tests/datasets/schema_tests.py
@@ -19,59 +19,31 @@
 import pytest
 from marshmallow import ValidationError
 
+from superset.datasets.schemas import validate_python_date_format
+
 
 # pylint: disable=too-few-public-methods
 @pytest.mark.parametrize(
     "payload",
     [
-        (
-            {
-                "column_name": "insert_time",
-                "filterable": True,
-                "groupby": True,
-                "python_date_format": None,
-            }
-        ),
-        (
-            {
-                "column_name": "insert_time",
-                "filterable": True,
-                "groupby": True,
-                "python_date_format": "epoch_ms",
-            }
-        ),
-        (
-            {
-                "column_name": "insert_time",
-                "filterable": True,
-                "groupby": True,
-                "python_date_format": "epoch_s",
-            }
-        ),
-        (
-            {
-                "column_name": "insert_time",
-                "filterable": True,
-                "groupby": True,
-                "python_date_format": "%Y/%m/%dT%H:%M:%S.%f",
-            }
-        ),
-        (
-            {
-                "column_name": "insert_time",
-                "filterable": True,
-                "groupby": True,
-                "python_date_format": "%Y%m%d",
-            }
-        ),
+        None,
+        "epoch_ms",
+        "epoch_s",
+        "%Y/%m/%dT%H:%M:%S.%f",
+        "%Y-%m-%dT%H:%M:%S.%f",
+        "%Y%m%d",
     ],
 )
-def test_datasets_schema_update_column_datetime_format(payload) -> None:
-    from superset.datasets.schemas import DatasetColumnsPutSchema
+def test_validate_python_date_format(payload) -> None:
+    assert None == validate_python_date_format(payload)
 
-    schema = DatasetColumnsPutSchema()
 
-    try:
-        schema.load(payload)
-    except ValidationError as err:
-        assert False, err.messages
+@pytest.mark.parametrize(
+    "payload",
+    [
+        "%d%m%Y",
+    ],
+)
+def test_validate_python_date_format_raises(payload) -> None:
+    with pytest.raises(ValidationError):
+        validate_python_date_format(payload)

From 4c849e290d05223c2ed75db488b20153fd8e55d6 Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Fri, 17 Nov 2023 16:59:25 +0800
Subject: [PATCH 06/19] change the validate method to dateutil.parser.isoparse

---
 superset/datasets/schemas.py              | 9 ++++++---
 tests/unit_tests/datasets/schema_tests.py | 2 +-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py
index f60e6d185fd51..a56c8c9b77c25 100644
--- a/superset/datasets/schemas.py
+++ b/superset/datasets/schemas.py
@@ -18,6 +18,7 @@
 from datetime import datetime
 from typing import Any
 
+from dateutil.parser import isoparse
 from flask_babel import lazy_gettext as _
 from marshmallow import fields, pre_load, Schema, ValidationError
 from marshmallow.validate import Length
@@ -43,12 +44,14 @@
 
 
 def validate_python_date_format(value: str) -> None:
-    if value in ("epoch_s", "epoch_ms"):
-        return
+    if value in ("epoch_s", "epoch_ms", None):
+        return None
     try:
-        datetime.now().strftime(value or "")
+        dt_str = datetime.now().strftime(value)
+        isoparse(dt_str)
     except ValueError as ex:
         raise ValidationError([_("Invalid date/timestamp format")]) from ex
+    return None
 
 
 class DatasetColumnsPutSchema(Schema):
diff --git a/tests/unit_tests/datasets/schema_tests.py b/tests/unit_tests/datasets/schema_tests.py
index 973b7236f1061..647bce56e6ff3 100644
--- a/tests/unit_tests/datasets/schema_tests.py
+++ b/tests/unit_tests/datasets/schema_tests.py
@@ -35,7 +35,7 @@
     ],
 )
 def test_validate_python_date_format(payload) -> None:
-    assert None == validate_python_date_format(payload)
+    assert validate_python_date_format(payload) is None
 
 
 @pytest.mark.parametrize(

From dfb55e1f90ef1a123c6a0907a8ef913829233681 Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Fri, 17 Nov 2023 17:50:28 +0800
Subject: [PATCH 07/19] Adjust function return type

---
 superset/datasets/schemas.py              | 11 +++++++----
 tests/unit_tests/datasets/schema_tests.py |  2 +-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py
index a56c8c9b77c25..37d3686519237 100644
--- a/superset/datasets/schemas.py
+++ b/superset/datasets/schemas.py
@@ -43,22 +43,25 @@
 }
 
 
-def validate_python_date_format(value: str) -> None:
+def validate_python_date_format(value: str) -> bool:
     if value in ("epoch_s", "epoch_ms", None):
-        return None
+        return True
     try:
         dt_str = datetime.now().strftime(value)
         isoparse(dt_str)
     except ValueError as ex:
         raise ValidationError([_("Invalid date/timestamp format")]) from ex
-    return None
+    return True
 
 
 class DatasetColumnsPutSchema(Schema):
     id = fields.Integer(required=False)
     column_name = fields.String(required=True, validate=Length(1, 255))
     type = fields.String(allow_none=True)
-    advanced_data_type = fields.String(allow_none=True, validate=Length(1, 255))
+    advanced_data_type = fields.String(
+        allow_none=True,
+        validate=Length(1, 255),
+    )
     verbose_name = fields.String(allow_none=True, metadata={Length: (1, 1024)})
     description = fields.String(allow_none=True)
     expression = fields.String(allow_none=True)
diff --git a/tests/unit_tests/datasets/schema_tests.py b/tests/unit_tests/datasets/schema_tests.py
index 647bce56e6ff3..4c8d8cb456664 100644
--- a/tests/unit_tests/datasets/schema_tests.py
+++ b/tests/unit_tests/datasets/schema_tests.py
@@ -35,7 +35,7 @@
     ],
 )
 def test_validate_python_date_format(payload) -> None:
-    assert validate_python_date_format(payload) is None
+    assert validate_python_date_format(payload)
 
 
 @pytest.mark.parametrize(

From ff6ca171042a6e63df54769e0e7dc1128ed872ad Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Sat, 18 Nov 2023 15:10:32 +0800
Subject: [PATCH 08/19] `%Y/%m/%d` should be the raises case

---
 tests/unit_tests/datasets/schema_tests.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/unit_tests/datasets/schema_tests.py b/tests/unit_tests/datasets/schema_tests.py
index 4c8d8cb456664..8aa327f77a0cc 100644
--- a/tests/unit_tests/datasets/schema_tests.py
+++ b/tests/unit_tests/datasets/schema_tests.py
@@ -26,10 +26,8 @@
 @pytest.mark.parametrize(
     "payload",
     [
-        None,
         "epoch_ms",
         "epoch_s",
-        "%Y/%m/%dT%H:%M:%S.%f",
         "%Y-%m-%dT%H:%M:%S.%f",
         "%Y%m%d",
     ],
@@ -42,6 +40,7 @@ def test_validate_python_date_format(payload) -> None:
     "payload",
     [
         "%d%m%Y",
+        "%Y/%m/%dT%H:%M:%S.%f",
     ],
 )
 def test_validate_python_date_format_raises(payload) -> None:

From baae852896c2a132b4e9c9771e6437faab169cab Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Sat, 18 Nov 2023 15:11:22 +0800
Subject: [PATCH 09/19] Per the type hint value cannot be None

---
 superset/datasets/schemas.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py
index 37d3686519237..1e9767b773600 100644
--- a/superset/datasets/schemas.py
+++ b/superset/datasets/schemas.py
@@ -44,7 +44,7 @@
 
 
 def validate_python_date_format(value: str) -> bool:
-    if value in ("epoch_s", "epoch_ms", None):
+    if value in ("epoch_s", "epoch_ms"):
         return True
     try:
         dt_str = datetime.now().strftime(value)

From 0f320a873abe8dab85cb748499fb286d9acdbdb4 Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Mon, 22 Jan 2024 18:27:30 +0800
Subject: [PATCH 10/19] Add the low-level consistency.

---
 superset/commands/dataset/exceptions.py | 12 ++++++++++++
 superset/commands/dataset/update.py     |  8 ++++++++
 superset/connectors/sqla/models.py      | 12 ++++++++++++
 superset/daos/dataset.py                | 13 +++++++++++++
 superset/datasets/schemas.py            |  6 +++---
 5 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/superset/commands/dataset/exceptions.py b/superset/commands/dataset/exceptions.py
index 4b8acaca08b8d..0f01824a3a9b7 100644
--- a/superset/commands/dataset/exceptions.py
+++ b/superset/commands/dataset/exceptions.py
@@ -90,6 +90,18 @@ def __init__(self) -> None:
         super().__init__([_("One or more columns already exist")], field_name="columns")
 
 
+class DatasetColumnsConstraintsValidationError(ValidationError):
+    """
+    Marshmallow validation error when dataset columns have an invalid python_date_format
+    """
+
+    def __init__(self) -> None:
+        super().__init__(
+            [_("One or more columns have an invalid python_date_format")],
+            field_name="columns",
+        )
+
+
 class DatasetMetricsNotFoundValidationError(ValidationError):
     """
     Marshmallow validation error when dataset metric for update does not exist
diff --git a/superset/commands/dataset/update.py b/superset/commands/dataset/update.py
index 8a72c24fd5f28..300506fdfbf4b 100644
--- a/superset/commands/dataset/update.py
+++ b/superset/commands/dataset/update.py
@@ -26,6 +26,7 @@
 from superset.commands.dataset.exceptions import (
     DatabaseChangeValidationError,
     DatasetColumnNotFoundValidationError,
+    DatasetColumnsConstraintsValidationError,
     DatasetColumnsDuplicateValidationError,
     DatasetColumnsExistsValidationError,
     DatasetExistsValidationError,
@@ -139,6 +140,13 @@ def _validate_columns(
                 ):
                     exceptions.append(DatasetColumnsExistsValidationError())
 
+            # validate python_date_format is ISO8601 format
+            for col in columns:
+                if not DatasetDAO.validate_column_pdf_is_iso8601(
+                    col["python_date_format"]
+                ):
+                    exceptions.append(DatasetColumnsConstraintsValidationError())
+
     def _validate_metrics(
         self, metrics: list[dict[str, Any]], exceptions: list[ValidationError]
     ) -> None:
diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index 624eb2ce5a530..e236cffcee98c 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -66,6 +66,7 @@
     relationship,
     RelationshipProperty,
     Session,
+    validates,
 )
 from sqlalchemy.orm.mapper import Mapper
 from sqlalchemy.schema import UniqueConstraint
@@ -830,6 +831,17 @@ def init_on_load(self) -> None:
     def __repr__(self) -> str:
         return str(self.column_name)
 
+    @validates("python_date_format")
+    def validate_pdf_is_iso8601(self, _: str, dt_format: str) -> str:
+        if dt_format in ("epoch_s", "epoch_ms"):
+            return dt_format
+        try:
+            dt_str = datetime.now().strftime(dt_format)
+            dateutil.parser.isoparse(dt_str)
+        except ValueError as ex:
+            raise ValueError("python_date_format is invalid ISO 8601 format") from ex
+        return dt_format
+
     @property
     def is_boolean(self) -> bool:
         """
diff --git a/superset/daos/dataset.py b/superset/daos/dataset.py
index 0a4425dbd7a66..ed99e91aecb8b 100644
--- a/superset/daos/dataset.py
+++ b/superset/daos/dataset.py
@@ -17,8 +17,10 @@
 from __future__ import annotations
 
 import logging
+from datetime import datetime
 from typing import Any
 
+import dateutil.parser
 from sqlalchemy.exc import SQLAlchemyError
 
 from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn
@@ -150,6 +152,17 @@ def validate_metrics_uniqueness(dataset_id: int, metrics_names: list[str]) -> bo
         ).all()
         return len(dataset_query) == 0
 
+    @staticmethod
+    def validate_column_pdf_is_iso8601(dt_format: str) -> bool:
+        if dt_format in ("epoch_s", "epoch_ms", None):
+            return True
+        try:
+            dt_str = datetime.now().strftime(dt_format)
+            dateutil.parser.isoparse(dt_str)
+            return True
+        except ValueError:
+            return False
+
     @classmethod
     def update(
         cls,
diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py
index 0ce47f4761d17..45a44043a5d23 100644
--- a/superset/datasets/schemas.py
+++ b/superset/datasets/schemas.py
@@ -44,11 +44,11 @@
 }
 
 
-def validate_python_date_format(value: str) -> bool:
-    if value in ("epoch_s", "epoch_ms"):
+def validate_python_date_format(dt_format: str) -> bool:
+    if dt_format in ("epoch_s", "epoch_ms"):
         return True
     try:
-        dt_str = datetime.now().strftime(value)
+        dt_str = datetime.now().strftime(dt_format)
         isoparse(dt_str)
     except ValueError as ex:
         raise ValidationError([_("Invalid date/timestamp format")]) from ex

From 6f697c509438625ec8984d523bbd6fb64e476820 Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Mon, 22 Jan 2024 22:37:40 +0800
Subject: [PATCH 11/19] fix python_date_time maybe none

---
 superset/connectors/sqla/models.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index e236cffcee98c..4561cbf7ce298 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -833,7 +833,7 @@ def __repr__(self) -> str:
 
     @validates("python_date_format")
     def validate_pdf_is_iso8601(self, _: str, dt_format: str) -> str:
-        if dt_format in ("epoch_s", "epoch_ms"):
+        if dt_format in ("epoch_s", "epoch_ms", None):
             return dt_format
         try:
             dt_str = datetime.now().strftime(dt_format)

From cefc744fe3fb31cfb7c2384567d399cffb006399 Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Mon, 22 Jan 2024 23:20:09 +0800
Subject: [PATCH 12/19] fix python_date_format maybe none

---
 superset/commands/dataset/update.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/superset/commands/dataset/update.py b/superset/commands/dataset/update.py
index 300506fdfbf4b..458d17d3d8555 100644
--- a/superset/commands/dataset/update.py
+++ b/superset/commands/dataset/update.py
@@ -143,7 +143,7 @@ def _validate_columns(
             # validate python_date_format is ISO8601 format
             for col in columns:
                 if not DatasetDAO.validate_column_pdf_is_iso8601(
-                    col["python_date_format"]
+                    col.get("python_date_format", None)
                 ):
                     exceptions.append(DatasetColumnsConstraintsValidationError())
 

From e6f8e2674e9d6423910892b05185f9d3b24b24b7 Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Mon, 22 Jan 2024 23:34:03 +0800
Subject: [PATCH 13/19] remove unnecessary test case

---
 tests/unit_tests/config_test.py | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/tests/unit_tests/config_test.py b/tests/unit_tests/config_test.py
index d28c3ecf74d77..a69d9eaede25c 100644
--- a/tests/unit_tests/config_test.py
+++ b/tests/unit_tests/config_test.py
@@ -211,7 +211,6 @@ def test_python_date_format_by_column_name(
         "dttm_columns": {
             "id": {"python_date_format": "epoch_ms"},
             "dttm": {"python_date_format": "epoch_s"},
-            "duration_ms": {"python_date_format": "invalid"},
         },
     }
     mocker.patch(
@@ -228,7 +227,6 @@ def test_python_date_format_by_column_name(
         return_value=[
             {"column_name": "id", "type": "INTEGER", "is_dttm": False},
             {"column_name": "dttm", "type": "INTEGER", "is_dttm": False},
-            {"column_name": "duration_ms", "type": "INTEGER", "is_dttm": False},
         ],
     )
 
@@ -242,12 +240,6 @@ def test_python_date_format_by_column_name(
     assert dttm_col.is_dttm
     assert dttm_col.python_date_format == "epoch_s"
 
-    duration_ms_col = [c for c in test_table.columns if c.column_name == "duration_ms"][
-        0
-    ]
-    assert duration_ms_col.is_dttm
-    assert duration_ms_col.python_date_format == "invalid"
-
 
 def test_expression_by_column_name(
     mocker: MockerFixture,

From 7f3055ab6188271c9609cf218f2d96f0b3e7d153 Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Mon, 22 Jan 2024 23:48:38 +0800
Subject: [PATCH 14/19] Updating UPDATING.md

---
 UPDATING.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/UPDATING.md b/UPDATING.md
index 84893e9080d2e..3fd73d8e669e6 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -40,6 +40,7 @@ assists people when migrating to a new version.
 - [26636](https://github.com/apache/superset/issues/26636): Sets the `DASHBOARD_VIRTUALIZATION` feature flag to `True` by default. This feature was introduced by [21438](https://github.com/apache/superset/pull/21438) and will enable virtualization when rendering a dashboard's charts in an attempt to reduce the number of elements (DOM nodes) rendered at once. This is especially useful for large dashboards.
 - [26637](https://github.com/apache/superset/issues/26637): Sets the `DRILL_BY` feature flag to `True` by default given that the feature has been tested for a while and reached a stable state.
 - [26462](https://github.com/apache/superset/issues/26462): Removes the Profile feature given that it's not actively maintained and not widely used.
+- [25510](https://github.com/apache/superset/pull/25510): (re)Restriction that only ISO 8601 formats are acceptable (enforced by way of validation at the API and database level) and that dataset owners will need to use a SQL expression instead to convert their string columns of the form %Y/%m/%d etc. to a DATE, DATETIME, etc. type.
 
 ### Potential Downtime
 

From 20e484e66f19e2b9e309455c2f44e8116c9bda86 Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Tue, 23 Jan 2024 18:08:49 +0800
Subject: [PATCH 15/19] refactor: validation at the DAOs level

---
 superset/commands/dataset/exceptions.py | 12 ------------
 superset/commands/dataset/update.py     |  8 --------
 superset/daos/dataset.py                | 17 +++++++++++++++--
 tests/unit_tests/config_test.py         |  8 ++++++++
 4 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/superset/commands/dataset/exceptions.py b/superset/commands/dataset/exceptions.py
index 0f01824a3a9b7..4b8acaca08b8d 100644
--- a/superset/commands/dataset/exceptions.py
+++ b/superset/commands/dataset/exceptions.py
@@ -90,18 +90,6 @@ def __init__(self) -> None:
         super().__init__([_("One or more columns already exist")], field_name="columns")
 
 
-class DatasetColumnsConstraintsValidationError(ValidationError):
-    """
-    Marshmallow validation error when dataset columns have an invalid python_date_format
-    """
-
-    def __init__(self) -> None:
-        super().__init__(
-            [_("One or more columns have an invalid python_date_format")],
-            field_name="columns",
-        )
-
-
 class DatasetMetricsNotFoundValidationError(ValidationError):
     """
     Marshmallow validation error when dataset metric for update does not exist
diff --git a/superset/commands/dataset/update.py b/superset/commands/dataset/update.py
index 458d17d3d8555..8a72c24fd5f28 100644
--- a/superset/commands/dataset/update.py
+++ b/superset/commands/dataset/update.py
@@ -26,7 +26,6 @@
 from superset.commands.dataset.exceptions import (
     DatabaseChangeValidationError,
     DatasetColumnNotFoundValidationError,
-    DatasetColumnsConstraintsValidationError,
     DatasetColumnsDuplicateValidationError,
     DatasetColumnsExistsValidationError,
     DatasetExistsValidationError,
@@ -140,13 +139,6 @@ def _validate_columns(
                 ):
                     exceptions.append(DatasetColumnsExistsValidationError())
 
-            # validate python_date_format is ISO8601 format
-            for col in columns:
-                if not DatasetDAO.validate_column_pdf_is_iso8601(
-                    col.get("python_date_format", None)
-                ):
-                    exceptions.append(DatasetColumnsConstraintsValidationError())
-
     def _validate_metrics(
         self, metrics: list[dict[str, Any]], exceptions: list[ValidationError]
     ) -> None:
diff --git a/superset/daos/dataset.py b/superset/daos/dataset.py
index ed99e91aecb8b..a513e21ecaef8 100644
--- a/superset/daos/dataset.py
+++ b/superset/daos/dataset.py
@@ -25,6 +25,7 @@
 
 from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn
 from superset.daos.base import BaseDAO
+from superset.daos.exceptions import DAOUpdateFailedError
 from superset.extensions import db
 from superset.models.core import Database
 from superset.models.dashboard import Dashboard
@@ -153,8 +154,8 @@ def validate_metrics_uniqueness(dataset_id: int, metrics_names: list[str]) -> bo
         return len(dataset_query) == 0
 
     @staticmethod
-    def validate_column_pdf_is_iso8601(dt_format: str) -> bool:
-        if dt_format in ("epoch_s", "epoch_ms", None):
+    def validate_column_python_date_format(dt_format: str) -> bool:
+        if dt_format in ("epoch_s", "epoch_ms"):
             return True
         try:
             dt_str = datetime.now().strftime(dt_format)
@@ -206,6 +207,18 @@ def update_columns(
         then we delete.
         """
 
+        for column in property_columns:
+            if (
+                "python_date_format" in column
+                and column["python_date_format"] is not None
+            ):
+                if not DatasetDAO.validate_column_python_date_format(
+                    column["python_date_format"]
+                ):
+                    raise DAOUpdateFailedError(
+                        "python_date_format is an invalid date/timestamp format."
+                    )
+
         if override_columns:
             db.session.query(TableColumn).filter(
                 TableColumn.table_id == model.id
diff --git a/tests/unit_tests/config_test.py b/tests/unit_tests/config_test.py
index a69d9eaede25c..d28c3ecf74d77 100644
--- a/tests/unit_tests/config_test.py
+++ b/tests/unit_tests/config_test.py
@@ -211,6 +211,7 @@ def test_python_date_format_by_column_name(
         "dttm_columns": {
             "id": {"python_date_format": "epoch_ms"},
             "dttm": {"python_date_format": "epoch_s"},
+            "duration_ms": {"python_date_format": "invalid"},
         },
     }
     mocker.patch(
@@ -227,6 +228,7 @@ def test_python_date_format_by_column_name(
         return_value=[
             {"column_name": "id", "type": "INTEGER", "is_dttm": False},
             {"column_name": "dttm", "type": "INTEGER", "is_dttm": False},
+            {"column_name": "duration_ms", "type": "INTEGER", "is_dttm": False},
         ],
     )
 
@@ -240,6 +242,12 @@ def test_python_date_format_by_column_name(
     assert dttm_col.is_dttm
     assert dttm_col.python_date_format == "epoch_s"
 
+    duration_ms_col = [c for c in test_table.columns if c.column_name == "duration_ms"][
+        0
+    ]
+    assert duration_ms_col.is_dttm
+    assert duration_ms_col.python_date_format == "invalid"
+
 
 def test_expression_by_column_name(
     mocker: MockerFixture,

From 6aee27c408c5624397d6533c637e0314a4d74e02 Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Tue, 23 Jan 2024 18:11:42 +0800
Subject: [PATCH 16/19] refactor: validation at the DAOs level

---
 superset/connectors/sqla/models.py | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index 4561cbf7ce298..624eb2ce5a530 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -66,7 +66,6 @@
     relationship,
     RelationshipProperty,
     Session,
-    validates,
 )
 from sqlalchemy.orm.mapper import Mapper
 from sqlalchemy.schema import UniqueConstraint
@@ -831,17 +830,6 @@ def init_on_load(self) -> None:
     def __repr__(self) -> str:
         return str(self.column_name)
 
-    @validates("python_date_format")
-    def validate_pdf_is_iso8601(self, _: str, dt_format: str) -> str:
-        if dt_format in ("epoch_s", "epoch_ms", None):
-            return dt_format
-        try:
-            dt_str = datetime.now().strftime(dt_format)
-            dateutil.parser.isoparse(dt_str)
-        except ValueError as ex:
-            raise ValueError("python_date_format is invalid ISO 8601 format") from ex
-        return dt_format
-
     @property
     def is_boolean(self) -> bool:
         """

From 0269e95f8ead75d769a2213516723bc309d7a845 Mon Sep 17 00:00:00 2001
From: John Bodley <john.bodley@gmail.com>
Date: Fri, 17 Nov 2023 08:55:49 -0800
Subject: [PATCH 17/19] fix(dataset): Ensure DATETIME FORMAT is ISO 8601
 compliant

---
 .../src/components/Datasource/DatasourceEditor.jsx              | 2 +-
 .../src/components/Datasource/DatasourceEditor.test.jsx         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/superset-frontend/src/components/Datasource/DatasourceEditor.jsx b/superset-frontend/src/components/Datasource/DatasourceEditor.jsx
index e4a4d0116a098..2bc92d7c0c1dd 100644
--- a/superset-frontend/src/components/Datasource/DatasourceEditor.jsx
+++ b/superset-frontend/src/components/Datasource/DatasourceEditor.jsx
@@ -329,7 +329,7 @@ function ColumnCollectionTable({
               control={
                 <TextControl
                   controlId="python_date_format"
-                  placeholder="%Y/%m/%d"
+                  placeholder="%Y-%m-%d"
                 />
               }
             />
diff --git a/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx b/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx
index 9a3d8455808c9..1221d8bf4f799 100644
--- a/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx
+++ b/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx
@@ -96,7 +96,7 @@ describe('DatasourceEditor', () => {
 
     const inputLabel = screen.getByPlaceholderText('Label');
     const inputDescription = screen.getByPlaceholderText('Description');
-    const inputDtmFormat = screen.getByPlaceholderText('%Y/%m/%d');
+    const inputDtmFormat = screen.getByPlaceholderText('%Y-%m-%d');
     const inputCertifiedBy = screen.getByPlaceholderText('Certified by');
     const inputCertDetails = screen.getByPlaceholderText(
       'Certification details',

From 94b148286e74b15aa1bfc1211b440222079e095e Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Tue, 23 Jan 2024 18:21:50 +0800
Subject: [PATCH 18/19] remove unnecessary test case

---
 tests/unit_tests/config_test.py | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/tests/unit_tests/config_test.py b/tests/unit_tests/config_test.py
index d28c3ecf74d77..a69d9eaede25c 100644
--- a/tests/unit_tests/config_test.py
+++ b/tests/unit_tests/config_test.py
@@ -211,7 +211,6 @@ def test_python_date_format_by_column_name(
         "dttm_columns": {
             "id": {"python_date_format": "epoch_ms"},
             "dttm": {"python_date_format": "epoch_s"},
-            "duration_ms": {"python_date_format": "invalid"},
         },
     }
     mocker.patch(
@@ -228,7 +227,6 @@ def test_python_date_format_by_column_name(
         return_value=[
             {"column_name": "id", "type": "INTEGER", "is_dttm": False},
             {"column_name": "dttm", "type": "INTEGER", "is_dttm": False},
-            {"column_name": "duration_ms", "type": "INTEGER", "is_dttm": False},
         ],
     )
 
@@ -242,12 +240,6 @@ def test_python_date_format_by_column_name(
     assert dttm_col.is_dttm
     assert dttm_col.python_date_format == "epoch_s"
 
-    duration_ms_col = [c for c in test_table.columns if c.column_name == "duration_ms"][
-        0
-    ]
-    assert duration_ms_col.is_dttm
-    assert duration_ms_col.python_date_format == "invalid"
-
 
 def test_expression_by_column_name(
     mocker: MockerFixture,

From b5ca46816b07b0914f8b02d52502938afe9bf49b Mon Sep 17 00:00:00 2001
From: mapledan <mapledan829@gmail.com>
Date: Tue, 23 Jan 2024 18:32:37 +0800
Subject: [PATCH 19/19] pylint: fix invalid-name

---
 superset/daos/dataset.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/superset/daos/dataset.py b/superset/daos/dataset.py
index a513e21ecaef8..4647e02ce68af 100644
--- a/superset/daos/dataset.py
+++ b/superset/daos/dataset.py
@@ -154,7 +154,7 @@ def validate_metrics_uniqueness(dataset_id: int, metrics_names: list[str]) -> bo
         return len(dataset_query) == 0
 
     @staticmethod
-    def validate_column_python_date_format(dt_format: str) -> bool:
+    def validate_python_date_format(dt_format: str) -> bool:
         if dt_format in ("epoch_s", "epoch_ms"):
             return True
         try:
@@ -212,7 +212,7 @@ def update_columns(
                 "python_date_format" in column
                 and column["python_date_format"] is not None
             ):
-                if not DatasetDAO.validate_column_python_date_format(
+                if not DatasetDAO.validate_python_date_format(
                     column["python_date_format"]
                 ):
                     raise DAOUpdateFailedError(