Bug 552919: Sort group_concat results so that they sort correctly for buglists

r=mkanat, a=mkanat (module owner)

5 files changed, 37 insertions, 16 deletions

diff --git a/Bugzilla/DB/Mysql.pm b/Bugzilla/DB/Mysql.pm
index 4b90a2a34..7f3eb2ef8 100644
--- a/Bugzilla/DB/Mysql.pm
+++ b/Bugzilla/DB/Mysql.pm
@@ -126,12 +126,15 @@ sub bz_last_key {
 }
 
 sub sql_group_concat {
-    my ($self, $column, $separator) = @_;
-    my $sep_sql;
-    if ($separator) {
-        $sep_sql = " SEPARATOR $separator";
+    my ($self, $column, $separator, $sort) = @_;
+    $separator = $self->quote(', ') if !defined $separator;
+    $sort = 1 if !defined $sort;
+    if ($sort) {
+        my $sort_order = $column;
+        $sort_order =~ s/^DISTINCT\s+//i;
+        $column = "$column ORDER BY $sort_order";
     }
-    return "GROUP_CONCAT($column$sep_sql)";
+    return "GROUP_CONCAT($column SEPARATOR $separator)";
 }
 
 sub sql_regexp {
diff --git a/Bugzilla/DB/Oracle.pm b/Bugzilla/DB/Oracle.pm
index a671a0e68..0819bd19a 100644
--- a/Bugzilla/DB/Oracle.pm
+++ b/Bugzilla/DB/Oracle.pm
@@ -119,7 +119,7 @@ sub bz_explain {
 
 sub sql_group_concat {
     my ($self, $text, $separator) = @_;
-    $separator ||= "','";
+    $separator = $self->quote(', ') if !defined $separator;
     return "group_concat(T_CLOB_DELIM($text, $separator))";
 }
 
diff --git a/Bugzilla/DB/Pg.pm b/Bugzilla/DB/Pg.pm
index c85c1d710..018900129 100644
--- a/Bugzilla/DB/Pg.pm
+++ b/Bugzilla/DB/Pg.pm
@@ -98,9 +98,14 @@ sub bz_last_key {
 }
 
 sub sql_group_concat {
-    my ($self, $text, $separator) = @_;
-    $separator ||= "','";
-    return "array_to_string(array_accum($text), $separator)";
+    my ($self, $text, $separator, $sort) = @_;
+    $sort = 1 if !defined $sort;
+    $separator = $self->quote(', ') if !defined $separator;
+    my $sql = "array_accum($text)";
+    if ($sort) {
+        $sql = "array_sort($sql)";
+    }
+    return "array_to_string($sql, $separator)";
 }
 
 sub sql_istring {
@@ -224,6 +229,20 @@ sub bz_setup_database {
                    )");
     }
 
+   $self->do(<<'END');
+CREATE OR REPLACE FUNCTION array_sort(ANYARRAY)
+RETURNS ANYARRAY LANGUAGE SQL
+IMMUTABLE STRICT
+AS $$
+SELECT ARRAY(
+    SELECT $1[s.i] AS each_item
+    FROM
+        generate_series(array_lower($1,1), array_upper($1,1)) AS s(i)
+    ORDER BY each_item
+);
+$$;
+END
+
     # PostgreSQL doesn't like having *any* index on the thetext
     # field, because it can't have index data longer than 2770
     # characters on that field.
diff --git a/Bugzilla/Install/DB.pm b/Bugzilla/Install/DB.pm
index ada500c58..d9a36b75b 100644
--- a/Bugzilla/Install/DB.pm
+++ b/Bugzilla/Install/DB.pm
@@ -3198,8 +3198,8 @@ sub _populate_bugs_fulltext {
             q{INSERT INTO bugs_fulltext (bug_id, short_desc, comments, 
                                          comments_noprivate)
                    SELECT bugs.bug_id, bugs.short_desc, }
-                 . $dbh->sql_group_concat('longdescs.thetext', $newline)
-          . ', ' . $dbh->sql_group_concat('nopriv.thetext',    $newline) .
+                 . $dbh->sql_group_concat('longdescs.thetext', $newline, 0)
+          . ', ' . $dbh->sql_group_concat('nopriv.thetext',    $newline, 0) .
                  qq{ FROM bugs 
                           LEFT JOIN longdescs
                                  ON bugs.bug_id = longdescs.bug_id
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index 61f5e995e..ad8ab0edb 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -308,10 +308,10 @@ sub COLUMNS {
                    . " * ($actual_time / ($actual_time + bugs.remaining_time))"
               . " END)",
 
-        'flagtypes.name' => $dbh->sql_group_concat('DISTINCT '
-                            . $dbh->sql_string_concat('flagtypes.name', 'flags.status'), "', '"),
+        'flagtypes.name' => $dbh->sql_group_concat('DISTINCT ' 
+            . $dbh->sql_string_concat('flagtypes.name', 'flags.status')),
 
-        'keywords' => $dbh->sql_group_concat('DISTINCT keyworddefs.name', "', '"),
+        'keywords' => $dbh->sql_group_concat('DISTINCT keyworddefs.name'),
     );
 
     # Backward-compatibility for old field names. Goes new_name => old_name.
@@ -354,8 +354,7 @@ sub COLUMNS {
         }
         elsif ($field->type == FIELD_TYPE_MULTI_SELECT) {
             $sql = $dbh->sql_group_concat(
-                'DISTINCT map_bug_' . $field->name . '.value',
-                $dbh->quote(', '));
+                'DISTINCT map_bug_' . $field->name . '.value');
         }
         else {
             $sql = 'bugs.' . $field->name;