bug #967610, double column sort with JOIN

ChangeLog

@@ -9,6 +9,8 @@ $Source$
     * Documentation.html: typos and XHTML validity, thanks to Cedric Corazza
     * libraries/export/sql.php: bug #1039639: under mysqli, some field types
       were wrongly exported as binary
+    * libraries/sqlparser.lib.php, /display_tbl.lib.php: bug #967610, double
+      column sort with JOIN 
 
 2004-10-11 Michal Čihař  <michal@cihar.com>
     * tbl_query_box.php: Don't try to replace %t and %f when table name is empty.

libraries/display_tbl.lib.php

@@ -452,42 +452,29 @@ function PMA_displayTableHeaders(&$is_display, &$fields_meta, $fields_cnt = 0, $
         $analyzed_sql = array();
     }
 
-    // can be result sorted?
+    // can the result be sorted?
     if ($is_display['sort_lnk'] == '1') {
 
         // Just as fallback
         $unsorted_sql_query     = $sql_query;
-
+        if (isset($analyzed_sql[0]['unsorted_query'])) {
-        // sorting by indexes, only if it makes sense
+            $unsorted_sql_query = $analyzed_sql[0]['unsorted_query'];
-        if (isset($analyzed_sql) && isset($analyzed_sql[0]) &&
-            isset($analyzed_sql[0]['querytype']) && $analyzed_sql[0]['querytype'] == 'SELECT' &&
-            isset($analyzed_sql[0]['table_ref']) && count($analyzed_sql[0]['table_ref']) == 1) {
-
-            $unsorted_sql_query = 'SELECT ';
-            if (isset($analyzed_sql[0]['queryflags']['distinct'])) {
-                $unsorted_sql_query .= ' DISTINCT ';
-            }
-            $unsorted_sql_query .= $analyzed_sql[0]['select_expr_clause'];
-            if (!empty($analyzed_sql[0]['from_clause'])) {
-                $unsorted_sql_query .= ' FROM ' . $analyzed_sql[0]['from_clause'];
         }
   
-            if (!empty($analyzed_sql[0]['where_clause'])) {
+        // we need $sort_expression and $sort_expression_nodir
-                $unsorted_sql_query .= ' WHERE ' . $analyzed_sql[0]['where_clause'];
+        // even if there are many table references
-            }
-            if (!empty($analyzed_sql[0]['group_by_clause'])) {
-                $unsorted_sql_query .= ' GROUP BY ' . $analyzed_sql[0]['group_by_clause'];
-            }
-            if (!empty($analyzed_sql[0]['having_clause'])) {
-                $unsorted_sql_query .= ' HAVING ' . $analyzed_sql[0]['having_clause'];
-            }
 
         $sort_expression = trim(str_replace('  ', ' ',$analyzed_sql[0]['order_by_clause']));
 
-            // Get rid of ASC|DESC
+        // Get rid of ASC|DESC (TODO: analyzer)
         preg_match('@(.*)([[:space:]]*(ASC|DESC))@si',$sort_expression,$matches);
         $sort_expression_nodir = isset($matches[1]) ? trim($matches[1]) : $sort_expression;
 
+        // sorting by indexes, only if it makes sense (only one table ref)
+        if (isset($analyzed_sql) && isset($analyzed_sql[0]) &&
+            isset($analyzed_sql[0]['querytype']) && $analyzed_sql[0]['querytype'] == 'SELECT' &&
+            isset($analyzed_sql[0]['table_ref']) && count($analyzed_sql[0]['table_ref']) == 1) {
+
             // grab indexes data:
             PMA_DBI_select_db($db);
 
@@ -513,7 +500,7 @@ function PMA_displayTableHeaders(&$is_display, &$fields_meta, $fields_cnt = 0, $
                 if (isset($row['Cardinality'])) {
                     $indexes_info[$row['Key_name']]['Cardinality'] = $row['Cardinality'];
                 }
-            //    I don't know what does following column mean....
+            //    I don't know what does the following column mean....
             //    $indexes_info[$row['Key_name']]['Packed']          = $row['Packed'];
                 $indexes_info[$row['Key_name']]['Comment']         = (isset($row['Comment']))
                                                                    ? $row['Comment']
@@ -714,7 +701,9 @@ function PMA_displayTableHeaders(&$is_display, &$fields_meta, $fields_cnt = 0, $
     }
 
     if ($is_display['sort_lnk'] == '1') {
-        $is_join = preg_match('@(.*)[[:space:]]+FROM[[:space:]]+.*[[:space:]]+JOIN@i', $sql_query, $select_stt);
+        //$is_join = preg_match('@(.*)[[:space:]]+FROM[[:space:]]+.*[[:space:]]+JOIN@im', $sql_query, $select_stt);
+        $is_join = (isset($analyzed_sql[0]['queryflags']['join']) ?TRUE:FALSE);
+        $select_expr = $analyzed_sql[0]['select_expr_clause'];
     } else {
         $is_join = FALSE;
     }
@@ -763,12 +752,14 @@ function PMA_displayTableHeaders(&$is_display, &$fields_meta, $fields_cnt = 0, $
             //       FROM `PMA_relation` AS `1` , `PMA_relation` AS `2`
 
             if (($is_join
-                && !preg_match('~([^[:space:],]|`[^`]`)[[:space:]]+(as[[:space:]]+)?' . $fields_meta[$i]->name . '~i', $select_stt[1], $parts))
+                //&& !preg_match('~([^[:space:],]|`[^`]`)[[:space:]]+(as[[:space:]]+)?' . $fields_meta[$i]->name . '~i', $select_stt[1], $parts))
+                && !preg_match('~([^[:space:],]|`[^`]`)[[:space:]]+(as[[:space:]]+)?' . $fields_meta[$i]->name . '~i', $select_expr, $parts))
                || ( isset($analyzed_sql[0]['select_expr'][$i]['expr'])
                    && isset($analyzed_sql[0]['select_expr'][$i]['column'])
                    && $analyzed_sql[0]['select_expr'][$i]['expr'] !=
                    $analyzed_sql[0]['select_expr'][$i]['column']
                   && !empty($fields_meta[$i]->table)) ) {
+                //$sort_tbl = PMA_backquote($fields_meta[$i]->table) . '.';
                 $sort_tbl = PMA_backquote($fields_meta[$i]->table) . ' . ';
             } else {
                 $sort_tbl = '';

libraries/sqlparser.lib.php

@@ -698,6 +698,7 @@ if ($is_minimum_common == FALSE) {
             'having_clause'  => '',
             'where_clause'   => '',
             'where_clause_identifiers'   => array(),
+            'unsorted_query' => '',
             'queryflags'     => array(),
             'select_expr'    => array(),
             'table_ref'      => array(),
@@ -753,6 +754,7 @@ if ($is_minimum_common == FALSE) {
  * ['queryflags']['select_from'] = 1;  if this is a real SELECT...FROM
  * ['queryflags']['distinct'] = 1;     for a DISTINCT
  * ['queryflags']['union'] = 1;        for a UNION
+ * ['queryflags']['join'] = 1;         for a JOIN
  *
  * lem9:  query clauses
  *        -------------
@@ -765,9 +767,12 @@ if ($is_minimum_common == FALSE) {
  * ['having_clause']
  * ['where_clause']
  *
- * and the identifiers of the where clause are put into the array
+ * The identifiers of the WHERE clause are put into the array
  * ['where_clause_identifier']
  *
+ * For a SELECT, the whole query without the ORDER BY clause is put into
+ * ['unsorted_query']
+ *
  * lem9:   foreign keys
  *         ------------
  * The CREATE TABLE may contain FOREIGN KEY clauses, so they get
@@ -781,6 +786,8 @@ if ($is_minimum_common == FALSE) {
  *
  * The array index of the first SELECT we find. Will be used to
  * insert a SQL_CALC_FOUND_ROWS.
+ *
+ * End of description of analyzer results
  */
 
         // must be sorted
@@ -1235,13 +1242,14 @@ if ($is_minimum_common == FALSE) {
         $seen_reserved_word = FALSE;
         $seen_group = FALSE;
         $seen_order = FALSE;
-        $in_group_by = FALSE; // true when we are into the GROUP BY clause
+        $in_group_by = FALSE; // true when we are inside the GROUP BY clause
-        $in_order_by = FALSE; // true when we are into the ORDER BY clause
+        $in_order_by = FALSE; // true when we are inside the ORDER BY clause
-        $in_having = FALSE; // true when we are into the HAVING clause
+        $in_having = FALSE; // true when we are inside the HAVING clause
-        $in_select_expr = FALSE; // true when we are into the select expr clause
+        $in_select_expr = FALSE; // true when we are inside the select expr clause
-        $in_where = FALSE; // true when we are into the WHERE clause
+        $in_where = FALSE; // true when we are inside the WHERE clause
         $in_from = FALSE;
         $in_group_concat = FALSE;
+        $unsorted_query = '';
 
         for ($i = 0; $i < $size; $i++) {
 //DEBUG echo "trace loop2 <b>"  . $arr[$i]['data'] . "</b> (" . $arr[$i]['type'] . ")<br />";
@@ -1297,6 +1305,10 @@ if ($is_minimum_common == FALSE) {
                       $subresult['queryflags']['union'] = 1;
                }
 
+               if ($upper_data == 'JOIN') {
+                      $subresult['queryflags']['join'] = 1;
+               }
+
                // if this is a real SELECT...FROM
                if ($upper_data == 'FROM' && isset($subresult['queryflags']['select_from']) && $subresult['queryflags']['select_from'] == 1) {
                    $in_from = TRUE;
@@ -1431,6 +1443,13 @@ if ($is_minimum_common == FALSE) {
                }
            }
 
+           // FIXME: is it correct to always add $sep ?
+           if (isset($subresult['queryflags']['select_from'])
+             && $subresult['queryflags']['select_from'] == 1 
+             && !$seen_order) {
+               $unsorted_query .= $arr[$i]['data'] . $sep;
+           }
+
            // clear $upper_data for next iteration
            $upper_data='';
 
@@ -1594,6 +1613,9 @@ if ($is_minimum_common == FALSE) {
         if (isset($where_clause)) {
             $subresult['where_clause'] = $where_clause;
         }
+        if (isset($unsorted_query) && !empty($unsorted_query)) {
+            $subresult['unsorted_query'] = $unsorted_query;
+        }
         if (isset($where_clause_identifiers)) {
             $subresult['where_clause_identifiers'] = $where_clause_identifiers;
         }