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 159808 c62c26e3c73f1dab8b6f905740c267bbafb64991
parent 159807 3cafeb339face40e7712eb33df7cde97edb0e5c1
child 159809 d52f80876573cbb795ec49583aef28becdba508a
push id3908
push usermozilla@noorenberghe.ca
push dateWed, 11 Dec 2013 07:36:26 +0000
treeherderfx-team@166d27bf3b4b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdolske
bugs897074
milestone29.0a1
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);
+                                        }
+        });
+    });
+});