sqlalchemy-bot / test_alembic_1

0 stars 0 forks source link

create_unique_constraint not working with naming conventions #452

Closed sqlalchemy-bot closed 7 years ago

sqlalchemy-bot commented 7 years ago

Migrated issue, originally created by Matt Jernigan (juanitogan)

v0.8.4

autogenerate creates this:

#!python
op.create_unique_constraint('form_name', 'forms', ['org_code', 'form_name'])
op.drop_constraint('uq_forms_form_name', 'forms', type_='unique')

instead of this:

#!python
op.drop_constraint('uq_forms_form_name', 'forms', type_='unique')
op.create_unique_constraint(op.f('uq_forms_form_name'), 'forms', ['org_code', 'form_name'])

Several problems here:

  1. Order: drop should be before create.
  2. Create should maybe use op.f()
  3. And, most notably, op.create_unique_constraint('form_name', ...) does not apply naming conventions (it creates a unique constraint called form_name in this case).
sqlalchemy-bot commented 7 years ago

Michael Bayer (zzzeek) wrote:

Hi there -

Unfortunately I have no idea what you are trying to do or how you are trying to do it. Alembic autogenerate does not detect changes in constraint names if that's what you're trying to do. Please provide minimal examples of the models in use both before and after the autogenerate operation.

sqlalchemy-bot commented 7 years ago

Michael Bayer (zzzeek) wrote:

So I tried to guess what you did, just a normal operation with a unique constraint being modified.

Start with env.py including this model:

#!python

target_metadata = metadata = MetaData(naming_convention={
        "uq": "uq_%(table_name)s_%(column_0_name)s"
    })

rev = 1

if rev == 1:
    Table('forms', metadata,
          Column('id', Integer, primary_key=True),
          Column('org_code', Text),
          Column('form_name', Text),
          UniqueConstraint('org_code')
          )

elif rev == 2:
    Table('forms', metadata,
          Column('id', Integer, primary_key=True),
          Column('org_code', Text),
          Column('form_name', Text),
          UniqueConstraint('org_code', 'form_name')
          )

generate rev 1 with autogenerate, comes out as:

#!python

def upgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.create_table('forms',
    sa.Column('id', sa.Integer(), nullable=False),
    sa.Column('org_code', sa.Text(), nullable=True),
    sa.Column('form_name', sa.Text(), nullable=True),
    sa.PrimaryKeyConstraint('id'),
    sa.UniqueConstraint('org_code', name=op.f('uq_forms_org_code'))
    )
    # ### end Alembic commands ###

def downgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.drop_table('forms')
    # ### end Alembic commands ###

bump "rev" to 2 in the above env.py - basically add a column to the unique constraint. run autogenerate. Here's the revision:

#!python

def upgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.drop_constraint(u'uq_forms_org_code', 'forms', type_='unique')
    op.create_unique_constraint(op.f('uq_forms_org_code'), 'forms', ['org_code', 'form_name'])
    # ### end Alembic commands ###

def downgrade():
    # ### commands auto generated by Alembic - please adjust! ###
    op.drop_constraint(op.f('uq_forms_org_code'), 'forms', type_='unique')
    op.create_unique_constraint(u'uq_forms_org_code', 'forms', ['org_code'])
    # ### end Alembic commands ###

Above, we can see:

  1. Order: drop is before create

  2. Create/drop do use op.f(), representing the constraint from the metadata. the constraint read from the database does not have it, that's an existing issue #264. With the possible exception of when the infamous "%(constraint_name)s" token is present, it doesn't matter much because whether or not the f() is present, the naming convention is skipped when a name is present. e.g. this migration works with or without the f().

  3. the naming conventions are always pre-applied in the case of autogenerate (hence the use of op.f()). This is so that even if you change your naming conventions in your code, your old migrations still use the names that existed at the time of those migrations.

So #264 exists but otherwise I can't reproduce any of the behaviors you refer towards.

sqlalchemy-bot commented 7 years ago

Matt Jernigan (juanitogan) wrote:

Okay, I guess I'm beginning to see where I went wrong. I went from an unnamed constraint to a named one. And, while some named constraints get the naming convention applied to them (like this check constraint did name="code_friendly" becomes ck_forms_code_friendly), I guess others don't (like the latter unique constraint name="form_name" stays at form_name). I find hidden and contradictory rules like this to be completely confusing but, if that's the way it's meant to be, so be it.

I went from this:

#!python
class Form(db.Model):
    __tablename__ = "forms"
    __table_args__ = (  CheckConstraint("code ~ '[A-Z._-]+'", name="code_friendly")
                    ,)

    code        = db.Column(db.String(10), primary_key=True)
    form_name   = db.Column(db.Unicode(50), unique=True, nullable=False)
    ...

to this:

#!python

