[schema] Deprecating the table_columns.database_expression column (#7653

)
apache · Jul 3, 2019 · 9dac805 · 9dac805
1 parent 346638a
commit 9dac805
Show file tree

Hide file tree

Showing 7 changed files with 49 additions and 60 deletions.
diff --git a/UPDATING.md b/UPDATING.md
@@ -27,6 +27,11 @@ assists people when migrating to a new version.
 make all Unix timestamp (which by definition are in UTC) comparisons refer
 to a timestamp in UTC as opposed to local time.
 
+* [7653](https://github.com/apache/incubator-superset/pull/7653): a change
+which deprecates the table_columns.database_expression column. Expressions
+should be handled by the DB engine spec conversion, Python date format, or
+custom column expression/type.
+
 * [5451](https://github.com/apache/incubator-superset/pull/5451): a change
 which adds missing non-nullable fields to the `datasources` table. Depending on
 the integrity of the data, manual intervention may be required.

diff --git a/superset/assets/src/datasource/DatasourceEditor.jsx b/superset/assets/src/datasource/DatasourceEditor.jsx
@@ -104,28 +104,7 @@ function ColumnCollectionTable({
                     {t('python datetime string pattern')}
                   </a>
                   {t(` expression. If time is stored in epoch format, put \`epoch_s\` or
-                      \`epoch_ms\`. Leave \`Database Expression\`
-                      below empty if timestamp is stored in '
-                      String or Integer(epoch) type`)}
-                </div>
-              }
-              control={<TextControl />}
-            />
-            <Field
-              fieldKey="database_expression"
-              label={t('Database Expression')}
-              descr={
-                <div>
-                  {t(`
-                    The database expression to cast internal datetime
-                    constants to database date/timestamp type according to the DBAPI.
-                    The expression should follow the pattern of
-                    %Y-%m-%d %H:%M:%S, based on different DBAPI.
-                    The string should be a python string formatter
-                    \`Ex: TO_DATE('{}', 'YYYY-MM-DD HH24:MI:SS')\` for Oracle
-                    Superset uses default expression based on DB URI if this
-                    field is blank.
-                  `)}
+                      \`epoch_ms\`.`)}
                 </div>
               }
               control={<TextControl />}

diff --git a/superset/connectors/base/models.py b/superset/connectors/base/models.py
@@ -394,7 +394,6 @@ def data(self):
             "groupby",
             "is_dttm",
             "type",
-            "database_expression",
             "python_date_format",
         )
         return {s: getattr(self, s) for s in attrs if hasattr(self, s)}

diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
@@ -112,7 +112,6 @@ class TableColumn(Model, BaseColumn):
     is_dttm = Column(Boolean, default=False)
     expression = Column(Text)
     python_date_format = Column(String(255))
-    database_expression = Column(String(255))
 
     export_fields = (
         "table_id",
@@ -126,7 +125,6 @@ class TableColumn(Model, BaseColumn):
         "expression",
         "description",
         "python_date_format",
-        "database_expression",
     )
 
     update_from_object_fields = [s for s in export_fields if s not in ("table_id",)]
@@ -195,18 +193,9 @@ def lookup_obj(lookup_column):
         return import_datasource.import_simple_obj(db.session, i_column, lookup_obj)
 
     def dttm_sql_literal(self, dttm):
-        """Convert datetime object to a SQL expression string
-
-        If database_expression is empty, the internal dttm
-        will be parsed as the string with the pattern that
-        the user inputted (python_date_format)
-        If database_expression is not empty, the internal dttm
-        will be parsed as the sql sentence for the database to convert
-        """
+        """Convert datetime object to a SQL expression string"""
         tf = self.python_date_format
-        if self.database_expression:
-            return self.database_expression.format(dttm.strftime("%Y-%m-%d %H:%M:%S"))
-        elif tf:
+        if tf:
             seconds_since_epoch = int(dttm.timestamp())
             if tf == "epoch_s":
                 return str(seconds_since_epoch)

