Last modified: 2014-01-31 21:24:43 UTC
Creating flaggedrevs table...A database query syntax error has occurred. The last attempted database query was: "CREATE INDEX page_time ON flaggedrevs (fr_page_id,fr_rev_timestamp) " from within function "DatabaseBase::sourceFile( /var/wwwroot/dereckson.be/mediawiki/extensions/FlaggedRevs/backend/schema/mysql/FlaggedRevs.sql )". Database returned error "1: index page_time already exists" See also https://bugzilla.wikimedia.org/show_bug.cgi?id=31696
I think sqlite sees the page_time index on the logging table and barfs... Many of our indexes due no have prefixes, as MySQL doesn't have this problem (INDEX name uniqueness is sensibly done per-table rather than per-db). Why are you installing FlaggedRevs with sqlite? I suppose you could prefix the indexes with "fr_", though if code eventually has a FORCE statement it may explode... I'm not sure if this is worth renaming the INDEXes over (unless it's done in a huge batch with all the core tables too).
* Many of our indexes do not have prefixes
[ Why are you installing FlaggedRevs with sqlite? ] Short answer. To ensure compliance of our extensions with SQLite. Normal answer. I've no scenario where I need a SQLite working wiki installation with FlaggedRevs. My plan were each time I've to work on an extension, I would try it on my test SQLite wiki, so I can also improve compatibility with this SQL engine. [ Indexes issue ] The core tables indexes and +- 60 extensions already work fine on SQLite. In thix context, I think it would be interesting to improve this compatibility extension by extension. What would be the drawbacks to rename indexes? They are used only internally by database aren't they?
(In reply to comment #3) > [ Why are you installing FlaggedRevs with sqlite? ] > > Short answer. To ensure compliance of our extensions with SQLite. > > Normal answer. I've no scenario where I need a SQLite working wiki installation > with FlaggedRevs. My plan were each time I've to work on an extension, I would > try it on my test SQLite wiki, so I can also improve compatibility with this > SQL engine. > > [ Indexes issue ] > > The core tables indexes and +- 60 extensions already work fine on SQLite. In > thix context, I think it would be interesting to improve this compatibility > extension by extension. > > What would be the drawbacks to rename indexes? They are used only internally by > database aren't they? If you renamed them, then FORCE INDEX code in extensions will break on existing installs. On WMF, this is slightly tedious, since these tables have millions of rows. Though it's no where near as tedious as it was before percona OSC.
I support creating a separate SQL file for SQLite. More tedious, but saves us the pointless live schema change at WMF.
(In reply to comment #5) > I support creating a separate SQL file for SQLite. More tedious, but saves us > the pointless live schema change at WMF. You can overload the indexName() function for the sqlite one to map regular index names to prefixed ones so that FORCEs always work.
No, this is not a good solution: (1) this is not coherent - most other extensions and most other indexes in your extension have already a prefix ; (2) nothing would give us the warranty some extension doesn't use <the prefix the method would choose><the index>.
CC' Asher. I'm ok with prefixing the flaggedrevs and flaggedrevs_tracking indexes if he doesn't mind running the change in the background.
(In reply to comment #7) > No, this is not a good solution: > > (1) this is not coherent - most other extensions and most other indexes in your > extension have already a prefix ; > > (2) nothing would give us the warranty some extension doesn't use <the prefix > the method would choose><the index>. Can you make a gerrit patch and cc asher?
Yes, of course. My two first July weeks were a little busy, but now I've some time to prepare that as soon as I will have emptied my MediaWiki/Wikimedia backlog. Estimate delivery time: Wed July 25th.
Created attachment 10901 [details] Removing this fixes the problem. Removing this line of code fixes the problem.
(In reply to comment #10) > Yes, of course. > > My two first July weeks were a little busy, but now I've some time to prepare > that as soon as I will have emptied my MediaWiki/Wikimedia backlog. > > Estimate delivery time: Wed July 25th. Any progress on this?
Dereckson?
Dereckson: Are you working on this, or shall this bug be reassigned to default assignee again?
Comment on attachment 10901 [details] Removing this fixes the problem. Patch solution isn't acceptable: indexes are there for performance reason. Generally speaking, remove/comment lines generating errors or warnings to "make stuff works" could have dramatic consequence. For example, a Debian maintainer commented a line in OpenSSH which generated a compilation warning. That'd lead to a major security hole on every Debian servers: http://www.debian.org/security/key-rollover/
Sorry for the delay. I'm planning to facilitate bug like this resolution to add in core (class DatabaseUpdater) some methods to drop or rename an index: Gerrit change #39169.
Here we are. Gerrit change #40297.
Patch needs rework according to Gerrit...
(In reply to comment #4) > If you renamed them, then FORCE INDEX code in extensions will break on > existing > installs. On WMF, this is slightly tedious, since these tables have millions > of > rows. Though it's no where near as tedious as it was before percona OSC. Another idea I can think of is to rename everything to properly prefixed versions. Then introduce a hook into the FORCE INDEX stuff. That way environments that don't want to rename tables can use the hook to rewrite index names to their old names that are in use.
Change 110451 had a related patch set uploaded by Aaron Schulz: Added missing prefix to some indexes https://gerrit.wikimedia.org/r/110451
Change 110451 merged by Chad: Added missing prefix to some indexes https://gerrit.wikimedia.org/r/110451
