Skip to content

Instantly share code, notes, and snippets.

@imkingdavid
Created February 6, 2013 19:28
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save imkingdavid/cc23ddbf247574397b61 to your computer and use it in GitHub Desktop.
Save imkingdavid/cc23ddbf247574397b61 to your computer and use it in GitHub Desktop.
IRC Log discussing migration roll backs for dropped tables. - end result: we probably shouldn't worry about it unless it actually becomes a problem.
[13:37:46] <nickvergessen> is migrations done now
[13:41:26] <imkingdavid> nickvergessen: the backend part has been merged but there are two more related PRs
[13:41:34] <nickvergessen> yeah
[13:41:36] <nickvergessen> just found that
[13:41:55] <imkingdavid> i'm working on testing the feature/migrations-extensions pr
[13:42:04] <imkingdavid> has some stuff to be changed in it
[13:46:56] --> spyro (~shentino@gentoo/contributor/shentino) has joined #phpBB-dev
[13:49:38] <-> erikfrerejean is now known as erik|away
[13:50:40] <nickvergessen> hmm
[13:51:55] <nickvergessen> https://area51.phpbb.com/phpBB/viewtopic.php?p=250441#p250441
[13:52:02] <nickvergessen> was not addressed in any way
[13:54:29] <imkingdavid> leave a note for nathan or remind him in the topic
[14:01:52] <naderman> imkingdavid: :)
[14:02:34] <imkingdavid> naderman: :)
[14:02:46] <imkingdavid> one small step for a man, one large step for a bertie
[14:02:57] <naderman> heh
[14:03:28] --> Raimon (~Adium@phpbb/manager/Raimon) has joined #phpBB-dev
[14:03:28] *** Mode #phpBB-dev +o Raimon by ChanServ
[14:04:45] <nickvergessen> one big fail for bertie
[14:04:51] <nickvergessen> this is serious and needs fixing
[14:08:59] <imkingdavid> nickvergessen: so the issue, as i understand it, is that one extension potentially drops a common table on which another extension has created a column, therefore potentially breaking the other extension? I'm not sure what can be done to prevent that
[14:09:23] <imkingdavid> should we just make it so no default phpbb tables can be dropped by extensions?
[14:09:31] <nickvergessen> remove potential from teh sentences
[14:09:47] <nickvergessen> no
[14:09:53] <nickvergessen> it needs general fixing
[14:10:33] <nickvergessen> and basically its simple
[14:10:37] <imkingdavid> well i'm assuming the mod team will at least glance at an extension's migrations before approving and if an extension tries to drop a default table that should raise red flags instantly
[14:10:50] <nickvergessen> you just need to log the current structure befor you start to do something
[14:11:15] <imkingdavid> so do an export of the table you're going to drop, and save that somewhere in the migrations table?
[14:12:16] <nickvergessen> yeah something like that
[14:12:39] <imkingdavid> let's say an extension drops the config table. now the board is completely borked. how does one solve this issue, just because they saved the schema in the database?
[14:12:52] <imkingdavid> they can hunt through the database in phpmyadmin to find the table schema and import it
[14:12:56] <imkingdavid> but it still doesn't have any data
[14:13:06] <imkingdavid> so it still doesn't matter that the schema was saved
[14:13:07] <nickvergessen> we dont care about that
[14:13:12] <imkingdavid> what do we care about then?
[14:13:18] <imkingdavid> i'm not understanding what you're trying to prevent
[14:13:19] <nickvergessen> if an extension creates an additional column on a table
[14:13:25] <nickvergessen> and another one drops it
[14:13:35] <nickvergessen> reverting the drop (by uninstalling the extension)
[14:13:50] <nickvergessen> will not restore the additional columns of the other extension
[14:15:59] <imkingdavid> I don't know why were so concerned about keeping the structure intact, when dropping a table is going to clear all data as well. both cases will required a restore from a recent backup
[14:16:14] <nickvergessen> revert should be a revert
[14:16:19] <nickvergessen> and not a reset to default
[14:19:03] <imkingdavid> I'm still not sure we should allow default tables to be dropped at all by extensions. I can think of 0 use cases for this, and allowing it only presents problems like this and others
[14:19:11] <imkingdavid> naderman: do you have any thoughts on this issue ^
[14:20:04] <nickvergessen> imkingdavid: this also applies to tables of extension
[14:20:05] <nickvergessen> s
[14:20:12] <naderman> it's an extensions job to work correctly
[14:20:23] <naderman> if an extension drops a table in a migration it better recreate it on rollback
[14:20:29] <naderman> we can't magically fix broken extensions
[14:20:35] <naderman> a broken mod could drop a table today too
[14:20:35] <nickvergessen> which it currently doesnt
[14:20:42] <naderman> nickvergessen: huh?
[14:20:49] <naderman> a migration can specify what happens on rollback
[14:20:54] <nickvergessen> yeah
[14:20:57] <naderman> so it should specify how to create the table on rollback
[14:21:07] <nickvergessen> but it doesnt know about additional columns from other extensions
[14:21:14] <naderman> oh I see
[14:21:23] <naderman> now I get it
[14:21:42] <naderman> imkingdavid: there's no point in preventing extensions from doing that
[14:21:50] <naderman> you'll end up with the same kind of problems with other tables etc.
[14:22:01] <naderman> really not a good idea to treat extensions somehow differently than core code
[14:22:10] <imkingdavid> in what case is an extension going to drop another extension's table?
[14:22:24] <nickvergessen> imkingdavid: in we dont care
[14:22:27] <nickvergessen> it can happen
[14:22:36] <naderman> imkingdavid: addons
[14:22:39] <nickvergessen> and reverting this stop should go back to the previous state
[14:22:47] <naderman> just like it may drop a core table
[14:22:49] <naderman> really doesn't matter
[14:22:58] <naderman> they should just be allowed to change in the schema what they want to change
[14:23:08] <naderman> nickvergessen: yeah you're right about that part
[14:23:17] <naderman> I suppose we need some mechanism for recreating a table
[14:23:23] <naderman> although that won't entirely fix it either
[14:23:33] <imkingdavid> so we care about schema but not dat
[14:23:34] <imkingdavid> data*
[14:23:35] <naderman> I mean at that point you are creating all kinds of conflicts that cannot be solved automatically
[14:23:47] <imkingdavid> we create an empty table that the extension cannot use anywa
[14:23:50] <imkingdavid> how does this help?
[14:23:51] <naderman> nickvergessen: imagine you try to apply an add column on a dropped table
[14:23:52] <imkingdavid> anyway*
[14:23:57] <naderman> I really think we can just ignore this
[14:24:02] <naderman> extensions will just have to not do shit
[14:24:03] <naderman> ^_^
[14:24:41] <naderman> I think this is part of what we'll just have to see with the alphas
[14:24:45] <naderman> and hopefully some real extensions
[14:24:50] <naderman> if any of this turns into a real issue
[14:24:56] <naderman> but I doubt it will actually come up in a real scenario
[14:24:59] <imkingdavid> it's up to the board admin to keep backups anyway
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment