[GitHub] john-bodley commented on a change in pull request #5449: [schema] Updating the tables schema

[GitBox <gi...@apache.org>]

john-bodley commented on a change in pull request #5449: [schema] Updating the tables schema
URL: https://github.com/apache/incubator-superset/pull/5449#discussion_r250893098
 
 

 ##########
 File path: superset/migrations/versions/1d73b15530e7_update_tables.py
 ##########
 @@ -0,0 +1,93 @@
+"""update tables
+
+Revision ID: 1d73b15530e7
+Revises: 1d9e835a84f9
+Create Date: 2018-07-20 11:36:04.535859
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '1d73b15530e7'
+down_revision = '1d9e835a84f9'
+
+from alembic import op
+import sqlalchemy as sa
+
+from superset.utils import generic_find_uq_constraint_name
+
+
+conv = {
+    'uq': 'uq_%(table_name)s_%(column_0_name)s',
+}
+
+tables = sa.Table(
+    'tables',
+    sa.MetaData(),
+    sa.Column('id', sa.Integer, primary_key=True),
+    sa.Column('database_id', sa.Integer),
+    sa.Column('schema', sa.String(127)),
+    sa.Column('table_name', sa.String(127), nullable=False),
+)
+
+
+def upgrade():
+    bind = op.get_bind()
+    insp = sa.engine.reflection.Inspector.from_engine(bind)
+
+    # Reduce the size of the schema and table_name columns for constraint
+    # viability. Note most engines have a 64 character limit for these fields.
+    with op.batch_alter_table('tables') as batch_op:
+        batch_op.alter_column(
+            'schema',
+            existing_nullable=True,
+            existing_type=sa.String(255),
+            type_=sa.String(127),
+        )
+
+        # Additionally enforce that the table_name column be non-nullable.
+        batch_op.alter_column(
+            'table_name',
+            existing_type=sa.String(250),
+            nullable=False,
+            type_=sa.String(127),
+        )
+
+    # Add the missing uniqueness constraint.
+    with op.batch_alter_table('tables', naming_convention=conv) as batch_op:
+        batch_op.create_unique_constraint(
+            'uq_tables_database_id',
+            ['database_id', 'schema', 'table_name'],
+        )
+
+
+def downgrade():
+    bind = op.get_bind()
+    insp = sa.engine.reflection.Inspector.from_engine(bind)
+
+    # Drop the missing uniqueness constraint.
+    with op.batch_alter_table('tables', naming_convention=conv) as batch_op:
+        batch_op.drop_constraint(
+            generic_find_uq_constraint_name(
+                'tables',
+                {'database_id', 'schema', 'table_name'},
 
 Review comment:
   I think the issue here is that if there were two different views of the same table how would one differentiate between them as datasources. Note in a database construct view names need to be unique.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org