? Failure

User tests: Successful: Unsuccessful:

avatar jbultena
jbultena
20 Oct 2014

fix for issue 4423. The limit function for mssql wasn't implement correctly.

avatar jbultena jbultena - open - 20 Oct 2014
avatar jissues-bot jissues-bot - change - 20 Oct 2014
Labels Added: ?
avatar sovainfo
sovainfo - comment - 20 Oct 2014

Disagree with these changes. Please motivate your changes.

avatar brianteeman brianteeman - change - 20 Oct 2014
Category MS SQL
avatar jbultena
jbultena - comment - 20 Oct 2014

issue 4423:

After Installing Joomla 3.3.6 Full package, receive next message:
::::::::::::::::::::::::::::::::::::
Error displaying the error page: Application Instantiation Error: [Microsoft][SQL Server Native Client 11.0][SQL Server]Invalid object name 'hox1t_session'.SQL=SELECT * FROM ( SELECT [session_id] , ROW_NUMBER() OVER (ORDER BY (select 0)) AS RowNumber FROM [hox1t_session] WHERE [session_id] = 'f5i6jovj1kq5tlonjnhp8pvfl0') _myResults WHERE RowNumber BETWEEN 1 AND 1
::::::::::::::::::::::::::::::::::::

the problem is that the select syntax is not correct. There are 2 where clauses! The function processLimit is not correct for mssql.

avatar sovainfo
sovainfo - comment - 20 Oct 2014

From codereview point of view, nothing really changed! First you move the call to processLimit back to all types, then you restrict it in processLimit to apply only when there is a select mentioned in the query.

BTW Ran the statement in SQL 2012 without problem, ofcourse no rows.

Looks like an application issue to me! Don't see any reason to apply a limit. The code used: $db->setQuery($query, 0, 1); But considering it is Legacy code introduced from framework about two years ago, won't propose to change it. Applicable to any RDBMS!

avatar jbultena
jbultena - comment - 21 Oct 2014

Second try.

This error happens after a clean install with MSSQL 2008 with session handler configured as database.
the problem is that the select statment have 2 where clauses and the processlimit function is using for a DELETE command.

joomla method:
SELECT * FROM ( DELETE , ROW_NUMBER() OVER (ORDER BY (select 0)) AS RowNumber FROM [mijn_session] WHERE [time] < '1413885789') _myResults WHERE RowNumber BETWEEN 1 AND 0
(2x WHERE and a DELETE)

correct method:
SELECT * FROM ( SELECT row_number() OVER (ORDER BY column) AS rownum, column2, column3, .... columnX FROM table) AS A WHERE A.rownum BETWEEN (@start) AND (@start + @rowsperpage)

It's looks like nothing changed but by changing the order of building the select statetment give me the correct SQL statement and a working site!. This problem exist a long time since joomla 2.5 and is many times reported.

avatar jbultena jbultena - close - 21 Oct 2014
avatar jbultena jbultena - change - 21 Oct 2014
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2014-10-21 14:11:33
avatar sovainfo
sovainfo - comment - 21 Oct 2014

No, the correct method would be:
DELETE FROM [mijn_session] WHERE [time] < '1413885789'
Which you'll get when applying my PR's to query. Again, the processLimit should not be called for DELETE.

BTW also the PR of @bakual on returning when processLimit is called when both offset and limit are zero.
Note again that that is not sufficient because it doesn't solve the issues of DELETE with LIMIT 1 specified!!!

avatar sovainfo
sovainfo - comment - 21 Oct 2014

Not all statements $db->setQuery($query, 0, 1); are wrong. Several are: redirect, users, session among them. No need to put a LIMIT on queries that only return empty or 1 row. When the query contains a WHERE on the PK or other unique index with an exact match there is no need for the LIMIT 1.

The MSSQL LIMIT implementation should definitely not be applied to anything else than SELECT!

avatar jbultena
jbultena - comment - 22 Oct 2014

somewhere in the joomla framwork a function is calling the $db->setQuery($query, 0, 1) in combination with delete. I can't find it yet where this is happening. Therefore i made a extra filter in the processLimit function to execute only SELECT statements. You're right this not correct but this is a workaround for make the site working.

2nd problem. the __toString function is not correct. A select command with an where clause as a JDatabaseQueryLimitable the to __toString returns a SQL stament with 2 where clauses and not 'WHERE .. AND ...'

avatar sovainfo
sovainfo - comment - 22 Oct 2014

Can you provide the sql for 2nd problem or steps to reproduce? Is this what we discussed on the Forum? Or is this new?

avatar jbultena
jbultena - comment - 22 Oct 2014

after a clean install joomla 3.3.6 on iis7+mssql 2008 (click on the button 'go to website' after install) this is the first error:

Error displaying the error page: Application Instantiation Error: [Microsoft][SQL Server Native Client 11.0][SQL Server]Incorrect syntax near ','.
SQL=SELECT * FROM ( DELETE , ROW_NUMBER() OVER (ORDER BY (select 0)) AS RowNumber FROM [mijn_session] WHERE [time] < '1413960733') _myResults WHERE RowNumber BETWEEN 1 AND 0

this is the error:
2 WHERE clauses

WHERE [time] < '1413960733')
WHERE RowNumber BETWEEN 1 AND 0

After a refresh of the page you get the DELETE error.

This a problem that exist a long time and is reported on joomlacode tracker, github and many other forums.

My solution is a fix for the first problem and a workaround for the second DELETE problem.

avatar jbultena
jbultena - comment - 22 Oct 2014

did some more research.

The delete on the session table happens in \libraries\cms\application\cms.php line 681. In \libraries\joomla\database\query\sqlsvr.php

This calls \libraries\joomla\database\query.php line 477. This calls the processLimit function without checking the limits

change
if ($this instanceof JDatabaseQueryLimitable )
to
if ($this instanceof JDatabaseQueryLimitable && ($this->limit > 0 || $this->offset > 0))

and it's working.

I'll make an PR for this file.

Add a Comment

Login with GitHub to post a comment