From 48a34eced28d82b77eb840d05702daffb8e3ddbb Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 24 Nov 2016 09:51:00 -0800
Subject: [PATCH] Prepare for InnoDB FULLTEXT support

Summary:
Ref T11741. This makes everything work if we switch to InnoDB, but never actually switches yet.

Since the default minimum word length (3) and stopword list (36 common English words) in InnoDB are generally pretty reasonable, I just didn't add any setup advice for them. I figure we're better off with simpler setup until we identify some real problem that the builtin stopwords create.

Test Plan: Swapped the `false` to `true`, ran `storage adjust`, got InnoDB fulltext indexes, searched for stuff, got default "AND" behavior.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11741

Differential Revision: https://secure.phabricator.com/D16942
---
 .../check/PhabricatorMySQLSetupCheck.php      | 30 ++++++++++++++++---
 .../schema/PhabricatorConfigSchemaSpec.php    |  7 ++++-
 .../document/PhabricatorSearchDocument.php    | 20 +++++++++----
 3 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/src/applications/config/check/PhabricatorMySQLSetupCheck.php b/src/applications/config/check/PhabricatorMySQLSetupCheck.php
index d2318abcdd..152af61bf4 100644
--- a/src/applications/config/check/PhabricatorMySQLSetupCheck.php
+++ b/src/applications/config/check/PhabricatorMySQLSetupCheck.php
@@ -121,8 +121,18 @@ final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck {
         ->addMySQLConfig('sql_mode');
     }
 
-    $stopword_file = $ref->loadRawMySQLConfigValue('ft_stopword_file');
+    $is_innodb_fulltext = false;
+    $is_myisam_fulltext = false;
     if ($this->shouldUseMySQLSearchEngine()) {
+      if (PhabricatorSearchDocument::isInnoDBFulltextEngineAvailable()) {
+        $is_innodb_fulltext = true;
+      } else {
+        $is_myisam_fulltext = true;
+      }
+    }
+
+    if ($is_myisam_fulltext) {
+      $stopword_file = $ref->loadRawMySQLConfigValue('ft_stopword_file');
       if ($stopword_file === null) {
         $summary = pht(
           'Your version of MySQL (on database host "%s") does not support '.
@@ -200,9 +210,9 @@ final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck {
       }
     }
 
-    $min_len = $ref->loadRawMySQLConfigValue('ft_min_word_len');
-    if ($min_len >= 4) {
-      if ($this->shouldUseMySQLSearchEngine()) {
+    if ($is_myisam_fulltext) {
+      $min_len = $ref->loadRawMySQLConfigValue('ft_min_word_len');
+      if ($min_len >= 4) {
         $namespace = PhabricatorEnv::getEnvConfig('storage.default-namespace');
 
         $summary = pht(
@@ -248,6 +258,18 @@ final class PhabricatorMySQLSetupCheck extends PhabricatorSetupCheck {
       }
     }
 
+    // NOTE: The default value of "innodb_ft_min_token_size" is 3, which is
+    // a reasonable value, so we do not warn about it: if it is set to
+    // something else, the user adjusted it on their own.
+
+    // NOTE: We populate a stopwords table at "phabricator_search.stopwords",
+    // but the default InnoDB stopword list is pretty reasonable (36 words,
+    // versus 500+ in MyISAM). Just use the builtin list until we run into
+    // concrete issues with it. Users can switch to our stopword table with:
+    //
+    // [mysqld]
+    //   innodb_ft_server_stopword_table = phabricator_search/stopwords
+
     $innodb_pool = $ref->loadRawMySQLConfigValue('innodb_buffer_pool_size');
     $innodb_bytes = phutil_parse_bytes($innodb_pool);
     $innodb_readable = phutil_format_bytes($innodb_bytes);
diff --git a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php
index d808f76657..49a99ab03a 100644
--- a/src/applications/config/schema/PhabricatorConfigSchemaSpec.php
+++ b/src/applications/config/schema/PhabricatorConfigSchemaSpec.php
@@ -63,7 +63,12 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
     $database = $this->getDatabase($database_name);
 
     $table = $this->newTable($table_name);
-    $fulltext_engine = 'MyISAM';
+
+    if (PhabricatorSearchDocument::isInnoDBFulltextEngineAvailable()) {
+      $fulltext_engine = 'InnoDB';
+    } else {
+      $fulltext_engine = 'MyISAM';
+    }
 
     foreach ($columns as $name => $type) {
       if ($type === null) {
diff --git a/src/applications/search/storage/document/PhabricatorSearchDocument.php b/src/applications/search/storage/document/PhabricatorSearchDocument.php
index 1016872572..aee47cb9d3 100644
--- a/src/applications/search/storage/document/PhabricatorSearchDocument.php
+++ b/src/applications/search/storage/document/PhabricatorSearchDocument.php
@@ -45,14 +45,24 @@ final class PhabricatorSearchDocument extends PhabricatorSearchDAO {
 
     $compiler = new PhutilSearchQueryCompiler();
 
-    $operators = queryfx_one(
-      $conn,
-      'SELECT @@ft_boolean_syntax AS syntax');
-    if ($operators) {
-      $compiler->setOperators($operators['syntax']);
+    if (self::isInnoDBFulltextEngineAvailable()) {
+      // The InnoDB fulltext boolean operators are always the same as the
+      // default MyISAM operators, so we do not need to adjust the compiler.
+    } else {
+      $operators = queryfx_one(
+        $conn,
+        'SELECT @@ft_boolean_syntax AS syntax');
+      if ($operators) {
+        $compiler->setOperators($operators['syntax']);
+      }
     }
 
     return $compiler;
   }
 
+  public static function isInnoDBFulltextEngineAvailable() {
+    // For now, never consider this engine to be available.
+    return false;
+  }
+
 }