Zend\Paginator\Adapter\DbSelect alternative solution to count, with subselect #4641

merged 4 commits into from Jun 12, 2013


None yet
6 participants

ralphschindler commented Jun 12, 2013

This is a generic solution that should work across all database platforms (I've tested against the Zend_Db-Examples repository schema, and it works on Sqlite, MySQL, and Postgres.)

Ideally, in the future, we will have a module per vendor of database so that we can have platform specific solutions that take advantage of platform specific features. For example, MySQL could use SQL_CALC_FOUND_ROWS.

First attempt at using subselect to calculate full count

This doesn't work on non join queries.... example:
SELECT COUNT(1) AS c FROM (SELECT FROM transaction_line WHERE transaction_id = '1') AS original_select

Commenting out the line that resets the the columns fixes this.


ralphschindler commented Jun 12, 2013

Hmm, resetting should reset columns not to empty but to a wildcard I think. I'll look into this.

Zend\Db & Zend\Paginator
- Paginator: removed clearing of columns from select
- Db\Sql\Select: when resettings columns, reset value should be array(self::SQL_STAR)

ralphschindler commented Jun 12, 2013

In addition, I am not sure columns() needs to be reset at all, what do you think about this patch now?

IMO, the columns don't need to be reset... looks good.


ralphschindler commented Jun 12, 2013

Yeah, I've asked in #zftalk.dev, and they agree. If we need to do any kind of performance enhancements (which is basically what resetting columns is), we can do them elsewhere at a different time (perhaps in those vendor specific modules I mentioned).

Thanks, I'll work up unit tests and get this going.

ralphschindler added some commits Jun 12, 2013

Zend\Paginator - added tests for DbSelect
Zend\Db\Sql\Select - reverted reset on column
- reorganized the Mock setup for getItems and count tests to work

@EvanDotPro EvanDotPro merged commit e04fb2a into zendframework:master Jun 12, 2013

1 check passed

default The Travis CI build passed

In issue did not reset columns in select query to determine the count of rows in the grouped queries. Here there was a problem in my opinion. The count() method throws an error when original_select will be such:

SELECT *, TRIM(str_column) as str_column FROM table

PDOException: SQLSTATE[42S21]: Column already exists: 1060 Duplicate column name 'str_column'

Query is good work: SELECT *, TRIM(str_column) as str_column FROM table

But query determine the count of rows throws an error:
SELECT COUNT(1) AS "c" FROM ( SELECT *, TRIM(str_column) as str_column FROM table ) AS "original_select"

The error resulted in higher.

I think need to reset columns original_select on '*'.


So everything will work correctly.

And why delete code which get join information, clear, and repopulate without columns?

If in join uses the same technique ( e.g. TRIM(str_column) as str_column ) appears the same error. For this you need to reset the column in join. Since it was originally.

To me it would seem that this change would dramatically increase the processing time in some cases. In the simplest case, I used a basic full table SELECT. In this scenario, the subquery will grab all the information from the table, when perform a count on that instead of providing just a simple count of the data we need. Is that not correct?


ralphschindler replied Nov 27, 2013

Perhaps, but admittedly performance was not the top priority. Correctness and cross-db platform interoperability were higher priorities. That said, there is a great PR (#5518) that will allow for custom Select objects during pagination.

As I believe the correctness problem is mainly down to problems with queries having group by, maybe we could simply check if there are any group -bys and then select the paginator strategy accordingly: without sub-select if there are no group-bys and using the sub select if there are)..?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment