[schema] Updating the base column schema (#5452)

UPDATING.md

@@ -27,6 +27,11 @@ assists people when migrating to a new version.
 which adds missing non-nullable fields to the `datasources` table. Depending on
 the integrity of the data, manual intervention may be required.
 
+* [5452](https://github.com/apache/incubator-superset/pull/5452): a change
+which adds missing non-nullable fields and uniqueness constraints to the
+`columns`and `table_columns` tables. Depending on the integrity of the data,
+manual intervention may be required.
+
 ## Superset 0.32.0
 
 * `npm run backend-sync` is deprecated and no longer needed, will fail if called

superset/connectors/base/models.py

@@ -347,7 +347,7 @@ class BaseColumn(AuditMixinNullable, ImportMixin):
     __tablename__ = None  # {connector_name}_column
 
     id = Column(Integer, primary_key=True)
-    column_name = Column(String(255))
+    column_name = Column(String(255), nullable=False)
     verbose_name = Column(String(1024))
     is_active = Column(Boolean, default=True)
     type = Column(String(32))

superset/connectors/druid/models.py

@@ -269,9 +269,7 @@ class DruidColumn(Model, BaseColumn):
     __tablename__ = 'columns'
     __table_args__ = (UniqueConstraint('column_name', 'datasource_id'),)
 
-    datasource_id = Column(
-        Integer,
-        ForeignKey('datasources.id'))
+    datasource_id = Column(Integer, ForeignKey('datasources.id'))
     # Setting enable_typechecks=False disables polymorphic inheritance.
     datasource = relationship(
         'DruidDatasource',

superset/connectors/sqla/models.py

@@ -100,7 +100,7 @@ class TableColumn(Model, BaseColumn):
         backref=backref('columns', cascade='all, delete-orphan'),
         foreign_keys=[table_id])
     is_dttm = Column(Boolean, default=False)
-    expression = Column(Text, default='')
+    expression = Column(Text)
     python_date_format = Column(String(255))
     database_expression = Column(String(255))
 

superset/migrations/versions/7f2635b51f5d_update_base_columns.py

@@ -0,0 +1,133 @@
+# 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.
+"""update base columns
+
+Note that the columns table was previously partially modifed by revision
+f231d82b9b26.
+
+Revision ID: 7f2635b51f5d
+Revises: 937d04c16b64
+Create Date: 2018-07-20 15:31:05.058050
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '7f2635b51f5d'
+down_revision = '937d04c16b64'
+
+from alembic import op
+from sqlalchemy import Column, engine, ForeignKey, Integer, String
+from sqlalchemy.ext.declarative import declarative_base
+
+from superset import db
+from superset.utils.core import generic_find_uq_constraint_name
+
+Base = declarative_base()
+
+conv = {
+    'uq': 'uq_%(table_name)s_%(column_0_name)s',
+}
+
+
+class BaseColumnMixin(object):
+    id = Column(Integer, primary_key=True)
+
+
+class DruidColumn(BaseColumnMixin, Base):
+    __tablename__ = 'columns'
+
+    datasource_id = Column(Integer, ForeignKey('datasources.id'))
+
+
+class TableColumn(BaseColumnMixin, Base):
+    __tablename__ = 'table_columns'
+
+    table_id = Column(Integer, ForeignKey('tables.id'))
+
+
+def upgrade():
+    bind = op.get_bind()
+    session = db.Session(bind=bind)
+
+    # Delete the orphaned columns records.
+    for record in session.query(DruidColumn).all():
+        if record.datasource_id is None:
+            session.delete(record)
+
+    # Enforce that the columns.column_name be non-nullable.
+    with op.batch_alter_table('columns') as batch_op:
+        batch_op.alter_column(
+            'column_name',
+            existing_type=String(255),
+            nullable=False,
+        )
+
+    # Delete the orphaned table_columns records.
+    for record in session.query(TableColumn).all():
+        if record.table_id is None:
+            session.delete(record)
+
+    # Reduce the size of the table_columns.column_name column for constraint
+    # viability and enforce that it be non-nullable.
+    with op.batch_alter_table('table_columns') as batch_op:
+        batch_op.alter_column(
+            'column_name',
+            existing_type=String(256),
+            nullable=False,
+            type_=String(255),
+        )
+
+    # Add the missing uniqueness constraint to the table_columns table.
+    with op.batch_alter_table('table_columns', naming_convention=conv) as batch_op:
+        batch_op.create_unique_constraint(
+            'uq_table_columns_column_name',
+            ['column_name', 'table_id'],
+        )
+
+
+def downgrade():
+    bind = op.get_bind()
+    insp = engine.reflection.Inspector.from_engine(bind)
+
+    # Remove the missing uniqueness constraint from the table_columns table.
+    with op.batch_alter_table('table_columns', naming_convention=conv) as batch_op:
+        batch_op.drop_constraint(
+            generic_find_uq_constraint_name(
+                'table_columns',
+                {'column_name', 'table_id'},
+                insp,
+            ) or 'uq_table_columns_column_name',
+            type_='unique',
+        )
+
+    # Restore the size of the table_columns.column_name column and forego that
+    # it be non-nullable.
+    with op.batch_alter_table('table_columns') as batch_op:
+        batch_op.alter_column(
+            'column_name',
+            existing_type=String(255),
+            nullable=True,
+            type_=String(256),
+        )
+
+    # Forego that the columns.column_name be non-nullable.
+    with op.batch_alter_table('columns') as batch_op:
+        batch_op.alter_column(
+            'column_name',
+            existing_type=String(255),
+            nullable=True,
+        )

tests/sqllab_tests.py

@@ -307,7 +307,7 @@ class SqlLabTests(SupersetTestCase):
             'columns': [{
                 'is_date': False,
                 'type': 'STRING',
-                'nam:qe': 'viz_type',
+                'name': 'viz_type',
                 'is_dim': True,
             }, {
                 'is_date': False,