Skip to content
This repository has been archived by the owner on May 3, 2020. It is now read-only.

upgrades: InnoDB upgrade fix #9

Merged

Conversation

egabancho
Copy link
Contributor

  • FIX Runs the sql statement which actually changes the engine from
    MyISAM to InnoDB.

Signed-off-by: Esteban J. G. Gabancho esteban.gabancho@gmail.com

"SELECT CONCAT('ALTER TABLE ',TABLE_NAME,' ENGINE=InnoDB;')"
" FROM INFORMATION_SCHEMA.TABLES"
" WHERE ENGINE='MyISAM'"
" AND table_schema = %s",
(current_app.config.get('CFG_DATABASE_NAME'),)
)
[run_sql(row[0]) for row in rows]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for row in rows:
    run_sql(row[0])

?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for row in rows:
    run_sql("ALTER TABLE '%s' ENGINE=InnoDB;", row[0])

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to do it using alembic instead, I'll update the PR asap

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not gonna happen 😢

@jirikuncar jirikuncar added this to the v0.1.2 milestone Sep 18, 2015
@jirikuncar jirikuncar self-assigned this Sep 18, 2015
@@ -39,13 +39,14 @@ def do_upgrade():
"""Carry out the upgrade."""
from flask import current_app
if current_app.config.get('CFG_DATABASE_TYPE') == 'mysql':
run_sql(
rows = run_sql(
"SELECT CONCAT('ALTER TABLE ',TABLE_NAME,' ENGINE=InnoDB;')"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"SELECT TABLE_NAME"

(current_app.config.get('CFG_DATABASE_NAME'),)
)
for table_name in table_names:
run_sql("ALTER TABLE `%s` ENGINE=InnoDB" % (table_name[0],))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`

vs.

'

cc @hachreak

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't make sense to change because actually this query is valid only for mysql.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@egabancho
Copy link
Contributor Author

It seems to be running just fine for CDS database, although it takes a lot of time

@jirikuncar jirikuncar assigned jirikuncar and unassigned egabancho Sep 21, 2015
@jirikuncar
Copy link
Contributor

:shipit:

* FIX Runs the sql statement which actually changes the engine from
  MyISAM to InnoDB.

Signed-off-by: Esteban J. G. Gabancho <esteban.gabancho@gmail.com>
@jirikuncar jirikuncar merged commit b0cbb9c into inveniosoftware-attic:master Sep 23, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants