Last Comment Bug 496471 - satchel is performing inefficient queries for autocomplete results
: satchel is performing inefficient queries for autocomplete results
Status: RESOLVED FIXED
:
Product: Toolkit
Classification: Components
Component: Form Manager (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla21
Assigned To: Matthew N. [:MattN]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-06-04 17:22 PDT by Justin Dolske [:Dolske]
Modified: 2013-01-17 16:24 PST (History)
5 users (show)
MattN+bmo: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v.1 Simple patch to silence warning (1.65 KB, patch)
2013-01-16 17:51 PST, Matthew N. [:MattN]
dolske: review+
Details | Diff | Splinter Review

Description Justin Dolske [:Dolske] 2009-06-04 17:22:45 PDT
Debug builds give console warnings when populating the form history autocomplete dropdown. This is because we have an index on the "value" column, but the query is doing "ORDER BY UPPER(value)". MattN says we should be able to add an index for UPPER(value), which would fix this. Adding "/* do not warn (bug ###)*/" to the query will suppress the warning in the meantime, if we want to.
Comment 1 Matthew N. [:MattN] 2009-07-21 13:15:30 PDT
It turns out that SQLite does not support function-based indexes like I had thought.  We are changing the query in bug 370117 and bug 446247 making it more complex which will make it even harder to index.
Comment 2 :Gavin Sharp [email: gavin@gavinsharp.com] 2013-01-10 14:07:44 PST
Is this still relevant? Should we just add the warning removal comment? Or did this get addressed by those other bugs?
Comment 3 Matthew N. [:MattN] 2013-01-16 17:51:54 PST
Created attachment 703135 [details] [diff] [review]
v.1 Simple patch to silence warning

(In reply to :Gavin Sharp (away Jan 16-23) from comment #2)
> Is this still relevant? Should we just add the warning removal comment? Or did 
> this get addressed by those other bugs?

The query changed but we are still doing an ORDER BY on the result of function calls[1].  There haven't been any reports of slow form history in the 3.5 years since the more complex query landed and there is caching in JS for narrowing down existing results so I think we can silence this warning. I also did performance benchmarks in 2009 and the results were acceptable at the time.

AFAICT, SQLite still doesn't support function-based indexes.

[1] https://mxr.mozilla.org/mozilla-central/source/toolkit/components/satchel/nsFormAutoComplete.js?rev=5ce71981e005#255
Comment 4 Matthew N. [:MattN] 2013-01-16 18:41:33 PST
Thanks.

https://hg.mozilla.org/integration/fx-team/rev/7d49dd8c58dd
Comment 5 Matthew N. [:MattN] 2013-01-17 16:24:13 PST
https://hg.mozilla.org/mozilla-central/rev/7d49dd8c58dd

Note You need to log in before you can comment on or make changes to this bug.