?
Referenced as Pull Request for: # 8563
avatar rysiekpl
rysiekpl
28 Oct 2015

This is an old issue that has never been properly solved. The original issue has been closed due to lack of testing.

Website I am running started having this issue, and we have verified that:

  • the issue exists in the newest Joomla (3.4.5),
  • the patch below solves the issue.

We are using this patch in production on a high-traffic website. All credits for the patch should go to Michal Michaláč, who seems to be the author of the original patch.

GitHub does not allow me to attach a patch, so here it is, pasted:

--- table.php.orig  2015-10-28 17:44:49.199224888 +0100
+++ table.php   2015-10-28 17:46:52.161204224 +0100
@@ -1367,6 +1367,11 @@
            throw new UnexpectedValueException(sprintf('%s does not support ordering.', get_class($this)));
        }

+        // Speedup by SQL optimalization
+        if ($this->_db->name == 'mysql')
+            return $this->reorderMysql($where);
+       
+        // Default (slow) reorder
        $k = $this->_tbl_key;

        // Get the primary keys and ordering values for the selection.
@@ -1408,6 +1413,47 @@
        return true;
    }

+
+    /**
+     * Method to compact the ordering values of rows in a group of rows
+     * defined by an SQL WHERE clause.
+     *
+     * @param   string  $where  WHERE clause to use for limiting the selection of rows to compact the ordering values.
+     *
+     * @return  mixed  Boolean  True on success.
+     *
+     * @link    http://docs.joomla.org/JTable/reorder
+     * @since   11.1
+     */
+    protected function reorderMysql($where = '')
+    {
+        $k = $this->_tbl_key;
+        
+        $this->_db->setQuery('set @num = 0');
+        $this->_db->execute();
+        
+        $query = $this->_db->getQuery(true)
+            ->update($this->_tbl)
+            ->set('ordering = @num := @num + 1')
+            ->where('ordering >= 0')
+            ->order('ordering');
+ 
+        // Setup the extra where and ordering clause data.
+        if ($where)
+        {
+            $query->where($where);
+        }
+        
+        // Warning: Unpatched version of JDatabaseQuery->__toString ignores 'order' to update query.
+        // Then query must be built from string like this:
+        //$query = "update {$this->_tbl} set ordering = @num := @num + 1 where ordering >= 0 " . $where? (" and " . $where): "" . " order by ordering";
+        
+        $this->_db->setQuery($query);
+        $this->_db->execute();
+ 
+        return true;
+    }
+
    /**
     * Method to move a row in the ordering sequence of a group of rows defined by an SQL WHERE clause.
     * Negative numbers move the row up in the sequence and positive numbers move it down.

Votes

# of Users Experiencing Issue
0/1
Average Importance Score
5.00

avatar rysiekpl rysiekpl - open - 28 Oct 2015
avatar zero-24
zero-24 - comment - 29 Oct 2015

Can you send your changes as pull request? So we can test, review and merge it easy? If you need help please see: https://docs.joomla.org/Using_the_Github_UI_to_Make_Pull_Requests


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8189.

avatar zero-24 zero-24 - change - 29 Oct 2015
Category Libraries
avatar zero-24 zero-24 - change - 29 Oct 2015
Labels Added: ?
avatar zero-24 zero-24 - change - 29 Oct 2015
Title
JTable->reorder is extremely slow
JTable->reorder is extremely slow
avatar sandrodz
sandrodz - comment - 10 Nov 2015

I can confirm, we are seeing same issue with 100,000 article DB, saving is extremely slow. Foreach doesn't cut it here.

patch works for me.

avatar rysiekpl rysiekpl - reference | 4d6a1a1 - 28 Nov 15
avatar rysiekpl
rysiekpl - comment - 28 Nov 2015

Pull request created.

avatar rysiekpl
rysiekpl - comment - 25 Jan 2016

It occurs to me that this has still not been fixed, after years in the wild, after submitting a patch, creating a pull request, and a long discussion about that very PR.

We are still affected by this, and we're applying the very patch submitted here every single time we upgrade Joomla. This is becoming a huge problem and a strong argument for migrating off of to something else.

Are there any plans to merge the PR or fix the bug in some other way?

avatar brianteeman
brianteeman - comment - 25 Jan 2016

Every PR needs at least two testers before it can be merged.
I dont see any PR from you only one from @alikon which has no tests

avatar rysiekpl
rysiekpl - comment - 25 Jan 2016

This is the PR: #8563

I thought it was linked here; there even was a discussion on the PR.

avatar alikon
alikon - comment - 26 Jan 2016

@rysiekpl can you test #8576 wich is not a mysql only solution

avatar gitcubefree
gitcubefree - comment - 7 Feb 2016

This solution did NOT seem to make a difference in J v3.4.8
We are also using Seblod v3.7.2
12000+ records in the articles
Our Assets table shows everything in sync using ACL Manager
It still takes about 23 seconds to save a new article.
Any other ideas or tests to speed things up?


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8189.

avatar Fedik
Fedik - comment - 8 Feb 2016

@gitcubefree do test without seblod, because they use some additional call of "reorder" ...

avatar alikon
alikon - comment - 8 Feb 2016

@gitcubefree can you test #8576

avatar aleksandar-todorovic
aleksandar-todorovic - comment - 9 Feb 2016

@alikon

@rysiekpl and I tested the patch #8576 and we're now able to verify that that patch works.

avatar wojsmol
wojsmol - comment - 9 Feb 2016
avatar brianteeman
brianteeman - comment - 11 Mar 2016

Closed as we have a PR #8576


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/8189.

avatar brianteeman brianteeman - change - 11 Mar 2016
Status New Closed
Closed_Date 0000-00-00 00:00:00 2016-03-11 22:36:57
Closed_By brianteeman
avatar brianteeman brianteeman - close - 11 Mar 2016

Add a Comment

Login with GitHub to post a comment