Ticket #778 (closed Bug: fixed)

Opened 3 months ago

Last modified 2 months ago

[patch proposal] faulty orderby with join

Reported by: Steffen Owned by: Shadowhand
Priority: major Milestone: 2.2.1
Component: Libraries:ORM Version: SVN HEAD
Keywords: Cc:

Description

Assuming these models:

class Article_Model extends ORM {
  protected $table_name = 'articles';
  protected $has_many = array('authors' => 'article_authors');
}

class Author_Model extends ORM {
  protected $table_name = 'authors';
  protected $belongs_to = array('article');
}

class Article_author_Model extends ORM {
  protected $table_name = 'article_authors';
  protected $has_one = array('author', 'article');
}

and somewhere in the code:

$articles = $article_model->where("title = 'Test'")->find_all();

foreach ($articles as $article) {
  echo $article->title.'<br/>';
  foreach($article->authors as $author) {
    echo 'Author: '.$author->name.'<br/>';
  }
}

The generated sql for the authors looks like this:

SELECT `authors`.* FROM authors JOIN (`article_authors`) ON (`article_authors`.`author_id` = `authors`.`id`) WHERE `article_authors`.`article_id` = 1 ORDER BY `id` ASC

whereas it should better look like:

SELECT `authors`.* FROM authors JOIN (`article_authors`) ON (`article_authors`.`author_id` = `authors`.`id`) WHERE `article_authors`.`article_id` = 1 ORDER BY `authors`.`id` ASC

In the current version the query fails in Pdosqlite (part because of "JOIN (article_authors) ON" but I'll open another ticket for this) but mainly because it doesn't know which id column to use for sorting.

I added a patch proposal which hopefully doesn't break BC.

Patch Database.php:

--- Database.php
+++ (clipboard)
@@ -662,14 +662,21 @@
 	 * @param   string        direction of the order
 	 * @return  Database_Core        This Database object.
 	 */
-	public function orderby($orderby, $direction = NULL)
+	public function orderby($orderby, $direction = NULL, $table = NULL)
 	{
 		if ( ! is_array($orderby))
 		{
 			$orderby = array($orderby => $direction);
 		}
 
-		foreach ($orderby as $column => $direction)
+		if($table !== NULL)
+		{
+		  $orderby_table = $this->driver->escape_table($table).'.';
+		} else {
+		  $orderby_table = '';
+		}
+		
+	  foreach ($orderby as $column => $direction)
 		{
 			$direction = strtoupper(trim($direction));
 
@@ -677,8 +684,8 @@
 			{
 				$direction = 'ASC';
 			}
-
-			$this->orderby[] = $this->driver->escape_column($column).' '.$direction;
+			
+			$this->orderby[] = $orderby_table.$this->driver->escape_column($column).' '.$direction;
 		}
 
 		return $this;

Patch ORM.php:

--- ORM.php
+++ (clipboard)
@@ -479,7 +479,7 @@
 		if ( ! isset($this->db_applied['orderby']))
 		{
 			// Apply sorting
-			$this->db->orderby($this->sorting);
+			$this->db->orderby($this->sorting, NULL, $this->table_name);
 		}
 
 		return $this->load_result(TRUE);

Change History

Changed 2 months ago by Shadowhand

  • status changed from new to closed
  • resolution set to fixed

Fixed in r3445, merged in r3446.

Changed 2 months ago by Shadowhand

  • version changed from 2.2 Release to SVN HEAD
Note: See TracTickets for help on using tickets.