Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[schema] Deprecating the table_columns.database_expression column #7653

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
23 changes: 1 addition & 22 deletions superset/assets/src/datasource/DatasourceEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 />}
Expand Down
1 change: 0 additions & 1 deletion superset/connectors/base/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Expand Down
15 changes: 2 additions & 13 deletions superset/connectors/sqla/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",)]
Expand Down Expand Up @@ -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)
Expand Down
17 changes: 1 addition & 16 deletions superset/connectors/sqla/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ class TableColumnInlineView(CompactCRUDMixin, SupersetModelView): # noqa
"expression",
"is_dttm",
"python_date_format",
"database_expression",
]
add_columns = edit_columns
list_columns = [
Expand Down Expand Up @@ -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"),
Expand All @@ -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"),
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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)))
8 changes: 0 additions & 8 deletions tests/fixtures/datasource.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
"groupby": True,
"is_dttm": True,
"type": "DATETIME",
"database_expression": "",
},
{
"id": 505,
Expand All @@ -51,7 +50,6 @@
"groupby": True,
"is_dttm": False,
"type": "VARCHAR(16)",
"database_expression": None,
},
{
"id": 506,
Expand All @@ -63,7 +61,6 @@
"groupby": True,
"is_dttm": None,
"type": "VARCHAR(255)",
"database_expression": None,
},
{
"id": 508,
Expand All @@ -75,7 +72,6 @@
"groupby": True,
"is_dttm": None,
"type": "VARCHAR(10)",
"database_expression": None,
},
{
"id": 509,
Expand All @@ -87,7 +83,6 @@
"groupby": True,
"is_dttm": None,
"type": "BIGINT(20)",
"database_expression": None,
},
{
"id": 510,
Expand All @@ -99,7 +94,6 @@
"groupby": False,
"is_dttm": False,
"type": "BIGINT(20)",
"database_expression": None,
},
{
"id": 532,
Expand All @@ -111,7 +105,6 @@
"groupby": True,
"is_dttm": None,
"type": "BIGINT(20)",
"database_expression": None,
},
{
"id": 522,
Expand All @@ -123,7 +116,6 @@
"groupby": False,
"is_dttm": False,
"type": "NUMBER",
"database_expression": None,
},
],
"metrics": [
Expand Down