2nd attempt at gracefully dealing with sort columns that contain

nulls.  This time around, do a query to determine whether or not the
sort column has nulls in it.  If it doesn't, then use our comparators
as before.

There are NULLs in the sort column, so we can't use MySQL comparators.
Fall back to iterating over every child row to get to the current one.
This can be wildly inefficient for really large albums, but it should
be a rare case that the user is sorting an album with null values in
the sort column.

Fixes #627

1 files changed, 45 insertions, 15 deletions

diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index 3cc9dd53..d1c6feb9 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -420,8 +420,15 @@ class Item_Model extends ORM_MPTT {
     }
     $db = Database::instance();
 
-    // We can't use isset() in this comparison because ORM::__get() confuses it.
-    if ($child->{$this->sort_column} !== null) {
+    // If the comparison column has NULLs in it, we can't use comparators on it and will have to
+    // deal with it the hard way.
+    $count = $db->from("items")
+      ->where("parent_id", $this->id)
+      ->where($this->sort_column, NULL)
+      ->count_records();
+
+    if (empty($count)) {
+      // There are no NULLs in the sort column, so we can just use it directly.
       $position = $db->query("
         SELECT COUNT(*) AS position FROM {items}
         WHERE `parent_id` = {$this->id}
@@ -443,21 +450,36 @@ class Item_Model extends ORM_MPTT {
           AND `{$this->sort_column}` = (SELECT `{$this->sort_column}`
                                         FROM {items} WHERE `id` = $child->id)
         ORDER BY `id` ASC");
+      foreach ($result as $row) {
+        $position++;
+        if ($row->id == $child->id) {
+          break;
+        }
+      }
     } else {
-      // If the sort value is null, then we can't take the approach of doing a comparison to get
-      // most of the way there.  We have to iterate the entire data set.
-      $position = 0;
-      $result = $db->query("
-        SELECT id FROM {items}
-        WHERE `parent_id` = {$this->id}
-          AND `{$this->sort_column}` IS NULL
-        ORDER BY `id` ASC");
-    }
+      // There are NULLs in the sort column, so we can't use MySQL comparators.  Fall back to
+      // iterating over every child row to get to the current one.  This can be wildly inefficient
+      // for really large albums, but it should be a rare case that the user is sorting an album
+      // with null values in the sort column.
+      //
+      // Reproduce the children() functionality here using Database directly to avoid loading the
+      // whole ORM for each row.
+      $orderby = array($this->sort_column => $this->sort_order);
+      // Use id as a tie breaker
+      if ($this->sort_column != "id") {
+        $orderby["id"] = "ASC";
+      }
 
-    foreach ($result as $row) {
-      $position++;
-      if ($row->id == $child->id) {
-        break;
+      $position = 0;
+      foreach ($db->select("id")
+               ->from("items")
+               ->where("parent_id", $this->id)
+               ->orderby($orderby)
+               ->get() as $row) {
+        $position++;
+        if ($row->id == $child->id) {
+          break;
+        }
       }
     }
 
@@ -564,6 +586,10 @@ class Item_Model extends ORM_MPTT {
   function children($limit=null, $offset=0, $where=array(), $orderby=null) {
     if (empty($orderby)) {
       $orderby = array($this->sort_column => $this->sort_order);
+      // Use id as a tie breaker
+      if ($this->sort_column != "id") {
+        $orderby["id"] = "ASC";
+      }
     }
     return parent::children($limit, $offset, $where, $orderby);
   }
@@ -582,6 +608,10 @@ class Item_Model extends ORM_MPTT {
   function descendants($limit=null, $offset=0, $where=array(), $orderby=null) {
     if (empty($orderby)) {
       $orderby = array($this->sort_column => $this->sort_order);
+      // Use id as a tie breaker
+      if ($this->sort_column != "id") {
+        $orderby["id"] = "ASC";
+      }
     }
     return parent::descendants($limit, $offset, $where, $orderby);
   }