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

Rename Unique Constraint #246

Closed
sqlalchemy-bot opened this issue Nov 18, 2014 · 9 comments
Closed

Rename Unique Constraint #246

sqlalchemy-bot opened this issue Nov 18, 2014 · 9 comments
Labels

Comments

@sqlalchemy-bot
Copy link

Migrated issue, originally created by nickretallack (@nickretallack)

I created a model with a UniqueConstraint like so:

class Person(db.Model):
	__tablename__ = 'person'

	id = db.Column(db.Integer, primary_key=True)

	first_name = db.Column(db.String, nullable=False)
	last_name = db.Column(db.String, nullable=False)

	__table_args__ = (
		db.UniqueConstraint(first_name, last_name),
	)

Then I changed the constraint's name.

class Person(db.Model):
	__tablename__ = 'person'

	id = db.Column(db.Integer, primary_key=True)

	first_name = db.Column(db.String, nullable=False)
	last_name = db.Column(db.String, nullable=False)

	__table_args__ = (
		db.UniqueConstraint(first_name, last_name, name="person_name"),
	)

The migration it generated looked like this:

def upgrade():
    ### commands auto generated by Alembic - please adjust! ###
    op.create_unique_constraint('person_name', 'person', ['first_name', 'last_name'])
    ### end Alembic commands ###


def downgrade():
    ### commands auto generated by Alembic - please adjust! ###
    op.drop_constraint('person_name', 'person')
    ### end Alembic commands ###

If I run this, I end up with two identical constraints in the database. Unfortunately, there doesn't seem to be a rename_constraint function anywhere. I suppose it can be handled by dropping the existing constraint and then recreating it? Alembic should detect and drop the existing constraint, right? But since it isn't named in the source code, I suppose Alembic can't find it? Either way, it could look at the existing constraints and realize that there isn't a corresponding one in the source code, and detect that the constraint must be renamed or dropped because of that. Unless you want to support people creating constraints on the database that don't exist in the code.

@sqlalchemy-bot
Copy link
Author

Changes by nickretallack (@nickretallack):

  • edited description

@sqlalchemy-bot
Copy link
Author

Michael Bayer (@zzzeek) wrote:

Unfortunately, there doesn't seem to be a rename_constraint function anywhere.

relational databases dont generally have RENAME CONSTRAINT functions, so yes, drop and recreate.

Alembic should detect and drop the existing constraint, right?

if this is SQLite, it does not because SQLite is the only database that supports unnamed unique constraints. Issue https://bitbucket.org/zzzeek/sqlalchemy/issue/3244/sqlite-inspection-with will help in SQLAlchemy 1.0, which when done in concert with Alembic's new SQLite migrations feature, will make this somewhat possible, but since the constraint is not named, there's still a guessing game going on that might never be foolproof.

Either way, it could look at the existing constraints and realize that there isn't a corresponding one in the source code, and detect that the constraint must be renamed or dropped because of that.

you can't drop constraints on SQLite (theres no ALTER TABLE..DROP CONSTRAINT), you can only recreate the whole table. If this isn't SQLite, then the constraint should have been given a name in which case it would have shown up with a DROP. If for some reason there's unnamed constraints on some other backend, then there's again no way to drop them, because "DROP CONSTRAINT" requires a name.

Unless you want to support people creating constraints on the database that don't exist in the code.

there are edge and exception cases that deal with Alembic having to navigate around this, implicitly created constraints and indexes and such, however if the DB is managing explicit constraints that the user doesn't want in the source code, that's again what include_object is for as far as autogenerate.

@sqlalchemy-bot
Copy link
Author

Michael Bayer (@zzzeek) wrote:

no bug here, no action to be taken, if you have further questions about how unique constraints work please bring them to the mailing list, thanks

@sqlalchemy-bot
Copy link
Author

Changes by Michael Bayer (@zzzeek):

  • changed status to closed

@sqlalchemy-bot
Copy link
Author

nickretallack (@nickretallack) wrote:

This is in postgres.

@sqlalchemy-bot
Copy link
Author

nickretallack (@nickretallack) wrote:

The constraint isn't totally nameless. The database generates a name for it.

@sqlalchemy-bot
Copy link
Author

Michael Bayer (@zzzeek) wrote:

OK then you'd see it here. it would show up as a drop.

@sqlalchemy-bot
Copy link
Author

Michael Bayer (@zzzeek) wrote:

OK and why you dont see it is this: we reflect the uniques from PG, we see a UQ like "person_first_name_last_name_key". But what PG actually does is it also creates an INDEX with that name, e.g. there's a constraint and an index, we see both of these. Already we are confused as we have to tell the difference between these two. At this point the code goes into a super-cautious section where it only emits the DROP if guesses that these might be two unrelated objects. If I change that logic to be more liberal I get this:

    batch_op.create_unique_constraint('uq_person', ['first_name', 'last_name'])
    batch_op.drop_constraint(u'person_first_name_last_name_key')
    batch_op.drop_index('person_first_name_last_name_key', table_name='person')

which is wrong! because that index will be already dropped when that drop_constraint happens.

this logic can be improved with additional information it will get in 1.0. But it's not changing anytime soon. It is much much much worse for autogenerate to create migrations that aren't there rather than miss some things, because the former issue happens on every single autogen run. so the logic is very conservative.

@sqlalchemy-bot
Copy link
Author

Adrian (@thiefmaster) wrote:

Relational databases dont generally have RENAME CONSTRAINT functions

At least PostgreSQL supports ALTER TABLE ... RENAME CONSTRAINT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant