Closed Bug 493934 Opened 15 years ago Closed 15 years ago

Autocomplete queries are wrongly cutting away some result

Categories

(Toolkit :: Places, defect)

Product:
Toolkit
Component:
Places
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9.2a1

People

(Reporter: mak, Assigned: mak)

References

Details

(Keywords: fixed1.9.1)

Attachments

(2 files, 1 obsolete file)

patch v1.1
826 bytes, patch
Details | Diff | Splinter Review
patch v1.2
827 bytes, patch
sdwilsh
: review+
beltzner
: approval1.9.1+
Details | Diff | Splinter Review
Attached patch patch v1.0 (obsolete) — — Splinter Review 
due to the internal LIMIT clause, in case we have the same result in both disk and temp tables we could end cutting away some valid result.

Testing this requires to run autocomplete tests with sync enabled, so i'm not sure i can write a simple test for this. Will be tested as soon as we enable syncing for autocomplete (probably Bug 483980, where i found this issue, tests did exactly detect this problem).
Attachment #378569 - Flags: review?(sdwilsh)
i'd like to see this fixed in 1.9.1
Blocks: 483980
Comment on attachment 378569 [details] [diff] [review]
patch v1.0

>         "SELECT " MOZ_PLACES_COLUMNS " FROM moz_places "
>+          "WHERE id NOT IN (SELECT id FROM moz_places_temp) "
nit: this indentation seems a bit odd.

r=sdwilsh
Attachment #378569 - Flags: review?(sdwilsh) → review+
Attached patch patch v1.1 — — Splinter Review 
Can we land this for 1.9.1?
A test would be quite complex to write, this problem has been found while working on bug 483980, so when that bug will be ready we will execute all autocomplete tests with temp tables, and our current tests will test this too.
Due to this in some situation we could not show all results to the user in the awesomebar, cutting away some page wrongly.
Attachment #378569 - Attachment is obsolete: true
Attachment #378645 - Flags: approval1.9.1?
Attached patch patch v1.2 — — Splinter Review 
even better, be sure to use frecency index, r=sdwilsh on IRC
Attachment #378645 - Attachment is obsolete: true
Attachment #378648 - Flags: approval1.9.1?
Attachment #378645 - Flags: approval1.9.1?
Comment on attachment 378648 [details] [diff] [review]
patch v1.2

> even better, be sure to use frecency index, r=sdwilsh on IRC
and in the bug!
Attachment #378648 - Flags: review+
Comment on attachment 378645 [details] [diff] [review]
patch v1.1

a191=beltzner; needs to land on trunk for a green cycle before you can land it on 191
Attachment #378645 - Attachment is obsolete: false
Attachment #378648 - Flags: approval1.9.1? → approval1.9.1+
http://hg.mozilla.org/mozilla-central/rev/d50a28b50a2c
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: