- #48239: Comment thread coding inefficient

3 files changed, 88 insertions, 62 deletions

diff --git a/database/updates.inc b/database/updates.inc
index b09442529..10c6c5aab 100644
--- a/database/updates.inc
+++ b/database/updates.inc
@@ -1586,3 +1586,33 @@ function system_update_171() {
   $ret[] = update_sql('DELETE FROM {users_roles} WHERE rid IN ('. DRUPAL_ANONYMOUS_RID. ', '. DRUPAL_AUTHENTICATED_RID. ')');
   return $ret;
 }
+
+function system_update_172() {
+  // Multi-part update
+  if (!isset($_SESSION['system_update_172'])) {
+    $_SESSION['system_update_172'] = 0;
+    $_SESSION['system_update_172_max'] = db_result(db_query('SELECT MAX(cid) FROM comments'));
+  }
+
+  include_once './modules/comment.module';
+
+  $limit = 20;
+  $result = db_query_range("SELECT cid, thread FROM {comments} WHERE cid > %d", $_SESSION['system_update_172'], 0, $limit);
+  while ($comment = db_fetch_object($result)) {
+    $_SESSION['system_update_172'] = $comment->cid;
+    $thread = explode('.', rtrim($comment->thread, '/'));
+    foreach ($thread as $i => $offset) {
+      // Decode old-style comment codes: 1,2,...,9,90,91,92,...,99,990,991,...
+      $thread[$i] = int2vancode((strlen($offset) - 1) * 10 + substr($offset, -1, 1));
+    }
+    $thread = implode('.', $thread) .'/';
+    db_query("UPDATE comments SET thread = '%s' WHERE cid = %d", $thread, $comment->cid);
+  }
+
+  if ($_SESSION['system_update_172'] == $_SESSION['system_update_172_max']) {
+    unset($_SESSION['system_update_172']);
+    unset($_SESSION['system_update_172_max']);
+    return array();
+  }
+  return array('#finished' => $_SESSION['system_update_172'] / $_SESSION['system_update_172_max']);
+}
diff --git a/modules/comment.module b/modules/comment.module
index bc498b193..b760b82a4 100644
--- a/modules/comment.module
+++ b/modules/comment.module
@@ -573,25 +573,8 @@ function comment_save($edit) {
           // Strip the "/" from the end of the thread.
           $max = rtrim($max, '/');
 
-          // Next, we increase this value by one.  Note that we can't
-          // use 1, 2, 3, ... 9, 10, 11 because we order by string and
-          // 10 would be right after 1.  We use 1, 2, 3, ..., 9, 91,
-          // 92, 93, ... instead.  Ugly but fast.
-          $decimals = (string) substr($max, 0, strlen($max) - 1);
-          $units = substr($max, -1, 1);
-          if ($units) {
-            $units++;
-          }
-          else {
-            $units = 1;
-          }
-
-          if ($units == 10) {
-            $units = '90';
-          }
-
           // Finally, build the thread field for this new comment.
-          $thread = $decimals . $units .'/';
+          $thread = int2vancode(vancode2int($max) + 1) .'/';
         }
         else {
           // This is comment with a parent comment: we increase
@@ -608,7 +591,7 @@ function comment_save($edit) {
 
           if ($max == '') {
             // First child of this parent.
-            $thread = $parent->thread .'.1/';
+            $thread = $parent->thread .'.'. int2vancode(0) .'/';
           }
           else {
             // Strip the "/" at the end of the thread.
@@ -619,19 +602,8 @@ function comment_save($edit) {
             $parent_depth = count(explode('.', $parent->thread));
             $last = $parts[$parent_depth];
 
-            // Next, we increase this value by one.  Note that we can't
-            // use 1, 2, 3, ... 9, 10, 11 because we order by string and
-            // 10 would be right after 1.  We use 1, 2, 3, ..., 9, 91,
-            // 92, 93, ... instead.  Ugly but fast.
-            $decimals = (string)substr($last, 0, strlen($last) - 1);
-            $units = substr($last, -1, 1);
-            $units++;
-            if ($units == 10) {
-              $units = '90';
-            }
-
             // Finally, build the thread field for this new comment.
-            $thread = $parent->thread .'.'. $decimals . $units .'/';
+            $thread = $parent->thread .'.'. int2vancode(vancode2int($last) + 1) .'/';
           }
         }
 
@@ -1717,3 +1689,29 @@ function comment_invoke_comment(&$comment, $op) {
   }
   return $return;
 }
+
+/**
+ * Generate vancode.
+ *
+ * Consists of a leading character indicating length, followed by N digits
+ * with a numerical value in base 36. Vancodes can be sorted as strings
+ * without messing up numerical order.
+ *
+ * It goes:
+ * 00, 01, 02, ..., 0y, 0z,
+ * 110, 111, ... , 1zy, 1zz,
+ * 2100, 2101, ..., 2zzy, 2zzz,
+ * 31000, 31001, ...
+ */
+function int2vancode($i = 0) {
+  $num = base_convert((int)$i, 10, 36);
+  $length = strlen($num);
+  return chr($length + ord('0') - 1) . $num;
+}
+
+/**
+ * Decode vancode back to an integer.
+ */
+function vancode2int($c = '00') {
+  return base_convert(substr($c, 1), 36, 10);
+}
diff --git a/modules/comment/comment.module b/modules/comment/comment.module
index bc498b193..b760b82a4 100644
--- a/modules/comment/comment.module
+++ b/modules/comment/comment.module
@@ -573,25 +573,8 @@ function comment_save($edit) {
           // Strip the "/" from the end of the thread.
           $max = rtrim($max, '/');
 
-          // Next, we increase this value by one.  Note that we can't
-          // use 1, 2, 3, ... 9, 10, 11 because we order by string and
-          // 10 would be right after 1.  We use 1, 2, 3, ..., 9, 91,
-          // 92, 93, ... instead.  Ugly but fast.
-          $decimals = (string) substr($max, 0, strlen($max) - 1);
-          $units = substr($max, -1, 1);
-          if ($units) {
-            $units++;
-          }
-          else {
-            $units = 1;
-          }
-
-          if ($units == 10) {
-            $units = '90';
-          }
-
           // Finally, build the thread field for this new comment.
-          $thread = $decimals . $units .'/';
+          $thread = int2vancode(vancode2int($max) + 1) .'/';
         }
         else {
           // This is comment with a parent comment: we increase
@@ -608,7 +591,7 @@ function comment_save($edit) {
 
           if ($max == '') {
             // First child of this parent.
-            $thread = $parent->thread .'.1/';
+            $thread = $parent->thread .'.'. int2vancode(0) .'/';
           }
           else {
             // Strip the "/" at the end of the thread.
@@ -619,19 +602,8 @@ function comment_save($edit) {
             $parent_depth = count(explode('.', $parent->thread));
             $last = $parts[$parent_depth];
 
-            // Next, we increase this value by one.  Note that we can't
-            // use 1, 2, 3, ... 9, 10, 11 because we order by string and
-            // 10 would be right after 1.  We use 1, 2, 3, ..., 9, 91,
-            // 92, 93, ... instead.  Ugly but fast.
-            $decimals = (string)substr($last, 0, strlen($last) - 1);
-            $units = substr($last, -1, 1);
-            $units++;
-            if ($units == 10) {
-              $units = '90';
-            }
-
             // Finally, build the thread field for this new comment.
-            $thread = $parent->thread .'.'. $decimals . $units .'/';
+            $thread = $parent->thread .'.'. int2vancode(vancode2int($last) + 1) .'/';
           }
         }
 
@@ -1717,3 +1689,29 @@ function comment_invoke_comment(&$comment, $op) {
   }
   return $return;
 }
+
+/**
+ * Generate vancode.
+ *
+ * Consists of a leading character indicating length, followed by N digits
+ * with a numerical value in base 36. Vancodes can be sorted as strings
+ * without messing up numerical order.
+ *
+ * It goes:
+ * 00, 01, 02, ..., 0y, 0z,
+ * 110, 111, ... , 1zy, 1zz,
+ * 2100, 2101, ..., 2zzy, 2zzz,
+ * 31000, 31001, ...
+ */
+function int2vancode($i = 0) {
+  $num = base_convert((int)$i, 10, 36);
+  $length = strlen($num);
+  return chr($length + ord('0') - 1) . $num;
+}
+
+/**
+ * Decode vancode back to an integer.
+ */
+function vancode2int($c = '00') {
+  return base_convert(substr($c, 1), 36, 10);
+}