SQLite workarounds:

• http://stackoverflow.com/questions/16847574/how-to-use-row-number-in-sqlite
• http://stackoverflow.com/questions/9151706/getting-row-number-for-query
• http://stackoverflow.com/questions/329197/cross-table-update-in-sqlite3

so, for what i understand this will work on all database drivers, except SQLLite
i can only test in mysql, but is this ready for testing?

There is missing a few unit tests for windowRowNumber but you can start testing.

ok will try to test today or tomorrow, but as said i can only test mysql.

I have found one issue with windowRowNumber on mysql database.
It can be used only once per query.
This is current limitation. I will try to find some solution for that.

I have found one issue with windowRowNumber on mysql database.
It can be used only once per query.
This is current limitation. I will try to find some solution for that.

JTable::reorder() does not need to call it more than once, right ?
but i understand that you want to extend it for more usage cases ?

Example of some query that will be created if it is called twice

SELECT *

    FROM
    (
        SELECT lang.lang_code
         ,(SELECT @a := @a + 1 FROM (SELECT @a := 0) AS row_init) as num1
         ,(SELECT @a := @a + 1 FROM (SELECT @a := 0) AS row_init) as num2
        FROM #__languages AS lang
        ORDER_BY _window_order_
    )
    AS window

ORDER BY _order_

1. about making the variable name @a to be unique
   you can add a static counter variable to the method, and use @a1 , @a2, @a3, etc

2. about window_order how are you going to handle multiple values for it ?

maybe it is best to throw an exception if someone tries to call it multiple times (throw because it is not implemented / supported thus prevent wrong usage of it, and no B/C issue, since the method is new)

• JTable::reorder() does not need to call it more than once, (right ?)
• in the future someone may implement multiple calls on it

maybe it is best to throw an exception if someone tries to call it multiple times (throw because it is not implemented / supported thus prevent wrong usage of it, and no B/C issue, since the method is new)

