Closed Bug 499985 Opened 16 years ago Closed 2 years ago

Sorting of query results should be done entirely in SQL

Categories

(Toolkit :: Places, defect, P3)

defect

Tracking

()

RESOLVED WONTFIX

People

(Reporter: adw, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Snappy:P3][fxperf:p3])

We can't notify viewers of asynchronously loaded batches of results as long as we're sorting in C++ after all results are retrieved. Currently we perform a first-pass sort in SQL and a final sort in C++ after all results have been loaded.
Depends on: 499990
Need to decide how bug 398190 relates to this one. I'm not sure yet where exactly in the code ORDER BY clauses need to go -- at first look I was thinking PlacesSQLQueryBuilder::SelectAsURI() and friends -- but I'd be (pleasantly) surprised if multiple paths don't need to be updated.
i'd say to dupe bug 398190 to this and try to upstream all sorting, as far as possible.
Depends on: 530236
This is hairier than I first thought, so I spun out bug 530236 to get started on nsNavHistoryFolderResultNode. Then I'll move on to nsNavHistoryQueryResultNode. Smaller focused bugs should be easier to review also. One problem is that nsNavHistoryContainerResultNode::FillStats must be called in some cases before the node's children are sorted. I have to find a way to do FillStats's work in SQL or avoid it altogether. One way would be to keep the stats up-to-date in the DB instead of computing them dynamically. Although, in a nightly I tried to get a folder and query container to refresh in the Library by renaming and adding children and such, but the view didn't update the containers' visit dates and visit counts like the code seems to be saying it should.
Whiteboard: [Snappy]
Whiteboard: [Snappy] → [Snappy:P3]
Assignee: adw → nobody
Status: ASSIGNED → NEW
Priority: -- → P3
Whiteboard: [Snappy:P3] → [Snappy:P3][fxperf]
Whiteboard: [Snappy:P3][fxperf] → [Snappy:P3][fxperf:p3]
Severity: normal → S3

I'm marking the bug as wontfix, not because it's not something we want to do, but because we are unlikely to make this kind of change to the existing querying code. We should refactor, or even rewrite, the querying API and consider performance of batch results from the beginning, doing as much as possible in SQL.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.