Bug 897074 - Limit the number of tokens used in satchel's getAutoCompleteResults SQL queries. r=dolske
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Tue, 10 Dec 2013 23:34:42 -0800
changeset 159887 c62c26e3c73f1dab8b6f905740c267bbafb64991
parent 159886 3cafeb339face40e7712eb33df7cde97edb0e5c1
child 159888 d52f80876573cbb795ec49583aef28becdba508a
push id25817
push userryanvm@gmail.com
push dateWed, 11 Dec 2013 18:30:50 +0000
treeherdermozilla-central@bf77172966ef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdolske
bugs897074
milestone29.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 897074 - Limit the number of tokens used in satchel's getAutoCompleteResults SQL queries. r=dolske
toolkit/components/satchel/FormHistory.jsm
toolkit/components/satchel/test/unit/test_autocomplete.js
--- a/toolkit/components/satchel/FormHistory.jsm
+++ b/toolkit/components/satchel/FormHistory.jsm
@@ -93,16 +93,17 @@ Components.utils.import("resource://gre/
 Components.utils.import("resource://gre/modules/Services.jsm");
 
 XPCOMUtils.defineLazyServiceGetter(this, "uuidService",
                                    "@mozilla.org/uuid-generator;1",
                                    "nsIUUIDGenerator");
 
 const DB_SCHEMA_VERSION = 4;
 const DAY_IN_MS  = 86400000; // 1 day in milliseconds
+const MAX_SEARCH_TOKENS = 30;
 const NOOP = function noop() {};
 
 let supportsDeletedTable =
 #ifdef ANDROID
   true;
 #else
   false;
 #endif
@@ -964,17 +965,18 @@ this.FormHistory = {
     if (searchString.length > 1) {
         searchTokens = searchString.split(/\s+/);
 
         // build up the word boundary and prefix match bonus calculation
         boundaryCalc = "MAX(1, :prefixWeight * (value LIKE :valuePrefix ESCAPE '/') + (";
         // for each word, calculate word boundary weights for the SELECT clause and
         // add word to the WHERE clause of the query
         let tokenCalc = [];
-        for (let i = 0; i < searchTokens.length; i++) {
+        let searchTokenCount = Math.min(searchTokens.length, MAX_SEARCH_TOKENS);
+        for (let i = 0; i < searchTokenCount; i++) {
             tokenCalc.push("(value LIKE :tokenBegin" + i + " ESCAPE '/') + " +
                             "(value LIKE :tokenBoundary" + i + " ESCAPE '/')");
             where += "AND (value LIKE :tokenContains" + i + " ESCAPE '/') ";
         }
         // add more weight if we have a traditional prefix match and
         // multiply boundary bonuses by boundary weight
         boundaryCalc += tokenCalc.join(" + ") + ") * :boundaryWeight)";
     } else if (searchString.length == 1) {
@@ -1016,17 +1018,18 @@ this.FormHistory = {
 
     let stmt = dbCreateAsyncStatement(query, params);
 
     // Chicken and egg problem: Need the statement to escape the params we
     // pass to the function that gives us the statement. So, fix it up now.
     if (searchString.length >= 1)
       stmt.params.valuePrefix = stmt.escapeStringForLIKE(searchString, "/") + "%";
     if (searchString.length > 1) {
-      for (let i = 0; i < searchTokens.length; i++) {
+      let searchTokenCount = Math.min(searchTokens.length, MAX_SEARCH_TOKENS);
+      for (let i = 0; i < searchTokenCount; i++) {
         let escapedToken = stmt.escapeStringForLIKE(searchTokens[i], "/");
         stmt.params["tokenBegin" + i] = escapedToken + "%";
         stmt.params["tokenBoundary" + i] =  "% " + escapedToken + "%";
         stmt.params["tokenContains" + i] = "%" + escapedToken + "%";
       }
     } else {
       // no additional params need to be substituted into the query when the
       // length is zero or one
--- a/toolkit/components/satchel/test/unit/test_autocomplete.js
+++ b/toolkit/components/satchel/test/unit/test_autocomplete.js
@@ -1,12 +1,14 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+"use strict";
+
 var testnum = 0;
 var fac;
 var prefs;
 
 let numRecords, timeGroupingSize, now;
 
 const DEFAULT_EXPIRE_DAYS = 180;
 
@@ -230,8 +232,51 @@ add_test(function test13() {
   }
 
   let results = autocompleteService.autoCompleteSearch("field5", "sync1", null, null);
   do_check_eq(results.matchCount, 2, "synchronous matchCount");
   do_check_eq(results.getValueAt(0), "sync1");
   do_check_eq(results.getValueAt(1), "sync1a");
   run_next_test();
 });
+
+add_test(function test_token_limit_DB() {
+    function test_token_limit_previousResult(previousResult) {
+        do_log_info("Check that the number of tokens used in a search is not capped to " +
+                    "MAX_SEARCH_TOKENS when using a previousResult");
+        // This provide more accuracy since performance is less of an issue.
+        let changes = [ ];
+        // Search for a string where the first 30 tokens match the value above but the 31st does not
+        // when re-using a previous result.
+        fac.autoCompleteSearchAsync("field_token_cap",
+                                    "a b c d e f g h i j k l m n o p q r s t u v w x y z 1 2 3 4 .",
+                                    null, previousResult, {
+                                        onSearchCompletion : function(aResults) {
+                                            do_check_eq(aResults.matchCount, 0,
+                                                        "All search tokens should be used with " +
+                                                        "previous results");
+                                            run_next_test();
+                                        }
+                                    });
+    }
+
+    do_log_info("Check that the number of tokens used in a search is capped to MAX_SEARCH_TOKENS " +
+                "for performance when querying the DB");
+    let changes = [ ];
+    changes.push({ op : "add", fieldname: "field_token_cap",
+                   // value with 36 unique tokens
+                   value: "a b c d e f g h i j k l m n o p q r s t u v w x y z 1 2 3 4 5 6 7 8 9 0",
+                   timesUsed: 1, firstUsed: 0, lastUsed: 0 });
+    updateFormHistory(changes, () => {
+        // Search for a string where the first 30 tokens match the value above but the 31st does not
+        // (which would prevent the result from being returned if the 31st term was used).
+        fac.autoCompleteSearchAsync("field_token_cap",
+                                    "a b c d e f g h i j k l m n o p q r s t u v w x y z 1 2 3 4 .",
+                                    null, null, {
+                                        onSearchCompletion : function(aResults) {
+                                            do_check_eq(aResults.matchCount, 1,
+                                                        "Only the first MAX_SEARCH_TOKENS tokens " +
+                                                        "should be used for DB queries");
+                                            test_token_limit_previousResult(aResults);
+                                        }
+        });
+    });
+});