diff --git a/superset/connectors/sqla/views.py b/superset/connectors/sqla/views.py
@@ -65,7 +65,6 @@ class TableColumnInlineView(CompactCRUDMixin, SupersetModelView):  # noqa
         "expression",
         "is_dttm",
         "python_date_format",
-        "database_expression",
     ]
     add_columns = edit_columns
     list_columns = [
@@ -105,23 +104,10 @@ class TableColumnInlineView(CompactCRUDMixin, SupersetModelView):  # noqa
                 'datetime.html#strftime-strptime-behavior">'
                 "python datetime string pattern</a> "
                 "expression. If time is stored in epoch "
-                "format, put `epoch_s` or `epoch_ms`. Leave `Database Expression` "
-                "below empty if timestamp is stored in "
-                "String or Integer(epoch) type"
+                "format, put `epoch_s` or `epoch_ms`."
             ),
             True,
         ),
-        "database_expression": utils.markdown(
-            "The database expression to cast internal datetime "
-            "constants to database date/timestamp type according to the DBAPI. "
-            "The expression should follow the pattern of "
-            "%Y-%m-%d %H:%M:%S, based on different DBAPI. "
-            "The string should be a python string formatter \n"
-            "`Ex: TO_DATE('{}', 'YYYY-MM-DD HH24:MI:SS')` for Oracle "
-            "Superset uses default expression based on DB URI if this "
-            "field is blank.",
-            True,
-        ),
     }
     label_columns = {
         "column_name": _("Column"),
@@ -133,7 +119,6 @@ class TableColumnInlineView(CompactCRUDMixin, SupersetModelView):  # noqa
         "expression": _("Expression"),
         "is_dttm": _("Is temporal"),
         "python_date_format": _("Datetime Format"),
-        "database_expression": _("Database Expression"),
         "type": _("Type"),
     }
 

diff --git a/superset/migrations/versions/b4a38aa87893_deprecate_database_expression.py b/superset/migrations/versions/b4a38aa87893_deprecate_database_expression.py
@@ -0,0 +1,40 @@
+# 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.
+"""deprecate database expression
+
+Revision ID: b4a38aa87893
+Revises: ab8c66efdd01
+Create Date: 2019-06-05 11:35:16.222519
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "b4a38aa87893"
+down_revision = "ab8c66efdd01"
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    with op.batch_alter_table("table_columns") as batch_op:
+        batch_op.drop_column("database_expression")
+
+
+def downgrade():
+    with op.batch_alter_table("table_columns") as batch_op:
+        batch_op.add_column(sa.Column("database_expression", sa.String(255)))
diff --git a/tests/fixtures/datasource.py b/tests/fixtures/datasource.py
@@ -39,7 +39,6 @@
             "groupby": True,
             "is_dttm": True,
             "type": "DATETIME",
-            "database_expression": "",
         },
         {
             "id": 505,
@@ -51,7 +50,6 @@
             "groupby": True,
             "is_dttm": False,
             "type": "VARCHAR(16)",
-            "database_expression": None,
         },
         {
             "id": 506,
@@ -63,7 +61,6 @@
             "groupby": True,
             "is_dttm": None,
             "type": "VARCHAR(255)",
-            "database_expression": None,
         },
         {
             "id": 508,
@@ -75,7 +72,6 @@
             "groupby": True,
             "is_dttm": None,
             "type": "VARCHAR(10)",
-            "database_expression": None,
         },
         {
             "id": 509,
@@ -87,7 +83,6 @@
             "groupby": True,
             "is_dttm": None,
             "type": "BIGINT(20)",
-            "database_expression": None,
         },
         {
             "id": 510,
@@ -99,7 +94,6 @@
             "groupby": False,
             "is_dttm": False,
             "type": "BIGINT(20)",
-            "database_expression": None,
         },
         {
             "id": 532,
@@ -111,7 +105,6 @@
             "groupby": True,
             "is_dttm": None,
             "type": "BIGINT(20)",
-            "database_expression": None,
         },
         {
             "id": 522,
@@ -123,7 +116,6 @@
             "groupby": False,
             "is_dttm": False,
             "type": "NUMBER",
-            "database_expression": None,
         },
     ],
     "metrics": [