Speed up of saving new articles by csthomas · Pull Request #13505 · joomla/joomla-cms

csthomas · 2017-01-08T00:01:00Z

Pull Request for Issue #10567.

Related to older implementation at #12839 which requires postgreSQL >= 8.4.2.

Summary of Changes

Big performance improvement in JTable::reorder() method with B/C.
It works on sqlite, mysql, postgresql and mssql, (on oracle probably too).
Do not load any tables to php on "reordering".
New public method JDatabaseQuery::selectRowNumber($orderBy, $orderColumnAlias) which add an additional incremental column to SELECT type query.
You can use selectRowNumber only once per query.
to reorder articles use multi table UPDATE query with SELECT in subquery.

How it works.

create SELECT sub-query with row_number column.
create UPDATE query with the same where clause.
join above queries by primary key or keys if exists.
update ordering to new values from SELECT sub-query.

Details about `selectRowNumber()`

on MySQL PostgreSQL >= 8.4.0 and SQL Server SELECT query preserves position of row_number column - you can use safely $db->loadRowList(), examples:
- ...select('id')->selectRowNumber('ordering ASC', 'row_number') generates result row with keys as 'id, row_number'
- ...selectRowNumber('ordering ASC', 'row_number')->select('id') generates result row with keys as 'row_number,id'
on postgreSQL less that 8.4.0 and SQLite row_number is always on the last column, This is related to index column at loadRow() and loadRowList().

Other changes

PostgreSQL:
- ~~fix update query with join: PostgreSQL - return the same string each time of call __toString() on update query with join #13284~~ - merged
- ~~add sql var "CREATE TEMP SEQUENCE ROW_NUMBER" after establish a connection.~~
SQL Server:
- fix multi table UPDATE to generate correct query
- In SELECT query move HAVING before ORDER BY
SQLite:
- add support for multi table UPDATE query by INSERT OR REPLACE INTO
- add SQLite function ROW_NUMBER(init=null, partitionBy=null) after establish a connection.
Add unit tests for selectRowNumber and multi table UPDATE query.
Fix a few other unit test files that generated error after applied my changes.

Testing Instruction

go to back end to list of articles (com_content).
set order list by "Ordering asceding"
select any category and set level 1
add 3 example articles (articleA, articleB, articleC) in the same category as featured.
back to articles list and you should see articles on the top of list (articleC, articleB, articleA, ...olders...)
go to Featured Articles list and set order list by "Ordering asceding"
you should see on the top of list (articleC, articleB, articleA, ...olders...)

If something went wrong then test it first without PR.
After this PR result should be the same.

[UPDATED]
How to test on postgreSQL
PostgreSQL Query has 2 versions of code which depend on postgresql server version (< 8.4 or >= 8.4).
So it would be good to test 2 options but as usual there is probably nobody with postgresql >= 8.3.18 and < 8.4.

first do the test on postgresql as usual,
then simulate older postgresql by changing line:
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/database/driver/postgresql.php#L579
from {
to { return '8.3.18';
and test again.

Documentation Changes Required

Probably.

Introduce a new public method JDatabaseQuery::selectRowNumber().
Main database drivers (mysql, postgresql, sqlsrv and sqlite) can use Update with innerJoin.
SQLite driver has a new sqlite function ROW_NUMBER(init=null, partitionBy=null).

Benchmark (old)

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.

waader · 2017-01-08T09:52:17Z

sorry wrong pull

photodude · 2017-01-08T16:00:15Z

@csthomas would you consider also adding the JDatabaseQuery selectRowNumber() method and related changes to the Framework database repo

csthomas · 2017-01-08T17:15:14Z

I will but now I want to add it to joomla 3.7.

csthomas · 2017-01-21T16:42:08Z

I have added two versions of selectRowNumber(), for postgresql >= 8.4.0 and other for less than 8.4.0.

ghost · 2017-01-22T11:53:07Z

I have tested this item ✅ successfully on be84faa

With and without PR Result is same ordering like in "Testing instruction" described.

Tested on:

Joomla! 3.7.0-beta1-nightly
macOS Sierra, 10.12.2
Firefox 50.1.0
PHP 7.0.4
MySQLi 5.5.53-0

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13505.}

csthomas · 2017-01-23T10:10:58Z

@ggppdk Can I ask you for test mainly on postgreSQL (two versions).
If you have more time then on mysql and on mssql too.

ggppdk · 2017-01-24T06:01:28Z

@csthomas, I moved to new apartment, trying to keep up with existing obligations now, little time till 10 of February, i hope someone can step up to test this

csthomas · 2017-01-24T08:32:01Z

@ggppdk Thanks for response.

marrouchi · 2017-01-24T10:10:50Z

I have tested this item ✅ successfully on be84faa

Tested on mysql version 5.5.53-0ubuntu0.14.04.1

Before patch
COUNT :1116
TIME DIFF : 5.6671

After patch
COUNT :1120
TIME DIFF : 0.0319

It was successful following the Testing instruction.

But when i do the following, with or without the PR :

go to back end to list of articles (com_content).
set order list by "Ordering asceding"
select any category and set level 1
add 3 example articles (articleA, articleB, articleC) in the same category as not featured.
back to articles list and we see articles on the top of list (articleC, articleB, articleA, ...olders...)
check the articles (articleA, articleB, articleC) and hit "Feature" and we still see the same ordering
go to Featured Articles list and set order list by "Ordering asceding" we see on the top of list (articleB, articleC, articleA, ...olders...).

Any ideas on this anomaly ?

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13505.}

