?
avatar alex7r
alex7r
27 Jun 2016

Steps to reproduce the issue

Call getPagination

Expected result

1 query

Actual result

If query was build not with query builder, or query has sub query which Joomla can add only as string and sub query have group by
Pagination calls with query 2 times: 1 normal, 1 with count.
So on big data it can force multiple increase of time to load.

Additional comments

Why don't use SELECT SQL_CALC_FOUND_ROWS?
It will allow to calculate total rows without limit while getting limited data and then just ask for total.
So one calculation and almost no extra time needed.
Tested on number of projects.

avatar alex7r alex7r - open - 27 Jun 2016
avatar brianteeman brianteeman - change - 27 Jun 2016
Priority Urgent Medium
avatar brianteeman
brianteeman - comment - 27 Jun 2016

Rest priority to documented standards https://docs.joomla.org/Priority


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

avatar ggppdk
ggppdk - comment - 27 Jun 2016

Why don't use SELECT SQL_CALC_FOUND_ROWS?

There has been a resonable argument that DBMS specific optimizations, should not be added

About adding this,

  • i am using it too and gives some measurable benefit in large sites (not as much as you would expect because (if i remember correctly) the row counting query skips the ordering clause)
  • it should work with 99% of extensions in backend, using it in frontend it will break a few extensions (as i have seen, and that is why i am using it with try-catch in frontend)
avatar mbabker
mbabker - comment - 27 Jun 2016

Why don't use SELECT SQL_CALC_FOUND_ROWS?

As long as Joomla continues the charade of supporting multiple database engines MySQL specific syntax nor fixes should be introduced unless it is in a layer specific to that engine (i.e. the MySQL database drivers or the Smart Search MySQL adapter).

avatar alex7r
alex7r - comment - 27 Jun 2016

And?
Does some body even trying to post an answer or get along with problem? Or everybody just showing there activity?

specific optimizations
unless it is in a layer specific to that engine

Pagination calls getTotal on Model which in the end of the day calls
(int) $this->_db->getNumRows()
Any way.
So it's pretty specific layer, and what's the point of using specific DB Engine Driver if you don't what to use specific optimizations?

in frontend it will break a few extensions

well, let's die with dinosaurs. really, why upgrade to Joomla 3.5 from 2.5? it may not get along with couple of extensions

avatar mbabker
mbabker - comment - 27 Jun 2016

If you think the SELECT SQL_CALC_FOUND_ROWS query is more performant than the mysqli_num_rows() function (in the case of the MySQLi driver) submit a pull request changing that method's internals. Just remember though that method is used for more than just pagination so if you're making that type of change at the database layer it needs to not introduce regressions.

avatar ggppdk
ggppdk - comment - 27 Jun 2016

And? Does some body even trying to post an answer or get along with problem? Or everybody just showing there activity?

This will not get you more / better answers, it will get you fewer answers, please forgive me for trying to reply you

avatar alex7r
alex7r - comment - 27 Jun 2016

According to, you know, logic and MySQL documentation, and test profs,
using SELECT SQL_CALC_FOUND_ROWS instead of double querying gives performance benefits.

Why don't just implement and pull request? Because I want be sure that someone labeled wouldn't cut ropes for this pull just because he and his mandate think another way.

Oh, right I get your point.

Just remember though that method is used for more than just pagination so if you're making that type of change at the database layer it needs to not introduce regressions.

You just showing activity. As this function used about 10 times in the core and all of uses are type of getting pagination. So of course let's try to say that it to much to handle, there would be regression, let's just live the way our grands does.

please forgive me for trying to reply you

Just try to get my point: answers are needed, not all replies are answers.

And I'm submitting an issue not just to try to communicate, but trying to bring problem up and solve it.
Replies with: let's not do what we suppose to - are just don't feel right.

avatar mbabker
mbabker - comment - 27 Jun 2016

As this function used about 10 times in the core and all of uses are type of getting pagination. So of course let's try to say that it to much to handle, there would be regression, let's just live the way our grands does.

The method in the database driver is documented to perform an explicit function. That function is NOT solely there to support pagination. It might be the only purpose it serves to the core CMS extensions or MVC layer, but that does NOT mean that that is its only use case. The method is documented to:

Get the number of returned rows for the previous executed SQL statement.

So keep that in mind before you start flapping your gums about academics versus practical use cases.

Also, please refrain from bringing religious commentary to this tracker. There is zero need for that.

avatar alex7r
alex7r - comment - 27 Jun 2016

Okay, it was not about "bringing religious commentary", but it's my bad and my apologize for any one who could feel it offensive in religious way. I'll keep that in mind for future.

avatar ggppdk
ggppdk - comment - 27 Jun 2016

The method in the database driver is documented to perform an explicit function. That function is NOT solely there to support pagination.

We don't need to break it

    public function getTotal()
    {
        if (is_null($this->total))
        {
                $query = $this->buildCountQuery();
...

if the query is performed before getTotal() is called then it can set:

$this->total
avatar ggppdk
ggppdk - comment - 27 Jun 2016

Hhhh that is older code, i have to look at the newer one

ok it is similar

    public function getTotal()
    {
        // Get a storage key.
        $store = $this->getStoreId('getTotal');

        // Try to load the data from internal storage.
        if (isset($this->cache[$store]))
        {
            return $this->cache[$store];
        }

So
getItems() can set

$store_total; = $this->getStoreId('getTotal');
$this->cache[$store_total] = ...;
avatar alex7r
alex7r - comment - 1 Jul 2016

Well it's not solving the problem as there still would by
SELECT COUNT(*)
and
SELECT fields
queries and not only 1 query.

However I've almost done implementing solution for this but still working on it as I've faced an issue:
getItems() call getStart() which calls getTotal(), what means that we need to have total before we have called for getting Items.

avatar alex7r
alex7r - comment - 1 Jul 2016

Well, after all seems that there is no way to solve the issue without stripping https://gist.github.com/alex7r/44996460ce13587b36520e43a59b915f#file-list-php-L22-L25
This is an answer to initial question, so issue can be closed.
The reason has no relation to "no DBDriver specific optimizations".

Proposed solution could be implemented only if user can't change limit without rolling back to first pagination page.

avatar alex7r alex7r - change - 1 Jul 2016
Status New Closed
Closed_Date 0000-00-00 00:00:00 2016-07-01 14:14:14
Closed_By alex7r
avatar alex7r alex7r - close - 1 Jul 2016

Add a Comment

Login with GitHub to post a comment