Joomla! Issue Tracker - CMS

Download
Launch
Feeling Lucky

[#10252] - Incorrect DELETE query when using setLimit()

? Success

User tests: Successful: Unsuccessful:

avatar OctavianC
OctavianC
5 May 2016

Issue

Using a delete() query:

$db     = JFactory::getDbo();
$query  = $db->getQuery(true);
$query->delete('#__session')
    ->where($db->qn('time').' < '.$db->q(JFactory::getDate()->modify('-30 days')->toUnix()))
    ->setLimit(1);

echo $query->dump();

Produces this:

DELETE 
FROM kfdcs_session
WHERE `time` < '1459851698' LIMIT 0, 1

The query is incorrect - DELETE doesn't use an offset http://dev.mysql.com/doc/refman/5.7/en/delete.html

Summary of Changes

Because specifying an offset is optional in MySQL, LIMIT 0, 1 is the equivalent of LIMIT 1 so changing the processLimit() function shouldn't cause any issues with existing code. The output should now be:

DELETE 
FROM kfdcs_session
WHERE `time` < '1459851698' LIMIT 1

Testing Instructions

Just use the above code to test - you can add it in a component (eg. open /administrator/components/com_content/content.php and add it after defined('_JEXEC') or die;. Going to Articles will show the query.

Make sure no other SQL errors are present - pagination should still work, deleting items etc

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
1.00
avatar OctavianC OctavianC - open - 5 May 2016
avatar OctavianC OctavianC - change - 5 May 2016
Status New ⇒ Pending
avatar joomla-cms-bot joomla-cms-bot - change - 5 May 2016
Labels Added: ?
avatar andrepereiradasilva andrepereiradasilva - test_item - 5 May 2016 - Tested successfully
avatar andrepereiradasilva
andrepereiradasilva - comment - 5 May 2016

I have tested this item :white_check_mark: successfully on 2c33fd7

Note that i'm not a sql or even joomla session expert.
Just applied the patch, logged in, logged out in the frontend and admin and didn't get any issues.

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

avatar brianteeman
brianteeman - comment - 5 May 2016

This is only for the mysqli driver does it need to be done for the others as well?

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

avatar brianteeman brianteeman - change - 5 May 2016
Category ⇒ Libraries SQL
avatar OctavianC
OctavianC - comment - 5 May 2016

Any driver using MySQL would do - so that includes PDO as well. Haven't tested PDO to be honest

avatar mbabker
mbabker - comment - 5 May 2016

The other MySQL driver query classes extend the MySQLi class, so they have the same change already.

avatar sovainfo
sovainfo - comment - 6 May 2016

Suggest 

if ($limit > 0 )
{
    if ($offset > 0)
    {
        $query .= ' LIMIT ' . $offset . ', ' . $limit;
    }
    else
    {
        $query .= ' LIMIT ' . $limit;
    }
}
avatar zero-24 zero-24 - test_item - 7 May 2016 - Tested successfully
avatar zero-24
zero-24 - comment - 7 May 2016

I have tested this item :white_check_mark: successfully on 2c33fd7

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

avatar zero-24 zero-24 - change - 7 May 2016
Status Pending ⇒ Ready to Commit
avatar zero-24
zero-24 - comment - 7 May 2016

Thanks @OctavianC looks good to me.

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

avatar joomla-cms-bot joomla-cms-bot - change - 7 May 2016
Labels Added: ?
avatar brianteeman brianteeman - change - 7 May 2016
Milestone Added:
avatar wilsonge wilsonge - close - 7 May 2016
avatar wilsonge wilsonge - merge - 7 May 2016
avatar joomla-cms-bot joomla-cms-bot - close - 7 May 2016
avatar wilsonge wilsonge - change - 7 May 2016
Status Ready to Commit ⇒ Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 ⇒ 2016-05-07 14:08:30
Closed_By ⇒ wilsonge
avatar joomla-cms-bot joomla-cms-bot - change - 7 May 2016
Labels Removed: ?

Add a Comment

Login with GitHub to post a comment