PR#497: Even more indexes for DB - copr/copr

		`@@ -0,0 +1,30 @@`
		`+ """add indexes2`
		`+`
		`+ Revision ID: deadbeafc0de_add_indexes.py`
		`+ Revises: 10029c92dd0d`
		`+ Create Date: 2019-01-14 14:43:00.000000`
		`+`
		`+ """`
		`+`
		`+ # revision identifiers, used by Alembic.`
		`+ revision = 'deadbeafc0de'`
		`+ down_revision = 'deaddeadc0de'`
		`+`
		`+ from alembic import op`
		`+ import sqlalchemy as sa`
		`+`
		`+`
		`+ def upgrade():`
		`+ op.create_index('build_package_idx', 'build', ['package_id'], unique=False)`
		`+ op.create_index('copr_user_id_idx', 'copr', ['user_id'], unique=False)`
		`+ op.create_index('copr_name_group_id_idx', 'copr', ['name', 'group_id'], unique=False)`
		`+ op.create_index('package_copr_id_idx', 'package', ['copr_id'], unique=False)`
		`+ op.create_index('build_user_id_idx', 'build', ['user_id'], unique=False)`
		`+`
		`+`
		`+ def downgrade():`
		`+ op.drop_index('build_user_id_idx', table_name='build')`
		`+ op.drop_index('package_copr_id_idx', table_name='package')`
		`+ op.drop_index('copr_name_group_id_idx', table_name='copr')`
		`+ op.drop_index('copr_user_id_idx', table_name='copr')`
		`+ op.drop_index('build_package_idx', table_name='build')`

frontend/coprs_frontend/coprs/models.py

file modified

+5 -4

		`@@ -186,6 +186,7 @@`

		`__table_args__ = (`
		`db.Index('copr_webhook_secret', 'webhook_secret'),`
		`+ db.Index('copr_name_group_id_idx', 'name', 'group_id'),`
		`)`

		`id = db.Column(db.Integer, primary_key=True)`
		`@@ -208,7 +209,7 @@`
		`auto_createrepo = db.Column(db.Boolean, default=True)`

		`# relations`
		`- user_id = db.Column(db.Integer, db.ForeignKey("user.id"))`
		`+ user_id = db.Column(db.Integer, db.ForeignKey("user.id"), index=True)`
frostyx commented 5 years ago Hmm, for some reason I thought, that ForeignKey has automatically an index. One always learn something new.
		`user = db.relationship("User", backref=db.backref("coprs"))`
		`group_id = db.Column(db.Integer, db.ForeignKey("group.id"))`
		`group = db.relationship("Group", backref=db.backref("groups"))`
		`@@ -518,7 +519,7 @@`
		`builds = db.relationship("Build", order_by="Build.id")`

		`# relations`
		`- copr_id = db.Column(db.Integer, db.ForeignKey("copr.id"))`
		`+ copr_id = db.Column(db.Integer, db.ForeignKey("copr.id"), index=True)`
		`copr = db.relationship("Copr", backref=db.backref("packages"))`

		`copr_dir_id = db.Column(db.Integer, db.ForeignKey("copr_dir.id"), index=True)`
		`@@ -699,11 +700,11 @@`
		`srpm_url = db.Column(db.Text)`

		`# relations`
		`- user_id = db.Column(db.Integer, db.ForeignKey("user.id"))`
		`+ user_id = db.Column(db.Integer, db.ForeignKey("user.id"), index=True)`
		`user = db.relationship("User", backref=db.backref("builds"))`
		`copr_id = db.Column(db.Integer, db.ForeignKey("copr.id"), index=True)`
		`copr = db.relationship("Copr", backref=db.backref("builds"))`
		`- package_id = db.Column(db.Integer, db.ForeignKey("package.id"))`
		`+ package_id = db.Column(db.Integer, db.ForeignKey("package.id"), index=True)`
		`package = db.relationship("Package")`

		`chroots = association_proxy("build_chroots", "mock_chroot")`

msuchy commented 5 years ago

See individual commits for reasonings.

frostyx commented on line 14 of frontend/coprs_frontend/coprs/models.py 5 years ago

Hmm, for some reason I thought, that ForeignKey has automatically an index. One always learn something new.

frostyx commented 5 years ago

The only issue I have with this PR is that the first commit adds a migration and other commits modify it, which can hypothetically break things (checkout the first commit, alembic upgrade, checkout HEAD, alembic upgrade - will imho do nothing, because the particular migration was already executed, however, the indexes are missing). Adding migration per commit would be probably spamming unnecessary migrations, so personally, I would prefer model changes in separate commits as you did and adding a whole migration as the last commit.

Just wanted to state my opinion on it, but as it doesn't really matter, I am giving +1 anyway.

Pull-Request has been merged by msuchy

5 years ago