[sqla] Enforcing ISO 8601 date/timestamp formats

apache · Oct 17, 2019 · 3438a06 · 3438a06
1 parent d3406e0
commit 3438a06
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 9 deletions.
diff --git a/superset/assets/src/datasource/DatasourceEditor.jsx b/superset/assets/src/datasource/DatasourceEditor.jsx
@@ -97,14 +97,23 @@ function ColumnCollectionTable({
             <Field
               fieldKey="python_date_format"
               label={t('Datetime Format')}
-              descr={
+              descr={/* Note the fragmented translations may not work. */
                 <div>
-                  {t('The pattern of the timestamp format, use ')}
+                  {t('The pattern of timestamp format. For strings use ')}
                   <a href="https://docs.python.org/2/library/datetime.html#strftime-strptime-behavior">
                     {t('python datetime string pattern')}
                   </a>
-                  {t(` expression. If time is stored in epoch format, put \`epoch_s\` or
-                      \`epoch_ms\`.`)}
+                  {t(' expression which needs to adhere to the ')}
+                  <a href="https://en.wikipedia.org/wiki/ISO_8601">
+                    {t('ISO 8601')}
+                  </a>
+                  {t(` standard to ensure that the lexicographical ordering
+                      coincides with the chronological ordering. If the
+                      timestamp format does not adhere to the ISO 8601 standard
+                      you will need to define an expression and type for
+                      transforming the string into a date or timestamp. Note
+                      currently time zones are not supported. If time is stored
+                      in epoch format, put \`epoch_s\` or \`epoch_ms\`.`)}
                 </div>
               }
               control={<TextControl />}

diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
@@ -219,6 +219,8 @@ def dttm_sql_literal(self, dttm: DateTime) -> str:
             return "'{}'".format(dttm.strftime(tf))
         else:
             s = self.table.database.db_engine_spec.convert_dttm(self.type or "", dttm)
+
+            # TODO(john-bodley): SIP-15 will explicitly require a type conversion.
             return s or "'{}'".format(dttm.strftime("%Y-%m-%d %H:%M:%S.%f"))
 
 

diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py
@@ -17,6 +17,7 @@
 # pylint: disable=C,R,W
 """Views used by the SqlAlchemy connector"""
 import logging
+import re
 
 from flask import flash, Markup, redirect
 from flask_appbuilder import CompactCRUDMixin, expose
@@ -27,6 +28,7 @@
 from flask_babel import gettext as __
 from flask_babel import lazy_gettext as _
 from wtforms.ext.sqlalchemy.fields import QuerySelectField
+from wtforms.validators import Regexp
 
 from superset import appbuilder, db, security_manager
 from superset.connectors.base.views import DatasourceModelView
@@ -99,12 +101,17 @@ class TableColumnInlineView(CompactCRUDMixin, SupersetModelView):  # noqa
         ),
         "python_date_format": utils.markdown(
             Markup(
-                "The pattern of timestamp format, use "
+                "The pattern of timestamp format. For strings use "
                 '<a href="https://docs.python.org/2/library/'
                 'datetime.html#strftime-strptime-behavior">'
-                "python datetime string pattern</a> "
-                "expression. If time is stored in epoch "
-                "format, put `epoch_s` or `epoch_ms`."
+                "python datetime string pattern</a> expression which needs to "
+                'adhere to the <a href="https://en.wikipedia.org/wiki/ISO_8601">'
+                "ISO 8601</a> standard to ensure that the lexicographical ordering "
+                "coincides with the chronological ordering. If the timestamp "
+                "format does not adhere to the ISO 8601 standard you will need to "
+                "define an expression and type for transforming the string into a "
+                "date or timestamp. Note currently time zones are not supported. "
+                "If time is stored in epoch format, put `epoch_s` or `epoch_ms`."
             ),
             True,
         ),
@@ -121,6 +128,24 @@ class TableColumnInlineView(CompactCRUDMixin, SupersetModelView):  # noqa
         "python_date_format": _("Datetime Format"),
         "type": _("Type"),
     }
+    validators_columns = {
+        "python_date_format": [
+            # Restrict viable values to epoch_s, epoch_ms, or a strftime format
+            # which adhere's to the ISO 8601 format (without time zone).
+            Regexp(
+                re.compile(
+                    r"""
+                    ^(
+                        epoch_s|epoch_ms|
+                        (?P<date>%Y(-%m(-%d)?)?)([\sT](?P<time>%H(:%M(:%S(\.%f)?)?)?))?
+                    )$
+                    """,
+                    re.VERBOSE,
+                ),
+                message=_("Invalid date/timestamp format"),
+            )
+        ]
+    }
 
     add_form_extra_fields = {
         "table": QuerySelectField(
@@ -303,7 +328,6 @@ class TableModelView(DatasourceModelView, DeleteMixin, YamlExportMixin):  # noqa
         "template_params": _("Template parameters"),
         "modified": _("Modified"),
     }
-
     edit_form_extra_fields = {
         "database": QuerySelectField(
             "Database",