Limit and Offset not working correctly with MSSQL

[Shnoulle]

Limit and Offset not working correctly with MSSQL

[wtommyw]

Using the given example I was able to reproduce the issue in MSSQL versions 10, 11, 12, 14 and 15.

An example:
The table item contains 100 rows. Using the following code:

$criteria = new CDbCriteria();
$criteria->limit = 10;
$criteria->offset = 95;

$models = Item::model()->findAll($criteria);

The following SQL is generated:

SELECT *
FROM (SELECT TOP 10 * -- limit
    FROM (SELECT TOP 105 * -- offset+limit
        FROM [dbo].[item] [t]
            ORDER BY id) AS [__inner__]
    ORDER BY id DESC) AS [__outer__]
ORDER BY id ASC

This query first selects the top 105 rows, ordered by their id descending. Off these 105 rows it selects the top 10 and finally it flips these 10 back around so they are in ascending order.

The table does not contain 105 rows, so the inner query will return the top 100 rows. From which we then select the top 10. Causing the last page in the paginations to overlap with the previous (as @Shnoulle described)

Concluson: this is not a forward compatibility issue as this behaviour is the same in version 10 through 15. It appears that pagination hasn't worked properly for MSSQL since at least 2008. (I was not able to get any old versions running)

How does Yii2 handle this?
Yii2 diffirentiates between MSSQL <11 and MSSQL >=11, producing two different query's:
The following SQL is generated for MSSQL <11:

SELECT TOP 10 * FROM (
    SELECT rowNum = ROW_NUMBER() 
    OVER (
        ORDER BY (SELECT NULL)
    ), * FROM [item]
) sub WHERE rowNum > 95

The following SQL is generated for MSSQL >=11:

SELECT * FROM [item] 
    ORDER BY (SELECT NULL) 
    OFFSET 95 ROWS 
    FETCH NEXT 10 ROWS ONLY

Both of these queries return the expected result. (e.g. item 95 through 100).

I propose Yii2's MSSQL version check and Yii2's fix for MSSQL >11 are backported so that CMssqlCommandBuilder will generate the correct SQL for MSSQl version 11 and newer.

[jjdunn]

@marcovtwout wrote:

@Shnoulle If this is the same issue that was fixed in Yii 2, that would mean that offset/pagination has never worked on Yii 1 withMSSQL 2012 ( "v11") and up? Seems strange that this is reported now if this problem already exists for 10 years.

Hi all - I've been Yii1.x user with SQL Server since 2010 :-) The issue has existed for a long time !!
See https://code.google.com/archive/p/yii/issues/1501 - I am "Happy Lion" in the comments on this ticket. Also https://code.google.com/archive/p/yii/issues/997 which is a related ticket.

I've been running with a patched framework since Feb 2012 (11+ years!) - using the fix proposed in issue 1501 comment (7)

BUT - I am very glad to see a better solution. The fix in #4501 is more elegant. We are currently running Yii 1.1.23; now upgrading to 1.1.28 in prep for upgrade from PHP 8.0.x to 8.1/8.2

Big thanks to everyone who continues to develop & maintain Yii, including 1.1.x !! We considered upgrading to 2.x but it was a lot of work, we have a lot of custom components built on top of 1.1, and not much payoff. If I was starting a new app I'd certainly use 2.x

[jjdunn]

@marcovtwout - thanks for responding quickly. I completely understand reluctance to patch Yii 1.1.x, especially given 1) SQL Server is uncommon; 2) edge case of complex ORDER BY case.

We will simply revert to our long-standing / working patch (however ugly) :-)

[marcovtwout]

Could you clarify: Is this a forward compatibility issue with newer MSSQL versions? Or a bug when using older versions of MSSQL? And which versions exactly are working/broken?

This seems to be the related issue for Yii 2: yiisoft/yii2#4254, and the fix: yiisoft/yii2@212c5ee

[Shnoulle]

Could you clarify: Is this a forward compatibility issue with newer MSSQL versions? Or a bug when using older versions of MSSQL? And which versions exactly are working/broken?

It's forward compatibility issue with newer MSSQL. I'm sure it broke with SQL15, the related fix in Yii2 show SQL11

It's already fixed in Yii2, but not in Yii1 :) only Yii1 have issue .

Unsure MSSQL issue are OK to be merged.

[marcovtwout]

@Shnoulle If this is the same issue that was fixed in Yii 2, that would mean that offset/pagination has never worked on Yii 1 withMSSQL 2012 ( "v11") and up? Seems strange that this is reported now if this problem already exists for 10 years.

Could anyone perhaps try to reproduce the issue on older MSSQL versions, such as 14, 11 and 10?

[Shnoulle]

For LimeSurvey : it exist since a lot of time, but the real issue shown only on grid part.
And clearly : see the last 10 element and not 5 aren't seen like an issue

You can compare

Yii1 limit/offset version :

yii/framework/db/schema/mssql/CMssqlCommandBuilder.php

Line 216 in cea893f

protected function rewriteLimitOffsetSql($sql, $limit, $offset)

And Yii2 limit/offset/order compatible version https://github.com/yiisoft/yii2/blob/364e907875fd57ee218085cca796ac5d1c3c8d51/framework/db/mssql/QueryBuilder.php#L115

For LimeSurvey : seem to be fixed for 2 SQL server…

[Shnoulle]

Thank you for confirmation

I can not really create the PR for MSSQL <11 since i can not test it :(

[marcovtwout]

@Shnoulle Could you test if #4501 fixes your problem and could you verify the MSSQL version you are testing with?

[marcovtwout]

Fixed with #4501

[jjdunn]

we are testing using Yii 1.1.28, and the change in this ticket fails with a complex ORDER BY clause, as follows:

$crit = new CDbCriteria;
$crit->alias = 't';
$crit->addColumnCondition(....):
$crit->addInCondition(...);
$SQL= 'exists (select 1 from .... where ....)';
$crit->addCondition($SQL);
$crit->with = [....];
$FN= .....; // fieldname for sorting
$crit->order = '(case when t.Sitting = 0 then 1 else 0 end), s.Gender, (case when isnull(s.{$FN},0) >0 then 1 else 0 end), s.LastName, s.FirstName';

$crit->limit = $limit; // defined in prior code
$crit->offset = $offset; // defined in prior code
$total = TableName::model()->findAll($crit);  <<===== ERROR

2023/03/29 04:34:45[24.60.188.13] [error] [php] preg_match():
Compilation failed: missing closing parenthesis at offset 52
(C:\web\test\framework\db\schema\mssql\CMssqlCommandBuilder.php:304)

Note that this complex ORDER BY structure containing parentheses and a CASE statement, worked OK using the code cited in previous comment : comment: 7 from https://code.google.com/archive/p/yii/issues/1501

Environment: PHP 8.0.26 Win/x64 on Windows Server 2019; Apache httpd 2.4.54; SQL Server 14.0.3460.9; PHP PDO drivers 5.10.1.15814

[marcovtwout]

@jjdunn This is a known limitation of the Yii implementation, both before and after the fix from this ticket:

yii/framework/db/schema/mssql/CMssqlCommandBuilder.php

Lines 170 to 178 in d701ef0

	* <b>Regular expressions are used to alter the SQL query. The resulting SQL query
	* may be malformed for complex queries.</b> The following restrictions apply
	*
	* <ul>
	* <li>
	* In particular, <b>commas</b> should <b>NOT</b>
	* be used as part of the ordering expression or identifier. Commas must only be
	* used for separating the ordering clauses.
	* </li>

It looks like the comma in (case when isnull(s.{$FN},0) >0 then 1 else 0 end) is not allowed.

Since this code is pretty old and complex already I'm not too keen in adding support now, especially using the referenced implementation from google code, as it states "TODO optimize and redone cuz current implementation is very ugly". So I'm inclined to mark this a wontfix, but a thouroughly tested PR with proper implementation is always welcome.