Don't change MSSQL query if no limit or offset is set

[Bakual]

Issue

Currently, MSSQL creates an error for each database query where no limit is set. Like for inserts, updates and the like.
See #3602 and #4869 for the issue reports.

Solution

Add a check to the sqlsrv query class in the processLimit() method. Similar to what we already do in MySQL and PostgreSQL.

Testing

@sovainfo can you explain how to reproduce the issue? As far as I understood one sees that on first page after updating?

[sovainfo]

According to me the issue is that the limit is applied while there is no need:

• It is called several times by $db->setQuery($query, 0, 1); while result of the query is always no row or 1 row.
• It is called for statements other than type SELECT
• It is called even when both limit and offset are 0

This is independent of MSSQL, it applies to MySQL and PostgreSQL as well.

Don't mind that the function returns when both are not set. But that doesn't stop the errors on #__session deleting 1 row !

As explained in #4869 the update from J336 when set to test fails.

[Bakual]

This is independent of MSSQL, it applies to MySQL and PostgreSQL as well.

I'm not aware of any issues like this in MySQL and PostgreSQL. And judging from code, there should be any issues since they have a similar check already in their processLimit() method.

Don't mind that the function returns when both are not set. But that doesn't stop the errors on #__session deleting 1 row !

That may be a different issue. I don't know how you have to apply limits to a DELETE on MSSQL. Maybe the processLimit method needs to be adjusted to take care of that special case.

[sovainfo]

I thought the purpose of Joomla was to be independent of RDBMS. That means you need to do the right thing for all, not just one!. Calling routines when they shouldn't is still wrong. It doesn't make it right when it doesn't give problems in certain situations.

Just because PHP and MySQL allows sloppy programming, it doesn't means you should do it!
Specially if want to support other environments as well. Better to remove MSSQL and PostgreSQL from Joomla. They are never going to work as MySQL. And that seems to be the requirement here!

[Bakual]

Just to be clear: This PR just adds a behavior to MSSQL which is already present in PostgreSQL and MySQL query classes.
It makes it consistent on where it is checked and leaves the responsibility in the database specific query class. Which imho is correct.

[jbultena]

finnaly this bug gonna get fixed.

[roland-d]

@Bakual Shouldn't we do the same here as in other drivers:

if ($limit > 0 && $offset > 0)

[Bakual]

In other places we have the code to apply the limit within the if clause.
I went the other way around and bail out if the checks are not successful. I prefer this way in this case because the actual code has another if clause and is quite a few lines long.
I think it's easier to read this way.
Also PostgreSQL has yet another way

        if ($limit > 0)
        {
            $query .= ' LIMIT ' . $limit;
        }

        if ($offset > 0)
        {
            $query .= ' OFFSET ' . $offset;
        }

So there is not really a standard anyway 😄

[roland-d]

It is probably dependent on the database type on how to handle these values.

[Bakual]

Closing as fixed with #5293