Normalize annotation names in database

RESOLVED FIXED in Firefox 2 alpha2

Status

()

Firefox
Bookmarks & History
P1
normal
RESOLVED FIXED
12 years ago
8 years ago

People

(Reporter: Brett Wilson, Assigned: Brett Wilson)

Tracking

({fixed1.8.1})

Trunk
Firefox 2 alpha2
fixed1.8.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

19.55 KB, patch
Brian Ryner (not reading)
: review+
Details | Diff | Splinter Review
(Assignee)

Description

12 years ago
The annotation service currently stores names as text for each row. Since it looks increasingly like we will have relatively few numbers of annotation types and they will be applied to relatively large numbers of pages, it should be a performance and space win to separate out the names into a separate table.
(Assignee)

Updated

12 years ago
Priority: -- → P4
Summary: Normalize annotation service names → Normalize annotation names in database
Target Milestone: --- → Firefox 2 beta1
(Assignee)

Comment 1

12 years ago
Need this before a2 so DB format will be final by then.
Priority: P4 → P1
Target Milestone: Firefox 2 beta1 → Firefox 2 alpha2
(Assignee)

Comment 2

12 years ago
Created attachment 217314 [details] [diff] [review]
Patch

This patch creates a new table moz_anno_name that is used for the names of annotations. There is migration code that detects an unnormalized annotation table and moves to the new format. However, this will mean you will not be able to have an old and a new build coexisting on the same profile: the tables will be changed and the old build won't work.
Attachment #217314 - Flags: review?(bryner)
(Assignee)

Comment 3

12 years ago
Created attachment 217333 [details] [diff] [review]
Fixed patch

The previous patch broke annotation querying. This fixes it. The only difference is a one line change in nsNavHistory.cpp
Attachment #217314 - Attachment is obsolete: true
Attachment #217333 - Flags: review?(bryner)
Attachment #217314 - Flags: review?(bryner)
Comment on attachment 217333 [details] [diff] [review]
Fixed patch

>--- nsAnnotationService.cpp	23 Mar 2006 06:24:54 -0000	1.11
>+++ nsAnnotationService.cpp	5 Apr 2006 20:03:34 -0000
>+  // this needs to happen after moz_anno_name gets created
>+  if (migrateFromAlpha1)
>+    return MigrateFromAlpha1(aDBConn);

Do you really want to return early here?  Seems easy to add something after this "if" and forget about the migration case skipping it.

>+//
>+//    We don't remove anything from the moz_anno_name table. If we delete the
>+//    last item of a given name, that item really should go away. It will get
>+//    cleaned up on the next shutdown.

Where does it get cleaned up at?  If it happens implicitly somehow, you should comment on that.

Looks ok otherwise.
Attachment #217333 - Flags: review?(bryner) → review+
(Assignee)

Comment 5

12 years ago
> Do you really want to return early here?  Seems easy to add something after
> this "if" and forget about the migration case skipping it.

Good point


> Where does it get cleaned up at?  If it happens implicitly somehow, you should
> comment on that.

It gets cleaned up in nsNavHistoryExpire::EraseAnnotations in the patch for bug 328598 that you just rejected :) It currently just says "// FIXME bug 319455 expire annotations". We don't do any annotation expiration yet.
(Assignee)

Comment 6

12 years ago
Fixed on branch and trunk.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
(Assignee)

Comment 7

12 years ago
Note that this will upgrade the database format in a way that is not supportable by previous builds. Once you run a new build, you won't be able to run builds from before this patch without all kinds of things breaking like the bookmarks system. (It won't get corrupted, it just won't work).
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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.