#4437 MSSQL adds a redundant ORDER BY clause when with subquery with …

[trusek]

Q	A
Type	bug
BC Break	no
Fixed issues	#4437

Summary

Currently, it is checking the last ORDER BY clause if after this clause the number of open brackets is different from the number of closed brackets, then it adds an ORDER BY clause.
I changed the logic so that ORDER BY was added when, among all ORDER BY instances found in the query, the condition that number of open brackets is different than number of closed brackets for a particular clause is met.
Overall, I check every occurrence of ORDER BY, not just the last.

[morozov]

Thank you for the patch, @trusek. In addition to the unit test, it would be good to have an integration test that confirms that the simplified SQL also works and yields the expected results. See the testing guidelines and Doctrine\Tests\DBAL\Functional\ModifyLimitQueryTest.

[trusek]

I have a problem with an integration test. In SQL Server, the ORDER BY clause in a subquery must come together with TOP, OFFEST or FOR XML (ms_docs), so I figured I'd use a double modifyLimitQuery call to fit my case, but that creates bugs in other database engines e.g. IBM DB2 or SQL Server 2008. What am I supposed to do now?

[trusek]

I have no idea how to create a functional test for this case.
When I try to test SQL Server 2016, TOP, OFFEST or FOR XML is required for a subquery with ORDER BY [ms_docs], but it is incorrect for sqlite because the syntax is wrong.

[morozov]

I have no idea how to create a functional test for this case.

It should be the query that made you make this change in the codebase.

it is incorrect for sqlite because the syntax is wrong

Please make it as generic as possible and skip on those platforms that don't support this syntax.

[trusek]

I used QueryBuilder to create a subquery with ORDER BY. Which eliminated syntax errors for other database engines.
For some reason the MariaDB and MySql pipeline crashes with an error:

OK, but incomplete, skipped, or risky tests!
Tests: 5772, Assertions: 10176, Skipped: 672, Incomplete: 11.

Generating code coverage report in Clover XML format ... done [00:00.479]
/home/runner/work/_temp/1dcb457b-658c-4781-ba88-0ada93656118.sh: line 1:  5764 Segmentation fault      (core dumped) vendor/bin/phpunit -c ci/github/phpunit/mysqli.xml --coverage-clover=coverage.xml
Error: Process completed with exit code 139.

[morozov]

Apart from the new protected method, looks good. Please squash.

[morozov]

Thanks, @trusek.