class Form(db.Model):
    __tablename__ = "forms"
    __table_args__ = (  CheckConstraint("code ~ '^[A-Z0-9_-]+$'", name="code_friendly")
                    ,   ForeignKeyConstraint(
                            [   "org_code"
                            ,   "calendar_icon_code"
                            ]
                        ,   [   "form_images.org_code"
                            ,   "form_images.code"
                            ]
                        )
                    ,   UniqueConstraint("org_code", "form_name", name="form_name")
                    )

    org_code    = db.Column(db.String(10), db.ForeignKey("orgs.code"), primary_key=True)
    code        = db.Column(db.String(10), primary_key=True)
    form_name   = db.Column(db.Unicode(50), nullable=False)
    ...

and got this (which takes a while to untangle, rewrite, and put in an order that can be further modified with PK changes and loads to run without error -- which is the point suggestion of another message):

#!python
def upgrade():
    ### commands auto generated by Alembic - please adjust! ###
    op.create_table('orgs',
    sa.Column('code', sa.String(length=10), nullable=False),
    sa.Column('org_name', sa.Unicode(length=50), nullable=False),
    sa.Column('email', sa.Unicode(length=255), nullable=False),
    sa.Column('phone', sa.Unicode(length=20), nullable=False),
    sa.Column('created_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False),
    sa.Column('created_by', sa.Unicode(length=255), nullable=False),
    sa.Column('updated_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False),
    sa.Column('updated_by', sa.Unicode(length=255), nullable=False),
    sa.CheckConstraint("code ~ '^[a-z0-9-]+$'", name=op.f('ck_orgs_domain_friendly')),
    sa.PrimaryKeyConstraint('code', name=op.f('pk_orgs')),
    sa.UniqueConstraint('org_name', name=op.f('uq_orgs_org_name'))
    )
    op.create_table('users_orgs',
    sa.Column('user_id', sa.Integer(), nullable=False),
    sa.Column('org_code', sa.String(length=10), nullable=False),
    sa.Column('created_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False),
    sa.Column('created_by', sa.Unicode(length=255), nullable=False),
    sa.Column('updated_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False),
    sa.Column('updated_by', sa.Unicode(length=255), nullable=False),
    sa.ForeignKeyConstraint(['org_code'], ['orgs.code'], name=op.f('fk_users_orgs_org_code_orgs')),
    sa.ForeignKeyConstraint(['user_id'], ['users.id'], name=op.f('fk_users_orgs_user_id_users')),
    sa.PrimaryKeyConstraint('user_id', 'org_code', name=op.f('pk_users_orgs'))
    )
    op.add_column('event_dates', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.drop_constraint('fk_event_dates_study_id_studies_forms', 'event_dates', type_='foreignkey')
    op.create_foreign_key(op.f('fk_event_dates_org_code_studies_forms'), 'event_dates', 'studies_forms', ['org_code', 'study_id', 'form_code'], ['org_code', 'study_id', 'form_code'])
    op.add_column('form_images', sa.Column('created_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False))
    op.add_column('form_images', sa.Column('created_by', sa.Unicode(length=255), nullable=False))
    op.add_column('form_images', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.add_column('form_images', sa.Column('updated_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False))
    op.add_column('form_images', sa.Column('updated_by', sa.Unicode(length=255), nullable=False))
    op.create_unique_constraint('desc', 'form_images', ['org_code', 'desc'])
    op.drop_constraint('uq_form_images_desc', 'form_images', type_='unique')
    op.create_foreign_key(op.f('fk_form_images_org_code_orgs'), 'form_images', 'orgs', ['org_code'], ['code'])
    op.add_column('forms', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.create_unique_constraint('form_name', 'forms', ['org_code', 'form_name'])
    op.drop_constraint('uq_forms_form_name', 'forms', type_='unique')
    op.drop_constraint('fk_forms_calendar_icon_code_form_images', 'forms', type_='foreignkey')
    op.create_foreign_key(op.f('fk_forms_org_code_orgs'), 'forms', 'orgs', ['org_code'], ['code'])
    op.create_foreign_key(op.f('fk_forms_org_code_form_images'), 'forms', 'form_images', ['org_code', 'calendar_icon_code'], ['org_code', 'code'])
    op.add_column('precalendar', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.drop_constraint('fk_precalendar_study_id_studies_forms', 'precalendar', type_='foreignkey')
    op.create_foreign_key(op.f('fk_precalendar_org_code_studies_forms'), 'precalendar', 'studies_forms', ['org_code', 'study_id', 'form_code'], ['org_code', 'study_id', 'form_code'])
    op.add_column('question_items', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.drop_constraint('fk_question_items_form_code_questions', 'question_items', type_='foreignkey')
    op.create_foreign_key(op.f('fk_question_items_org_code_questions'), 'question_items', 'questions', ['org_code', 'form_code', 'question_seqno'], ['org_code', 'form_code', 'seqno'])
    op.add_column('questions', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.drop_constraint('fk_questions_form_code_forms', 'questions', type_='foreignkey')
    op.drop_constraint('fk_questions_image_code_form_images', 'questions', type_='foreignkey')
    op.create_foreign_key(op.f('fk_questions_org_code_forms'), 'questions', 'forms', ['org_code', 'form_code'], ['org_code', 'code'])
    op.create_foreign_key(op.f('fk_questions_org_code_form_images'), 'questions', 'form_images', ['org_code', 'image_code'], ['org_code', 'code'])
    op.add_column('responses', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.drop_constraint('fk_responses_study_id_event_dates', 'responses', type_='foreignkey')
    op.drop_constraint('fk_responses_form_code_questions', 'responses', type_='foreignkey')
    op.create_foreign_key(op.f('fk_responses_org_code_questions'), 'responses', 'questions', ['org_code', 'form_code', 'question_seqno'], ['org_code', 'form_code', 'seqno'])
    op.create_foreign_key(op.f('fk_responses_org_code_event_dates'), 'responses', 'event_dates', ['org_code', 'study_id', 'form_code', 'participant_uid', 'event_date'], ['org_code', 'study_id', 'form_code', 'participant_uid', 'date'], ondelete='CASCADE')
    op.add_column('studies', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.drop_index('ix_studies_common_order', table_name='studies')
    op.create_index('ix_studies_common_order', 'studies', ['org_code', 'start_date', 'end_date', 'study_name'], unique=False)
    op.create_foreign_key(op.f('fk_studies_org_code_orgs'), 'studies', 'orgs', ['org_code'], ['code'])
    op.add_column('studies_forms', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.drop_constraint('fk_studies_forms_form_code_forms', 'studies_forms', type_='foreignkey')
    op.create_foreign_key(op.f('fk_studies_forms_org_code_forms'), 'studies_forms', 'forms', ['org_code', 'form_code'], ['org_code', 'code'])
    ### end Alembic commands ###
sqlalchemy-bot commented 7 years ago

Michael Bayer (zzzeek) wrote:

I find hidden and contradictory rules like this to be completely confusing but, if that's the way it's meant to be, so be it.

the naming convention feature was added on top of the existing system where names were optional. in order for it to not add harsh requirements for existing code (things like, you can no longer say unique=True because you aren't expressing how you want the contraint named, things like that), it had to have some automatic behaviors. and that behavior is very simple. If you gave the constraint name already, no naming convention is applied. if you name it None, the naming convention is applied.

that is the whole rule for naming conventions.

Now, there is another feature that you are also using. You are using a special, magical, and completely optional token called %(constraint_name)s in your CHECK naming convention. This token by necessity completely changes the naming convention rule, in that it makes a name based on an existing name. If this feature is confusing, you don't need to use it. It is optional. As i have stated, while there is some documentation which refers to its behavior, this token really needs a big bright section indicating that it does this. Which is part of what is being worked on in https://bitbucket.org/zzzeek/sqlalchemy/issues/4080/warn-on-ambiguous-naming-convention-case.

in your second model, you created ForeignKeyContraints and UniqueConstraints, and for some reason, you filled in the "name" field. If you have a naming convention on your MetaData() that you'd like to make use of, and those conventions do not use the %(constraint_name)s "name based on a name" token, then these names should not be present. That is what the warning in https://bitbucket.org/zzzeek/sqlalchemy/issues/4080/warn-on-ambiguous-naming-convention-case will tell you, if I decide to add it - that you made a naming convention, but then you put names on your constraints that aren't needed, and this seems contradictory. If OTOH you use the conv() / f() token (another bug, those two functions don't have the same name across SQLAchemy / Alembic), you've stated your intent explicitly that you want to skip the naming convention for that name. The new warning will try to encourage this explicitness (but can't break things for existing projects).

The final thing I will note about your migration is that if one set of schema changes resulted in that many individual changes in one migration, that's a complete refactor of your schema into something dramatically different, and if the intent is to run on existing systems that have real data, is not very practical because all of those columns being added would likely need to have data migrated into them from other columns. This is a reason your suggestion that all the column drops should be listed out before the column adds is not feasible; usually a data migration needs to move data from one column to the other before dropping the old column. But also, with that degree of data change, you'd very possibly be better off by just building a brand new schema from scratch and migrating the old data to the new one, because that looks like way too many changes. If that's going to be run against production data, you'd be better off just dumping from one schema to another one rather than working through all of that one column at a time. And if there is not any production data, and this is all just development stuff, then I'd have the migration simply blow away the old schema and build the new one from scratch.

sqlalchemy-bot commented 7 years ago

Matt Jernigan (juanitogan) wrote:

It is quite apparent that you and I live in completely different worlds in regards to database development. While this is indeed a significant change to the data model, it is relatively pretty small compared to my usual work.

Also, I never suggested dropping columns before adding columns. That would be idiotic to set as a default order.

What this is really coming down to, however, is that you don't seem interested in understanding how other people work. So, if your goal is to build a tool that supports only how you work then, that's fine, but make that clear so I know to look for another tool up front. I come from the enterprise world where the database is the primary asset of the company and you protect your data integrity like a fortress. Domain/key normal form and other advanced forms are the norm because savvy companies need to slice data all sorts of ways to make their data work for them instead of the other way around -- where I see startups failing a lot. And, thus, feature changes often come with large migrations.

We tried Rails once but its normal form limitations were brutal and its automigrations were dangerous. Sure, that paradigm has its uses, but not to me. But if that's all you want Alembic to be, so be it. Me, however, I find SQLAlchemy to be worlds better than ActiveRecord and thought I would test Alembic as well on this small project. As it is right now, I find Alembic to be useful as a code generator for the model up front, easy to plug into the deployment, possibly useful for small changes but likely not due to the unfinished features, and more trouble than it's worth for large changes -- but with good potential to be much better. Because I also work with a data modeling tool, and the difficulties in offering help here, I probably should have just stuck with that for code generation and looked elsewhere for automating migration deployments.

Another challenge is that devs like me may write major migrations once every 5 years or so. Thus, I don't always remember off the top of my head the proper order for dropping constraints before transforming data, etc. When doing full-stack work I have the additional challenge that I don't have time to take a deep dive into all the tech and just need a tool that is straight forward and predictable. So, when you talk about meta data and options with this or that, I don't know what you are talking about and don't always have time to find out. I'll often look for another tool before wrestling with a tool that doesn't smell right.

So, if after all that, you have any interest in how I work, this is how my migration ended up, even though it's not in optimal default order for most scenarios:

#!python
def upgrade():
    ### commands auto generated by Alembic - please adjust! ###
    op.create_table('orgs',
    sa.Column('code', sa.String(length=10), nullable=False),
    sa.Column('org_name', sa.Unicode(length=50), nullable=False),
    sa.Column('email', sa.Unicode(length=255), nullable=False),
    sa.Column('phone', sa.Unicode(length=20), nullable=False),
    sa.Column('created_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False),
    sa.Column('created_by', sa.Unicode(length=255), nullable=False),
    sa.Column('updated_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False),
    sa.Column('updated_by', sa.Unicode(length=255), nullable=False),
    sa.CheckConstraint("code ~ '^[a-z0-9-]+$'", name=op.f('ck_orgs_domain_friendly')),
    sa.PrimaryKeyConstraint('code', name=op.f('pk_orgs')),
    sa.UniqueConstraint('org_name', name=op.f('uq_orgs_org_name'))
    )
    op.create_table('users_orgs',
    sa.Column('user_id', sa.Integer(), nullable=False),
    sa.Column('org_code', sa.String(length=10), nullable=False),
    sa.Column('created_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False),
    sa.Column('created_by', sa.Unicode(length=255), nullable=False),
    sa.Column('updated_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False),
    sa.Column('updated_by', sa.Unicode(length=255), nullable=False),
    sa.ForeignKeyConstraint(['org_code'], ['orgs.code'], name=op.f('fk_users_orgs_org_code_orgs')),
    sa.ForeignKeyConstraint(['user_id'], ['users.id'], name=op.f('fk_users_orgs_user_id_users')),
    sa.PrimaryKeyConstraint('user_id', 'org_code', name=op.f('pk_users_orgs'))
    )
    """
    op.add_column('event_dates', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.drop_constraint('fk_event_dates_study_id_studies_forms', 'event_dates', type_='foreignkey')
    op.create_foreign_key(op.f('fk_event_dates_org_code_studies_forms'), 'event_dates', 'studies_forms', ['org_code', 'study_id', 'form_code'], ['org_code', 'study_id', 'form_code'])
    op.add_column('form_images', sa.Column('created_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False))
    op.add_column('form_images', sa.Column('created_by', sa.Unicode(length=255), nullable=False))
    op.add_column('form_images', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.add_column('form_images', sa.Column('updated_at', sa.DateTime(timezone=True), server_default=sa.text('now()'), nullable=False))
    op.add_column('form_images', sa.Column('updated_by', sa.Unicode(length=255), nullable=False))
    op.create_unique_constraint('desc', 'form_images', ['org_code', 'desc'])
    op.drop_constraint('uq_form_images_desc', 'form_images', type_='unique')
    op.create_foreign_key(op.f('fk_form_images_org_code_orgs'), 'form_images', 'orgs', ['org_code'], ['code'])
    op.add_column('forms', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.create_unique_constraint('form_name', 'forms', ['org_code', 'form_name'])
    op.drop_constraint('uq_forms_form_name', 'forms', type_='unique')
    op.drop_constraint('fk_forms_calendar_icon_code_form_images', 'forms', type_='foreignkey')
    op.create_foreign_key(op.f('fk_forms_org_code_orgs'), 'forms', 'orgs', ['org_code'], ['code'])
    op.create_foreign_key(op.f('fk_forms_org_code_form_images'), 'forms', 'form_images', ['org_code', 'calendar_icon_code'], ['org_code', 'code'])
    op.add_column('precalendar', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.drop_constraint('fk_precalendar_study_id_studies_forms', 'precalendar', type_='foreignkey')
    op.create_foreign_key(op.f('fk_precalendar_org_code_studies_forms'), 'precalendar', 'studies_forms', ['org_code', 'study_id', 'form_code'], ['org_code', 'study_id', 'form_code'])
    op.add_column('question_items', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.drop_constraint('fk_question_items_form_code_questions', 'question_items', type_='foreignkey')
    op.create_foreign_key(op.f('fk_question_items_org_code_questions'), 'question_items', 'questions', ['org_code', 'form_code', 'question_seqno'], ['org_code', 'form_code', 'seqno'])
    op.add_column('questions', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.drop_constraint('fk_questions_form_code_forms', 'questions', type_='foreignkey')
    op.drop_constraint('fk_questions_image_code_form_images', 'questions', type_='foreignkey')
    op.create_foreign_key(op.f('fk_questions_org_code_forms'), 'questions', 'forms', ['org_code', 'form_code'], ['org_code', 'code'])
    op.create_foreign_key(op.f('fk_questions_org_code_form_images'), 'questions', 'form_images', ['org_code', 'image_code'], ['org_code', 'code'])
    op.add_column('responses', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.drop_constraint('fk_responses_study_id_event_dates', 'responses', type_='foreignkey')
    op.drop_constraint('fk_responses_form_code_questions', 'responses', type_='foreignkey')
    op.create_foreign_key(op.f('fk_responses_org_code_questions'), 'responses', 'questions', ['org_code', 'form_code', 'question_seqno'], ['org_code', 'form_code', 'seqno'])
    op.create_foreign_key(op.f('fk_responses_org_code_event_dates'), 'responses', 'event_dates', ['org_code', 'study_id', 'form_code', 'participant_uid', 'event_date'], ['org_code', 'study_id', 'form_code', 'participant_uid', 'date'], ondelete='CASCADE')
    op.add_column('studies', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.drop_index('ix_studies_common_order', table_name='studies')
    op.create_index('ix_studies_common_order', 'studies', ['org_code', 'start_date', 'end_date', 'study_name'], unique=False)
    op.create_foreign_key(op.f('fk_studies_org_code_orgs'), 'studies', 'orgs', ['org_code'], ['code'])
    op.add_column('studies_forms', sa.Column('org_code', sa.String(length=10), nullable=False))
    op.drop_constraint('fk_studies_forms_form_code_forms', 'studies_forms', type_='foreignkey')
    op.create_foreign_key(op.f('fk_studies_forms_org_code_forms'), 'studies_forms', 'forms', ['org_code', 'form_code'], ['org_code', 'code'])
    """
    ### end Alembic commands ###

    ### MJJ added/modified:

    # Would also change positions of new columns if Postgres allowed it.

    # Don't NOT NULL the columns until filled.
    op.add_column('event_dates', sa.Column('org_code', sa.String(length=10)))
    op.add_column('form_images', sa.Column('org_code', sa.String(length=10)))
    op.add_column('form_images', sa.Column('created_at', sa.DateTime(timezone=True), server_default=sa.text('now()')))
    op.add_column('form_images', sa.Column('created_by', sa.Unicode(length=255)))
    op.add_column('form_images', sa.Column('updated_at', sa.DateTime(timezone=True), server_default=sa.text('now()')))
    op.add_column('form_images', sa.Column('updated_by', sa.Unicode(length=255)))
    op.add_column('forms', sa.Column('org_code', sa.String(length=10)))
    op.add_column('precalendar', sa.Column('org_code', sa.String(length=10)))
    op.add_column('question_items', sa.Column('org_code', sa.String(length=10)))
    op.add_column('questions', sa.Column('org_code', sa.String(length=10)))
    op.add_column('responses', sa.Column('org_code', sa.String(length=10)))
    op.add_column('studies', sa.Column('org_code', sa.String(length=10)))
    op.add_column('studies_forms', sa.Column('org_code', sa.String(length=10)))

    # Fill in the new org stuff.
    op.execute("""
        INSERT INTO orgs
            (code, org_name, email, phone, created_by, updated_by)
        VALUES
            ('tlfb', 'XXXXX', 'xxxxxxxxxx', 'XXXXXXXXXXX', 'DBA', 'DBA')
        ;""")
    op.execute("UPDATE forms SET org_code = 'tlfb';")
    op.execute("UPDATE questions SET org_code = 'tlfb';")
    op.execute("UPDATE question_items SET org_code = 'tlfb';")
    op.execute("UPDATE studies_forms SET org_code = 'tlfb';")
    op.execute("UPDATE studies SET org_code = 'tlfb';")
    op.execute("UPDATE precalendar SET org_code = 'tlfb';")
    op.execute("UPDATE event_dates SET org_code = 'tlfb';")
    op.execute("UPDATE responses SET org_code = 'tlfb';")
    op.execute("""UPDATE form_images SET org_code = 'tlfb'
        , path = SUBSTR(path, 1, 15) || 'tlfb/' || SUBSTR(path, 16)
        , created_at = CURRENT_TIMESTAMP
        , created_by = 'DBA'
        , updated_at = CURRENT_TIMESTAMP
        , updated_by = 'DBA'
        ;""")
    op.execute("""
        INSERT INTO users_orgs
            (user_id, org_code, created_by, updated_by)
            (SELECT id, 'tlfb', 'DBA', 'DBA' FROM users)
        ;""")
    op.execute("UPDATE users SET admin = FALSE WHERE email NOT IN ('xxxxxxx', 'xxxxxxxxxxxxx');")

    # Now we can NOT NULL the columns.
    op.alter_column('event_dates', 'org_code', nullable = False)
    op.alter_column('form_images', 'org_code', nullable = False)
    op.alter_column('form_images', 'created_at', nullable = False)
    op.alter_column('form_images', 'created_by', nullable = False)
    op.alter_column('form_images', 'updated_at', nullable = False)
    op.alter_column('form_images', 'updated_by', nullable = False)
    op.alter_column('forms', 'org_code', nullable = False)
    op.alter_column('precalendar', 'org_code', nullable = False)
    op.alter_column('question_items', 'org_code', nullable = False)
    op.alter_column('questions', 'org_code', nullable = False)
    op.alter_column('responses', 'org_code', nullable = False)
    op.alter_column('studies', 'org_code', nullable = False)
    op.alter_column('studies_forms', 'org_code', nullable = False)

    # Do some more Alembic stuff (must drop FKs before redoing PKs).
    op.drop_constraint('fk_event_dates_study_id_studies_forms', 'event_dates', type_='foreignkey')
    op.drop_constraint('fk_forms_calendar_icon_code_form_images', 'forms', type_='foreignkey')
    op.drop_constraint('fk_precalendar_study_id_studies_forms', 'precalendar', type_='foreignkey')
    op.drop_constraint('fk_question_items_form_code_questions', 'question_items', type_='foreignkey')
    op.drop_constraint('fk_questions_form_code_forms', 'questions', type_='foreignkey')
    op.drop_constraint('fk_questions_image_code_form_images', 'questions', type_='foreignkey')
    op.drop_constraint('fk_responses_study_id_event_dates', 'responses', type_='foreignkey')
    op.drop_constraint('fk_responses_form_code_questions', 'responses', type_='foreignkey')
    op.drop_constraint('fk_studies_forms_form_code_forms', 'studies_forms', type_='foreignkey')

    # Redo PKs.
    op.drop_constraint('pk_forms', 'forms', type_='primary')
    op.create_primary_key(op.f('pk_forms'), 'forms', ['org_code', 'code'])

    op.drop_constraint('pk_questions', 'questions', type_='primary')
    op.create_primary_key(op.f('pk_questions'), 'questions', ['org_code', 'form_code', 'seqno'])

    op.drop_constraint('pk_question_items', 'question_items', type_='primary')
    op.create_primary_key(op.f('pk_question_items'), 'question_items', ['org_code', 'form_code', 'question_seqno', 'seqno'])

    op.drop_constraint('pk_studies_forms', 'studies_forms', type_='primary')
    op.create_primary_key(op.f('pk_studies_forms'), 'studies_forms', ['org_code', 'study_id', 'form_code'])

    op.drop_constraint('pk_precalendar', 'precalendar', type_='primary')
    op.create_primary_key(op.f('pk_precalendar'), 'precalendar', ['org_code', 'study_id', 'participant_uid', 'form_code'])

    op.drop_constraint('pk_event_dates', 'event_dates', type_='primary')
    op.create_primary_key(op.f('pk_event_dates'), 'event_dates', ['org_code', 'study_id', 'participant_uid', 'form_code', 'date'])

    op.drop_constraint('pk_responses', 'responses', type_='primary')
    op.create_primary_key(op.f('pk_responses'), 'responses', ['org_code', 'study_id', 'participant_uid', 'form_code', 'event_date', 'question_seqno', 'repeat_counter'])

    op.drop_constraint('pk_form_images', 'form_images', type_='primary')
    op.create_primary_key(op.f('pk_form_images'), 'form_images', ['org_code', 'code'])

    # Do the rest of the Alembic stuffs (in proper order).
    op.drop_constraint('uq_form_images_desc', 'form_images', type_='unique')
    #BUG!#op.create_unique_constraint('desc', 'form_images', ['org_code', 'desc'])
    op.create_unique_constraint(op.f('uq_form_images_desc'), 'form_images', ['org_code', 'desc'])
    op.drop_constraint('uq_forms_form_name', 'forms', type_='unique')
    #BUG!#op.create_unique_constraint('form_name', 'forms', ['org_code', 'form_name'])
    op.create_unique_constraint(op.f('uq_forms_form_name'), 'forms', ['org_code', 'form_name'])

    op.drop_index('ix_studies_common_order', table_name='studies')
    op.create_index('ix_studies_common_order', 'studies', ['org_code', 'start_date', 'end_date', 'study_name'], unique=False)

    op.create_foreign_key(op.f('fk_event_dates_org_code_studies_forms'), 'event_dates', 'studies_forms', ['org_code', 'study_id', 'form_code'], ['org_code', 'study_id', 'form_code'])
    op.create_foreign_key(op.f('fk_form_images_org_code_orgs'), 'form_images', 'orgs', ['org_code'], ['code'])
    op.create_foreign_key(op.f('fk_forms_org_code_orgs'), 'forms', 'orgs', ['org_code'], ['code'])
    op.create_foreign_key(op.f('fk_forms_org_code_form_images'), 'forms', 'form_images', ['org_code', 'calendar_icon_code'], ['org_code', 'code'])
    op.create_foreign_key(op.f('fk_precalendar_org_code_studies_forms'), 'precalendar', 'studies_forms', ['org_code', 'study_id', 'form_code'], ['org_code', 'study_id', 'form_code'])
    op.create_foreign_key(op.f('fk_question_items_org_code_questions'), 'question_items', 'questions', ['org_code', 'form_code', 'question_seqno'], ['org_code', 'form_code', 'seqno'])
    op.create_foreign_key(op.f('fk_questions_org_code_forms'), 'questions', 'forms', ['org_code', 'form_code'], ['org_code', 'code'])
    op.create_foreign_key(op.f('fk_questions_org_code_form_images'), 'questions', 'form_images', ['org_code', 'image_code'], ['org_code', 'code'])
    op.create_foreign_key(op.f('fk_responses_org_code_event_dates'), 'responses', 'event_dates', ['org_code', 'study_id', 'form_code', 'participant_uid', 'event_date'], ['org_code', 'study_id', 'form_code', 'participant_uid', 'date'], ondelete='CASCADE')
    op.create_foreign_key(op.f('fk_responses_org_code_questions'), 'responses', 'questions', ['org_code', 'form_code', 'question_seqno'], ['org_code', 'form_code', 'seqno'])
    op.create_foreign_key(op.f('fk_studies_org_code_orgs'), 'studies', 'orgs', ['org_code'], ['code'])
    op.create_foreign_key(op.f('fk_studies_forms_org_code_forms'), 'studies_forms', 'forms', ['org_code', 'form_code'], ['org_code', 'code'])

    # Fix old incorrect constraints.
    #BUG!#op.drop_constraint('ck_forms_code_friendly', 'forms', type_='check')
    op.drop_constraint('code_friendly', 'forms', type_='check')
    op.create_check_constraint(op.f('ck_forms_code_friendly'), "forms", "code ~ '^[A-Z0-9_-]+$'")

    #BUG!#op.drop_constraint('ck_question_types_code_friendly', 'question_types', type_='check')
    op.drop_constraint('code_friendly', 'question_types', type_='check')
    op.create_check_constraint(op.f('ck_question_types_code_friendly'), "question_types", "code ~ '^[A-Z_]+$'")

    #BUG!#op.drop_constraint('ck_form_images_code_friendly', 'form_images', type_='check')
    op.drop_constraint('code_friendly', 'form_images', type_='check')
    op.create_check_constraint(op.f('ck_form_images_code_friendly'), "form_images", "code ~ '^[A-Z0-9_-]+$'")
sqlalchemy-bot commented 7 years ago

Michael Bayer (zzzeek) wrote:

Also, I never suggested dropping columns before adding columns.

my apologies, I looked again at https://bitbucket.org/zzzeek/alembic/issues/454/autogenerate-could-easily-help-us-out-a and those are all constraint drops. I apologize.

What this is really coming down to, however, is that you don't seem interested in understanding how other people work. So, if your goal is to build a tool that supports only how you work then, that's fine, but make that clear so I know to look for another tool up front. I come from the enterprise world where the database is the primary asset of the company and you protect your data integrity like a fortress. Domain/key normal form and other advanced forms are the norm because savvy companies need to slice data all sorts of ways to make their data work for them instead of the other way around -- where I see startups failing a lot. And, thus, feature changes often come with large migrations.

It is unfortunate that you feel this way. In reality, you are sending me large out-of-context fragments of code (enormous migrations with no model / table information so I can see what's generating it) and vague descriptions of vast new areas of functionality and doing a fairly poor job of describing specific features very exactly. More productive would be, "here is a model with table A, and table B. When I run autogenerate, the drop_constraints and add_constraints are organized in this non-intuitive way, when it should be like this: ". That's something I can actually work with. Making me do the job of interviewing you and spending hours trying to visualize what you might be thinking is an enormous waste of time for everyone. If you want a new feature in a product, as opposed to a simple fix, there is some degree of salesmanship one should be employing, rather than attacking the project maintainer for not appreciating your ideas that you can't be bothered to describe clearly. In addition, the way issues like #452 are described, this does not give me any of the information I need to reproduce, but I see this is my fault because I did not put bug posting instructions on alembic the way I have on sqlalchemy, so I shall fix that now: https://bitbucket.org/zzzeek/alembic/issues/new

I have implemented literally thousands of fixes and features for hundreds if not thousands of users for well over a decade, and if you look through Alembic or SQLAlchemy you can see some extremely elaborate features that I had to get my head around in order to understand a use case very deeply that I did not previously. Within Alembic, an example is the entire branching feature which I spent months on and was entirely in response to a user very patiently and carefully describing their use case to me. You can see how I went from no clue to development momentum. Literally the entire versioning model of Alembic was rewritten. I would suggest that when you resort to ad hominem attacks on other developers, that sends a message that you don't really have interest in solving problems. Such attacks are counter productive for everyone. They are also fairly rare as among the thousands of users I've interacted with; patient and thoughtful communication is taken as the basic foundation upon which we all work, but every few years I find myself writing one of these responses, which is an unfortunate use of my time (and yours).

Another challenge is that devs like me may write major migrations once every 5 years or so. Thus, I don't always remember off the top of my head the proper order for dropping constraints before transforming data, etc. When doing full-stack work I have the additional challenge that I don't have time to take a deep dive into all the tech and just need a tool that is straight forward and predictable.

Most people don't write major migrations of this level very often at all and in production software they are very rare because they introduce a lot of risk. However, if you are writing one migration that rebuilds the datamodel such that you dropping a whopping 22 contraints and creating 15 new ones, as well as rewriting all the data in between all the old/new columns and tables, this is not the kind of scenario where any tool is going to just write that all out for you, unless you have built on top of a very constrained system like Django. The "I just want the tool to do it" mindset and the "I know how to do this and I need automation" mindset are very far apart, and my tools are very much on the "automation" side. When I worked at Major League Baseball and every few years we needed to do a large refactor against Oracle, I would spend many weeks developing and testing the migrations so that not only all the table structures were changed correctly, but also so that tens of millions of rows of data would be quickly and flawlessly migrated to the new structures on the server within a small downtime window. The failure of these migrations would have been catastrophic.

Moving forward, as the main suggestion that I can't immediately act upon is one of how migrations are ordered, I would like you to look into the customizing revision generation plugin point as a place to start. This would be where you can work up the specific rules one by one for individual review and we can see how appropriate each one may be either for inclusion or just as part of a recipe. For a large and complicated feature request like this, it would start here with individual, specific proposals. Thanks for your time.

sqlalchemy-bot commented 7 years ago

Michael Bayer (zzzeek) wrote:

for the naming convention confusion I am trying to clarify the documentation and API at https://bitbucket.org/zzzeek/sqlalchemy/issues/4080/warn-on-ambiguous-naming-convention-case . For the migration rendering issue at #454, I look forward to more concrete, well-defined and even prototyped suggestions.

sqlalchemy-bot commented 7 years ago

Matt Jernigan (juanitogan) wrote:

I'm sorry, I felt you were annoyed and abrasive from the start with my three reports. I tried ignoring it but it seemed to keep coming and so I responded. My apologies. I do appreciate the work you do and I find it unfortunate that we aren't communicating well.

sqlalchemy-bot commented 7 years ago

Michael Bayer (zzzeek) wrote:

I apologize for that. It was three bugs, all submitted (or at least first seen by me) within 20 minutes of each other at around 7 pm on Friday night, all three kind of dealing with overlapping difficulties with naming conventions (admittedly not a very straightforward feature). Two did not include any detail on how to reproduce the issue, the one that did I responded by explaining the behavior fully and beginning work on an upstream SQLAlchemy issue to repair the problem. One bug report was asking for something that appeared very large and complicated yet was described in a vague and ambiguous way.

sqlalchemy-bot commented 7 years ago

Matt Jernigan (juanitogan) wrote:

And that one I marked as a proposal and not a bug and I still don't know how to be more clear about it without spending more time describing it than the code it should take. It wasn't meant to be large and complicated. Sorry for making it appear so.