Bug 281582: Replace CONCAT and MATCH with Bugzilla::DB function call (Note: This patch also creates a database-agnostic fulltext search method.)

Patch By Tomas Kopal <Tomas.Kopal@altap.cz> r=mkanat, a=myk

4 files changed, 91 insertions, 17 deletions

diff --git a/Bugzilla/DB.pm b/Bugzilla/DB.pm
index 326e6cf63..1dccf535a 100644
--- a/Bugzilla/DB.pm
+++ b/Bugzilla/DB.pm
@@ -175,10 +175,10 @@ sub _handle_error {
 }
 
 # List of abstract methods we are checking the derived class implements
-our @_abstract_methods = qw(new sql_regexp sql_not_regexp sql_limit
-                            sql_to_days sql_date_format sql_interval
-                            bz_lock_tables bz_unlock_tables
-                            REQUIRED_VERSION PROGRAM_NAME);
+our @_abstract_methods = qw(REQUIRED_VERSION PROGRAM_NAME
+                            new sql_regexp sql_not_regexp sql_limit sql_to_days
+                            sql_date_format sql_interval
+                            bz_lock_tables bz_unlock_tables);
 
 # This overriden import method will check implementation of inherited classes
 # for missing implementation of abstract methods
@@ -215,6 +215,35 @@ sub sql_position {
 # General Info Methods
 #####################################################################
 
+sub sql_string_concat {
+    my ($self, @params) = @_;
+    
+    return join(' || ', @params);
+}
+
+sub sql_fulltext_search {
+    my ($self, $column, $text) = @_;
+
+    # This is as close as we can get to doing full text search using
+    # standard ANSI SQL, without real full text search support. DB specific
+    # modules shoud override this, as this will be always much slower.
+
+    # the text is already sql-quoted, so we need to remove the quotes first
+    my $quote = substr($self->quote(''), 0, 1);
+    $text = $1 if ($text =~ /^$quote(.*)$quote$/);
+
+    # make the string lowercase to do case insensitive search
+    my $lower_text = lc($text);
+
+    # split the text we search for to separate words
+    my @words = split(/\s+/, $lower_text);
+
+    # search for occurence of all specified words in the column
+    return "CASE WHEN (LOWER($column) LIKE ${quote}%" .
+           join("%${quote} AND LOWER($column) LIKE ${quote}%", @words) .
+           "%${quote}) THEN 1 ELSE 0 END";
+}
+
 # XXX - Needs to be documented.
 sub bz_server_version {
     my ($self) = @_;
@@ -714,6 +743,26 @@ formatted SQL command have prefix C<sql_>. All other methods have prefix C<bz_>.
               $text = the text to search (scalar)
  Returns:     formatted SQL for substring search (scalar)
 
+=item C<sql_string_concat>
+
+ Description: Returns SQL syntax for concatenating multiple strings (constants
+              or values from table columns) together.
+ Params:      @params = array of column names or strings to concatenate
+ Returns:     formatted SQL for concatenating specified strings
+
+=item C<sql_fulltext_search>
+
+ Description: Returns SQL syntax for performing a full text search for
+              specified text on a given column.
+              There is a ANSI SQL version of this method implemented using
+              LIKE operator, but it's not a real full text search. DB specific
+              modules shoud override this, as this generic implementation will
+              be always much slower. This generic implementation returns
+              'relevance' as 0 for no match, or 1 for a match.
+ Params:      $column = name of column to search (scalar)
+              $text = text to search for (scalar)
+ Returns:     formatted SQL for for full text search
+
 =item C<bz_lock_tables>
 
  Description: Performs a table lock operation on specified tables.
diff --git a/Bugzilla/DB/Mysql.pm b/Bugzilla/DB/Mysql.pm
index ae24d13bc..76cd0966d 100644
--- a/Bugzilla/DB/Mysql.pm
+++ b/Bugzilla/DB/Mysql.pm
@@ -88,7 +88,7 @@ sub sql_not_regexp {
 }
 
 sub sql_limit {
-    my ($self, $limit,$offset) = @_;
+    my ($self, $limit, $offset) = @_;
 
     if (defined($offset)) {
         return "LIMIT $offset, $limit";
@@ -97,6 +97,18 @@ sub sql_limit {
     }
 }
 
+sub sql_string_concat {
+    my ($self, @params) = @_;
+    
+    return 'CONCAT(' . join(', ', @params) . ')';
+}
+
+sub sql_fulltext_search {
+    my ($self, $column, $text) = @_;
+
+    return "MATCH($column) AGAINST($text)";
+}
+
 sub sql_to_days {
     my ($self, $date) = @_;
 
diff --git a/Bugzilla/DB/Pg.pm b/Bugzilla/DB/Pg.pm
index 9766da383..5963f5308 100644
--- a/Bugzilla/DB/Pg.pm
+++ b/Bugzilla/DB/Pg.pm
@@ -93,7 +93,7 @@ sub sql_not_regexp {
 }
 
 sub sql_limit {
-    my ($self, $limit,$offset) = @_;
+    my ($self, $limit, $offset) = @_;
 
     if (defined($offset)) {
         return "LIMIT $limit OFFSET $offset";
@@ -131,6 +131,15 @@ sub sql_interval {
     return "INTERVAL '$interval'";
 }
 
+sub sql_string_concat {
+    my ($self, @params) = @_;
+    
+    # Postgres 7.3 does not support concatenating of different types, so we
+    # need to cast both parameters to text. Version 7.4 seems to handle this
+    # properly, so when we stop support 7.3, this can be removed.
+    return 'CAST(' . join(' AS text) || CAST(', @params) . ' AS text)';
+}
+
 sub bz_lock_tables {
     my ($self, @tables) = @_;
    
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 775778a9d..c38d07054 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -545,14 +545,14 @@ sub init {
              # "content" is an alias for columns containing text for which we
              # can search a full-text index and retrieve results by relevance, 
              # currently just bug comments (and summaries to some degree).
-             # There's only one way to search a full-text index
-             # ("MATCH (...) AGAINST (...)"), so we only accept the "matches"
-             # operator, which is specific to full-text index searches.
+             # There's only one way to search a full-text index, so we only
+             # accept the "matches" operator, which is specific to full-text
+             # index searches.
 
              # Add the longdescs table to the query so we can search comments.
              my $table = "longdescs_$chartid";
-             push(@supptables, "INNER JOIN longdescs $table ON bugs.bug_id " . 
-                               "= $table.bug_id");
+             push(@supptables, "INNER JOIN longdescs AS $table " .
+                               "ON bugs.bug_id = $table.bug_id");
              if (Param("insidergroup") 
                  && !&::UserInGroup(Param("insidergroup")))
              {
@@ -563,11 +563,13 @@ sub init {
              # $term1 searches comments.
              # $term2 searches summaries, which contributes to the relevance
              # ranking in SELECT but doesn't limit which bugs get retrieved.
-             my $term1 = "MATCH($table.thetext) AGAINST(".&::SqlQuote($v).")";
-             my $term2 = "MATCH(bugs.short_desc) AGAINST(".&::SqlQuote($v).")";
+             my $term1 = $dbh->sql_fulltext_search("${table}.thetext",
+                                                   ::SqlQuote($v));
+             my $term2 = $dbh->sql_fulltext_search("bugs.short_desc",
+                                                   ::SqlQuote($v));
 
              # The term to use in the WHERE clause.
-             $term = $term1;
+             $term = "$term1 > 0";
 
              # In order to sort by relevance (in case the user requests it),
              # we SELECT the relevance value and give it an alias so we can
@@ -770,9 +772,11 @@ sub init {
              push(@supptables, "LEFT JOIN flagtypes $flagtypes " . 
                                "ON $flags.type_id = $flagtypes.id");
              
-             # Generate the condition by running the operator-specific function.
-             # Afterwards the condition resides in the global $term variable.
-             $ff = "CONCAT($flagtypes.name, $flags.status)";
+             # Generate the condition by running the operator-specific
+             # function. Afterwards the condition resides in the global $term
+             # variable.
+             $ff = $dbh->sql_string_concat("${flagtypes}.name",
+                                           "$flags.status");
              &{$funcsbykey{",$t"}};
              
              # If this is a negative condition (f.e. flag isn't "review+"),