I tried to implement some code but I'm back to the same conclusion. I will add throw Exception.
Mysql query would require a very complicated query like SELECT * FROM ( SELECT ... ( SELECT ... ...`.

I added throw RunTimeException to prevent anyone to use that method twice in select query.

For more advanced usage I would have to write very complicated code for mysql but it is not the purpose of this PR.

Thanks for any comment and welcome to test.

I tested in MySQL and in Postgresql,
I am getting in both: ambiguous 'ordering' column

Maybe that is why the test units using are failing too ?
I have made a PR against your branch please accept it, it seems to be working after fixing the ambiguous column

I have merged it, thanks, but the query looks messing. I probably change $alias to $prefix_alias in order to apply it to another column partitiionBy.

Beside the ambigious column, after patching it, i had tested it in both MySQL and in PostgreSQL,
and it worked properly (reodering and new record creation too)

Is there something remaining besides "beautifying" the query ?

Again, I changed code a little, sorry.
One thing to resolve is to remove duplication of sqlite function from:
https://github.com/joomla/joomla-cms/pull/12839/files#diff-5163215d21663baf6fd81deead41c134R97
and replace it by native from JDatabaseDriver->connection.
[UPDATED]
Resolved.

@csthomas
is it currently possible to run Joomla on sqLite ? do you have a working installation ?

[EDIT]
i mean is the sqLite driver finished / complete ?

I have not tested.
I read only https://www.mrgazz.com/computers/computers-mainmenu-138/mambo-mainmenu-147/sqlite-support-in-joomla

I added support only because unittests require test on $table->reorder().
Without support travis won't be happy:)

I think it is complete (except unittest as I mentioned above but If you folks does not mind so it can be). - DONE
Today I'm happy with this and I do not want any changes.

I tested sqlite in linux console:

php -a
php > require 'console.php'; // Custom cli to load application
START
php > $db = JDatabaseDriver::getInstance(['driver' => 'sqlite', 'database' => 'joomladb.sqlite', 'prefix' => 'j25_']); // Initialize sqlite
php > $query = $db->getQuery(true)->update('#__content')->set('ordering = sq.new_ordering');
php > $sq->select($db->quoteName('id', 'pk__0'))->where('ordering >= 0');
php > $query->where('ordering >= 0')->innerJoin('(' . (string) $sq . ') AS sq ON sq.pk__0 = id');
php > print_r($db->setQuery($query)->execute()); // I have only a few columns in j25_content
PDOStatement Object
(
    [queryString] => INSERT OR REPLACE INTO j25_content (id,ordering,catid,subcatid)
SELECT id,sq.new_ordering,catid,subcatid
FROM j25_content
INNER JOIN (
SELECT w.*, ROW_NUMBER() AS new_ordering
FROM ( 
SELECT `id` AS `pk__0`
FROM j25_content
WHERE ordering >= 0
ORDER BY ordering ) AS w,
( SELECT ROW_NUMBER(0) ) AS r
ORDER BY NULL) AS sq ON sq.pk__0 = id
WHERE ordering >= 0
)

To reorder all articles in all categories by one query (mysql):

php > require 'console.php';
START
php > $db = JFactory::getDbo();
php > $sq = $db->getQuery(true)->from('#__content')->selectRowNumber('ordering ASC', 'new_ordering', 'catid', 'catidAlias'); // To reverse order replace 'ordering ASC' to 'ordering DESC'
php > $query = $db->getQuery(true)->update('#__content')->set('ordering = sq.new_ordering');
php > $sq->select($db->quoteName('id', 'pk__0'))->where('ordering >= 0 AND state >= 0');
php > $query->where('ordering >= 0 AND state >= 0')->innerJoin('(' . (string) $sq . ') AS sq ON sq.pk__0 = id');
php > echo $query;

UPDATE #__content
INNER JOIN (
SELECT catid AS catidAlias,(SELECT @rownum := IF(@group = CONCAT_WS(',', catid) OR ((@group := CONCAT_WS(',', catid)) AND 0), @rownum + 1, 1) FROM (SELECT @rownum := 0, @group := '') AS r) AS new_ordering,`id` AS `pk__0`
FROM #__content
WHERE ordering >= 0
ORDER BY catid,ordering) AS sq ON sq.pk__0 = id
SET ordering = sq.new_ordering
WHERE ordering >= 0 AND state >= 0
php > $db->setQuery($query)->execute(); // Should be fast enough:)
1
php > print_r($db->setQuery("SELECT catid, ordering, title,id FROM #__content ORDER BY catid,ordering")->loadAssocList());

i honestly currently don't have the db knowledge to understand all changes you made.

One thing i would like to know, what is the performance improvement here? i assume it's big, but do you have concrete examples?

For now I do not have any examples.

Can I ask someone with sqlsrv to apply this PR and create a new featured article and check results.
I have not tested it on sqlsrv.

maybe @waader can help

One thing i would like to know, what is the performance improvement here? i assume it's big, but do you have concrete examples?

probably 50x-200x faster when reordering 1,000 or 2,000 rows, i will test when i get some time at end of week

There is something wrong with travis, 312b4c0 should be market as failed.
https://travis-ci.org/joomla/joomla-cms/jobs/180022945

I have found a inconsistency with sorting a query result.
In postreSQL you could sort result by global order by, but in mysql and sqlite there was not option for that. I added it now.

$q = $db->getQuery(1)->select('id, ordering')->selectRowNumber('ordering', 'RNorderingRN')->from('#__content')->order('ordering DESC');

which means set row number in ordering ASC but return result rows in ordering DESC order.

After I committed 'a correct way to create connection for sqlite' in unittest 312b4c0 the tests failed (take a look deeper in travis logs to see errors) because of a few bugs in unittests.
Now it is fixed in 2163ec8

I tested this on MariaDB 10.0.27, php 7.0.8 on my laptop.

After patch reorder is almost 100x faster for 4000 articles in one category. Total articles ~233k.

If anyone thinking about testing, I say that PR is ready for testing.

I looked at https://downloads.joomla.org/technical-requirements and saw that minimum version of postgreSQL is 8.3.18
Whether it is possible to raise the minimum version of PostgreSQL to 8.4 for Joomla 3.7?

I have resolved a merge conflict.
I created a separate PR #13097 to fix only system tests.

Whether it is possible to raise the minimum version of PostgreSQL to 8.4 for Joomla 3.7?

At some point there was discussion to even drop support, the people that use PostgreSQL should at least have a non ancient version
8.4.0 was released on 2009-07-01
8.3.18 was released on 2012-02-27

about, sorry i have not (re) tested yet as i had promised, will do on weekend, besides mysql / postgresql, will try to test on SQL server too

I have tested this item ✅ successfully on 29ab2b9

Tested on both MySQL and PostgreSQL

I have tested in both articles and featured managers with

• drag and drop ordering
• manually saving of multiple ordering numbers (added CSS to enable display of the ordering numbers and showed the save button)

Also tested with 3rd party extension

@andrepereiradasilva I have not seen your code review because I done git rebase a short time after.
I excluded access.php file from this PR as I saw that there was a conflict.
I took a decision to not change that file because it is not directly related.

only tested with mysql. followed test instructions and works as described.

It is nice to see some PRs in this repository that try to squeeze some extra performance out some code
that can have some visible benefits

but this PR does not try to squeeze a little extra performance out of something
It tries to addresses a real performance issue with JTable reorder

that causes unacceptable performance and race conditions in large websites, when saving articles and when reordering articles and large menus

Also this PR involved some hard work and considerable time by @csthomas to add code and test all DBs, (even support MSSQL that currently noone uses), when no-one else would spend time to do a proper job (for all DBs) for years
as the reorder performance issue exists for years

and this PR achieves a 50x-100x speedup of JTable reorder in large websites (and is much more complete work for MySQL too, unlike previous PRs)

even my similar purpose PR #11184 , that involved quite some time by me, i will be very happy to close after / if this PR gets enough testing to be merged

• about a couple of API methods added by this PR , they are currently only used in JTable reorder(), so i argue some people to have less worries of breaking something in different places
• this PR (besides author) was tested by me and @andrepereiradasilva on MySQL and (besides author) by me on PostgreSQL

Author has also tested on MSSQL, and i almost did too, but i run into some problems installing and configuring SQL server, that delayed me, but i will find time to do it in the short term future to test MSQL too

I hope more people, can put this PR in their "testing / contributing list" e.g. @alikon

@csthomas, can you check and fix the conflicts ?

Fixed but the last issue is still PostgreSQL version from
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/database/drive/postgresql.php#L57

It has to be at least 8.4.0.
If this can be done by other PR then this one can be merged:)

There isn't an acceptable way to raise the minimum supported database engine version and block upgrades on an unsupported environment in any current stable release (somewhere there's a PR to add this support because it will be a requirement for 4.0 but it is definitely not in 3.6.5), so from that aspect you're out of luck trying to raise the minimum.

From a practical perspective, you might get lucky though. There are no PostgreSQL 8.3 installs in the stats server and the oldest reported install uses 8.4.2.

Is it worth more to work on this?
PLT folks, can I bump postgreSQL version to 8.4.2?

If not then I will close that PR.

PLT folks, can I bump postgreSQL version to 8.4.2?

If there are a couple of persons using a previous postgreSQL version, then they can upgrade,
but i do not think there is even one , lol

There has been a lot of testing of many PR that were doing almost nothing,
why this important PR does not have any testers ?

The car is runs ok, but it is burning oil, and we care more for polishing it ?
First get it to mechanic to fix the problem (=test this PR) and then care to polish it

Can this PR get a high priority label (i guess critical would be for a security issue)

I have updated PR.
I took a decision to rebase it.
Later it would be easier to merge PR with upcoming new changes.

PR need to be tested again.
I tested on mysql, postgresql and sql server.

I ll re-test during week

can I bump postgreSQL version to 8.4.2?
If not then I will close that PR.

Not for 3.x. However, it might be a candidate for Joomla!4.0, so the PR should be made against 4.0-dev instead of staging.

Not for 3.x. However, it might be a candidate for Joomla!4.0, so the PR should be made against 4.0-dev instead of staging.

@nibra

Stopping this PR for 1 or 2 non-existing postgreSQL user of 8.3.NN is acceptable ?

while saving an article in a category with 1,000 articles
and having it make 1000 update queries and cost 10 seconds

or if you have 5,000 articles,
and require 50 seconds to 100 seconds is acceptable ?

Race conditions in DB updating when re-ordering large menu is acceptable ? (this PR minimizes them)

People having to hack the JTable reorder (using PR(s) proposed here or not proposed here) to make it usable in large websites is acceptable ?

There were other changes made in J3.4 J3.5 J3.6 that broke unintentionally several sites

and this change will not even break any 8.3.NN postgreSQL sites because they don't exist,
and even if a few exist, the change added effects saving and re-ordering, a user can still have time to upgrade

or if it is possible (because of the way upgrade is done)
add the minimum DB version check not only to installation as it is now,
by to the Joomla upgrade too, and user knows that a DB upgrade is needed

Is there an option to check what version of postgreSQL is running and use a workaround for that?
Example:
http://www.postgresonline.com/journal/archives/79-Simulating-Row-Number-in-PostgreSQL-Pre-8.4.html

I don't think we can check the db version for the update. We only have checks in place for the PHP version. We added those since we had that B/C break due to a serious security issue where we raised the PHP minimum to 5.3.10.

Stopping this PR for 1 or 2 non-existing postgreSQL user of 8.3.NN is acceptable ?

Either back that statement with facts or don't use such a statement when you want to break peoples sites.
Fact is, even with the stats plugin we don't know for sure how many PostgreSQL installations there are. Certainly not many, but most likely more than the 1-2 non-existing you say.

Semantic versioning is very clear about this question. You can't do it in 3.x. You can do it for 4.0.

We use Semantic Versioning, so BC break within the 3.x series is not acceptable. To get this improvement into 3,x, it will need a version switch providing the improvement for 8.4+, and keeping the old behaviour for pgsql <=8.4. There is no other option.

would be great to have it for 3.x but rules are rules and there is not other way as to make this change against the 4.0 branch

FWIW, the general argument outside the Joomla community is that semantic versioning only applies to the exposed API; it does NOT apply to the minimum supported version of a dependency (otherwise one could argue our updates of third party libraries are backward compatibility breaks as we in many cases also make changes which take advantage of the new version's features or have bumped a minimum dependency to ensure packages which weren't PHP 7 compatible weren't installed or to block packages with security vulnerabilities). While raising a minimum for more visible things like PHP and database versions is usually more painful than raising the minimum for the Registry or PHPMailer packages, the act of raising a minimum in and of itself is generally not a B/C break.

There is also a practical argument to be made here, given by this screenshot:

There are ZERO reported PostgreSQL 8.3 users in our statistics database. This may not be a 100% accurate statement due to the optional nature of sending the data, but this argument is now hitting a point where there's a practical side (nobody is using it, at least per the data the project has been collecting for a year now) and a procedural side.

As pointed out, until #12355 is in production (so 3.7) there isn't a way to check and block the upgrade if it were decided to allow it. So that does make it harder to just say yes.

imho we are still missing the main point:
is insane to update/reorder all records when a new article is created
whatever clever/complicated query we can write
we are try to circumvent the real problem

Without changing the way the data is stored and processed (which would be a more painful B/C break to deal with), what can you do about that?

@alikon

JTable::reorder should never use 1 SQL query per updated row, regardless of where it is called

• now about new article saving, author of this PR has made a solution for this case too, that is B/C but uses large numbers,

but after this PR is accepted, updating a category with 10,000 records will be have JTable::reorder() cost of e.g. 1 second instead of 120-200 seconds,

finally code added by this PR , i suspect will be useful in other places too !

Thanks for all comments. This PR won't go to 3.7.

I want to create another PR with less functionality (without partition) that won't require newer postgreSQL.

Example:

-- Added on create connection
CREATE TEMP SEQUENCE rn;
-- On queries
SELECT *, nextval('rn') - 1 AS rn FROM (SELECT id, title FROM #__content LIMIT 10 OFFSET 2) AS a, (SELECT setval('rn', 1)) AS x;

I close this PR. Please test a new version at #13505