csthomas · 2017-01-24T11:47:19Z

check the articles (articleA, articleB, articleC) and hit "Feature" and we still see the same ordering

this is ok, you add articles to featured but list order is by article ordering (not featured ordering)

go to Featured Articles list and set order list by "Ordering asceding" we see on the top of list (articleB, articleC, articleA, ...olders...).

You add in one time 3 articles to featured, sql will deliver 3 featured with ordering=0 to reorder. IMO It won't preserve any order. it is a set of 3 articles, not ordered list of 3 items.

csthomas · 2017-01-25T11:35:19Z

@waader @alikon @photodude Can you help with test? (mainly postgresql, but it would be nice to test mssql too)

photodude · 2017-01-25T18:06:36Z

@csthomas I'm currently primarily working on the Windows CI testing with Appveyor
See
joomla-framework/database#72
joomla-framework/database#74
joomla-framework/database#75
#13690
and the Alpha of the PHPCS 2 coding standards autofixers release see
joomla/coding-standards#143

So I haven't had time to test CMS specific PRs (my current local needs to be rebuilt too, especially for any PostgreSQL or MSSQL testing).

If you would be willing to pitch in on those projects, I could allocate time to setup a local for testing here.

There is some additional consideration with PostgreSQL or MSSQL testing, we often only merge those by code review especially when MySQL passes, or if there are no MySQL related changes. Since the change will be held up if try to find someone with a system configuration that meets those edge case requirements and time to test. Final call on a Merge by review would need to be made by the release leader or someone else with commit privileges.

We have two successful tests on MySQL, so technically we just need two reviewers to mark approved and this is RTC

photodude

Looks good by code review. Includes Unit tests, so we should be good overall.

marrouchi · 2017-01-26T00:02:40Z

I have tested this item ✅ successfully on be84faa

Tested on psql (PostgreSQL) 9.3.15

Before the patch
COUNT : 4044
TIME DIFF : 36.7341 sec

After the patch

COUNT : 4044
TIME DIFF : 0.1496 sec

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13505.}

jeckodevelopment · 2017-01-26T22:37:52Z

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13505.}

joomla-cms-bot added PR-staging Unit/System Tests labels Jan 8, 2017

csthomas force-pushed the row_number_37 branch from 63e9294 to 5787d92 Compare January 8, 2017 00:04

csthomas mentioned this pull request Jan 8, 2017

[RFC] Another way to fast reorder table with single SQL query #12839

Closed

csthomas changed the title ~~Fast table reorder for mysql, postgresql, mssql and sqlite~~ Speed up of saving new articles Jan 12, 2017

csthomas force-pushed the row_number_37 branch 2 times, most recently from 72733e6 to 9d7736e Compare January 15, 2017 09:02

Tomasz Narloch added 2 commits January 21, 2017 17:35

Fast table reorder for mysql, postgresql, mssql and sqlite

ec061e5

Two versions of selectRowNumber for postgreSQL

be84faa

csthomas force-pushed the row_number_37 branch from 2fbbaca to be84faa Compare January 21, 2017 16:36

photodude approved these changes Jan 25, 2017

View reviewed changes

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jan 26, 2017

jeckodevelopment added this to the Joomla 3.7.0 milestone Jan 26, 2017

rdeutz merged commit 1b036c6 into joomla:staging Jan 27, 2017

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 27, 2017

csthomas deleted the row_number_37 branch January 27, 2017 19:02

Sieger66 mentioned this pull request Feb 12, 2017

JTable->reorder is extremely slow #8563

Closed

csthomas mentioned this pull request Apr 9, 2017

Saving a new article into a category with thousands of articles that are mostly archived #15158

Closed

ggppdk mentioned this pull request Dec 28, 2017

Saving new Article on large category #12552

Closed

alikon mentioned this pull request Feb 23, 2018

[4.0] Cant save articles using Mysql (PDO) Driver #19767

Closed

richard67 mentioned this pull request May 18, 2019

[4.0] - Postgresql save article #24148

Merged

Uh oh!

Conversation

csthomas commented Jan 8, 2017 • edited by jeckodevelopment Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

How it works.

Details about selectRowNumber()

Other changes

Testing Instruction

Documentation Changes Required

Benchmark (old)

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

Uh oh!

waader commented Jan 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

photodude commented Jan 8, 2017

Uh oh!

csthomas commented Jan 8, 2017

Uh oh!

csthomas commented Jan 21, 2017

Uh oh!

ghost commented Jan 22, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tested on:

Uh oh!

csthomas commented Jan 23, 2017

Uh oh!

ggppdk commented Jan 24, 2017

Uh oh!

csthomas commented Jan 24, 2017

Uh oh!

marrouchi commented Jan 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

csthomas commented Jan 24, 2017

Uh oh!

csthomas commented Jan 25, 2017

Uh oh!

photodude commented Jan 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

photodude left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marrouchi commented Jan 26, 2017

Uh oh!

jeckodevelopment commented Jan 26, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

csthomas commented Jan 8, 2017 •

edited by jeckodevelopment

Loading

Details about `selectRowNumber()`

waader commented Jan 8, 2017 •

edited

Loading

ghost commented Jan 22, 2017 •

edited by ghost

Loading

marrouchi commented Jan 24, 2017 •

edited

Loading

photodude commented Jan 25, 2017 •

edited

Loading

photodude left a comment •

edited

Loading