? Success
Related to # 4870

User tests: Successful: Unsuccessful:

avatar sovainfo
sovainfo
16 May 2014

Fix for [#33307]: Error displaying the error page: Application Instantiation Error
http://joomlacode.org/gf/project/joomla/tracker/?action=TrackerItemEdit&tracker_item_id=33307&start=0

avatar sovainfo sovainfo - open - 16 May 2014
avatar nicksavov nicksavov - change - 21 Aug 2014
Labels Removed: ?
avatar nicksavov nicksavov - change - 21 Aug 2014
Title
Only run processLimit when a limit or offset is set
[#33307] Only run processLimit when a limit or offset is set
avatar brianteeman brianteeman - change - 21 Aug 2014
Status New Pending
avatar BugsFinder
BugsFinder - comment - 3 Oct 2014

@test ok, PR resolves my issue

avatar BugsFinder
BugsFinder - comment - 3 Oct 2014

ok, this fix work.

avatar brianteeman brianteeman - alter_testresult - 16 Oct 2014 - BugsFinder: Tested successfully
avatar joomlacorner joomlacorner - test_item - 17 Oct 2014 - Tested successfully
avatar sovainfo
sovainfo - comment - 20 Oct 2014

How embarrassing that this still not implemented! This is the first thing people have to do after installing J336 going to the administrator. It reports:

42000 [Microsoft][SQL Server Native Client 11.0][SQL Server]A nested INSERT, UPDATE, DELETE, or MERGE statement must have an OUTPUT clause.SQL=SELECT * FROM ( UPDATE [j336_extensions] SET [params] = '{"mediaversion":"a0029d5a3035565db1f2f0ddb93da8a6"}' WHERE [type] = 'library' AND [element] = 'joomla') _myResults WHERE RowNumber BETWEEN 1 AND 0

Which is caused by calling processLimit while it shouldn't!

Apply PR and you can login!

Maybe someone can change the prority to CRITCAL, or even better commit!

avatar mbabker
mbabker - comment - 20 Oct 2014

It's easy to just merge things without review or testing, but that's bad for quality assurance. Unfortunately, this has had next to no testing (there's been one good test so far). Find one or two others to test general use cases since you've modified the base query building class and we can get it merged.

avatar sovainfo
sovainfo - comment - 20 Oct 2014

This has been tested and proven to work numerous times in the field. Just visit the forum!

avatar mbabker
mbabker - comment - 20 Oct 2014

Any of them report a good test on any of the patches you've submitted? I can't speak for others, but I personally don't spend a lot of time in the forums and when I do it's usually not relating to bug activity anymore. Not trying to sound difficult, I promise.

avatar sovainfo
sovainfo - comment - 20 Oct 2014

See http://forum.joomla.org/viewtopic.php?f=717&t=825222&hilit=+3602#p3113359
quote: Further digging & debugging showed that the function 'processLimit()' in "libraries\joomla\database\query\sqlsrv.php" was mangling the queries. Adding the parameter check that the function 'limit()' in "libraries\joomla\database[drive]\sqlsrv.php" has, relieved the "Application Instantiation Error" issue.

http://forum.joomla.org/viewtopic.php?f=717&t=825222&hilit=+3602#p3113437

Requested several times to report a good test. Seems I am not the only one that didn't like the JC Issue reporting. Even now with issues.joomla.org and Github it seems to much trouble!

Then again, for a no-brainer like this!

avatar Bakual
Bakual - comment - 20 Oct 2014

Then again, for a no-brainer like this!

Usually, the no-brainers break stuff.

Actually, I think you're trying to solve this at the wrong place. Since it works for PostgreSQL and MySQL, I suspect the issue in the MS SQL query class.
Looking at the MySQLi one, I see a similar check done there:
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/database/query/mysqli.php#L46
PostgreSQL has also a bit a different check:
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/database/query/postgresql.php#L582

MSSQL on the other hand has no such checks and looks quite a bit different:
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/database/query/sqlsrv.php#L263

Thus I'd say fix the MSSQL one, and don't mess with the query itself where you possibly break something for the other drivers.
If you only make a change in MSSQL, it would also be easier to merge based on review or forum tests. Since it will have no effect on other installations for sure.

avatar Bakual
Bakual - comment - 21 Oct 2014

Please see #4870

avatar sovainfo
sovainfo - comment - 21 Oct 2014

In the wild it is proven not to break. If only BUGS were as easy fixed as they are introduced to the software!

You are not solving the real issue. See #4870.

avatar Bakual
Bakual - comment - 21 Oct 2014

You are not solving the real issue.

It does actually exactly the same thing as your PR here, but only for MSSQL while yours would affect all database drivers.

avatar sovainfo
sovainfo - comment - 21 Oct 2014

You should look at the big picture and not compare individual PR's. As mentioned there are several issues, not only the one you are talking about!

avatar roland-d roland-d - change - 27 Nov 2014
Rel_Number 4870
Relation Type Related to
avatar roland-d
roland-d - comment - 27 Nov 2014

I do agree that the change provided in #4870 is a better one. This makes the MSSQL driver consistent with the other drivers.

Indeed, there are more issues with MSSQL but there just aren't enough people testing them. At least not here on the issue tracker.

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

avatar Bakual
Bakual - comment - 3 Dec 2014

Closing as fixed with #5293

avatar Bakual Bakual - change - 3 Dec 2014
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2014-12-03 07:56:13
avatar Bakual Bakual - close - 3 Dec 2014

Add a Comment

Login with GitHub to post a comment