402161 - moz_places_visitcount index is bad in some profiles

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect, P3)

Description

[Dietrich Ayala (:dietrich)]

seth from https://bugzilla.mozilla.org/show_bug.cgi?id=381795#c17

doesn't this change imply we've got nightly testers (anyone before the fix for
bug #389876, so pre-m7?) that have a moz_places_visitcount index (that does
them no good, as it indexes rev_host, not visit_count):

-        NS_LITERAL_CSTRING("CREATE INDEX moz_places_visitcount ON moz_places
(rev_host)"));
+        NS_LITERAL_CSTRING("CREATE INDEX moz_places_visitcount ON moz_places
(visit_count)"));

should we log a spin off bug on that issue (about dropping and recreating that
index if we detect it's bad?) 

this would be moot if we fix bug #402076

Comment 1

[Marco Bonardo [:mak]]

i'm about detecting and replacing the bogus index on long idle time

Comment 2

[Marco Bonardo [:mak]]

Attachment: patch

actually the replacement happens after 15 minutes if the bogus index is found, i've not used a transaction since it should be quite fast.

Comment 3

[Marco Bonardo [:mak]]

reminder: add a note into Bug 391419 for the removal of the migration code

Comment 4

[Marco Bonardo [:mak]]

Comment on attachment 303682 [details] [diff] [review]
patch

somehow bugzilla had put the wrong review request -_-'

Comment 5

[Dietrich Ayala (:dietrich)]

Comment on attachment 303682 [details] [diff] [review]
patch


>+    if (hasResult) {
>+      // drop old index
>+      rv = mDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
>+          "DROP INDEX IF EXISTS moz_places_visitcount"));
>+      NS_ENSURE_SUCCESS(rv, rv);
>+      // create new index
>+      rv = mDBConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
>+          "CREATE INDEX IF NOT EXISTS moz_places_visitcount "
>+          "ON moz_places (visit_count)"));
>+      NS_ENSURE_SUCCESS(rv, rv);
>+    }
>+

are those IF EXISTS bits necessary, since you've already manually checked that the index is there and misconfigured? r=me otherwise.

Comment 6

[Marco Bonardo [:mak]]

well no, not really necessary, but i tend to always put IF EXISTS in index deletion and creation (my personal choice), it's another layer of control to avoid some embarassing situation if something goes wrong, since it is executed on a long idle and only in rare cases, it should not have any bad impact. if you prefer i can remove them.

Comment 7

[Marco Bonardo [:mak]]

ok, we can go on

Comment 8

[Reed Loden [:reed]]

Checking in toolkit/components/places/src/nsNavHistory.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v  <--  nsNavHistory.cpp
new revision: 1.257; previous revision: 1.256
done

Comment 9

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv