[#17065] - REGRESSION[4.0]: Database driver limit/offset not working anymore for SQL strings
- Closed
- 12 Jul 2017
- Medium
- Build: staging
- # 17065
Steps to reproduce the issue
Try to use a string SQL query such as:
$query = "SELECT * FROM #__mycomponent_table";
$limitStart = 0;
$limit = 5;
$db->setQuery ( $query, $limitStart, $limit );
$records = $db->loadObjectList();
Expected result
5 records are retrieved by the database driver
Actual result
All records are retrieved by the database driver
System information (as much as possible)
Using a query object it works, using a string SQL query it doesn't work anymore.
WORKING:
$queryObject = $db->getQuery(true);
$queryObject->select('*');
$queryObject->from('#__mycomponent_table');
$limitStart = 0;
$limit = 5;
$db->setQuery ( $queryObject, $limitStart, $limit );
$records = $db->loadObjectList();
5 records as expected
NOT WORKING:
$query = "SELECT * FROM #__mycomponent_table";
$limitStart = 0;
$limit = 5;
$db->setQuery ( $query, $limitStart, $limit );
$records = $db->loadObjectList();
All records in the database are retrieved
Additional comments
joomla-cms-bot
- | Labels |
Added:
?
|
||
franz-wohlkoenig
- | Category | ⇒ | SQL |
| Title |
|
||||||
| Title |
|
||||||
It should work. https://github.com/joomla-framework/database/blob/2.0-dev/src/Mysqli/MysqliDriver.php#L822
We internally convert string queries to a query object and apply limit and offset when setting the query to the driver.
It doesn't work. If you try it you see that it doesn't work.
I'm going to open a PR.
The only two possible places it could be failing is in that method I linked to or the __toString() method converting the query object back to a SQL string. But it is not the fact that prepared statements are now supported that is the problem.
Probably the problem is in the __toString() method.
I guess that it doesn't append the LIMIT x, x to the direct SQL string property.
All of our query objects implement the LimitableInterface so that part of the check shouldn't be failing. So the limit/offset should be getting pushed in as long as they are both non-null values. My guess would be $query->setLimit() probably because of too restrictive checks.
The problem when you do the following in the setQuery:
$query = $this->getQuery(true)->setQuery($query);
is that the object 'type' and other variables are not initialized.
Thus the __toString method and others never execute the:
$this->processLimit($query, $this->limit, $this->offset);
Quickest fix is probably within the if statement at https://github.com/joomla-framework/database/blob/2.0-dev/src/DatabaseQuery.php#L225 to change that to call processLimit as well (and ensuring the subclasses have that same check if they aren't calling back into the parent class).
I confirm you that changing the code as you suggested and adding one more processLimit call works:
if ($this->sql) { return $this->sql; }
to
if ($this->sql) { $this->sql = $this->processLimit($this->sql, $this->limit, $this->offset); return $this->sql; }
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-07-12 00:56:54 |
| Closed_By | ⇒ | mbabker |
Looks like the new execution of the MySql prepared statement is the cause of the issue.
Indeed the following does not apply the LIMIT clause to a string query stored in Joomla\Database\Mysqli\MysqliQuery->sql property
$this->prepared->execute()