User tests: Successful: Unsuccessful:
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
Labels |
Removed:
?
|
Title |
|
Status | New | ⇒ | Pending |
ok, this fix work.
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!
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.
This has been tested and proven to work numerous times in the field. Just visit the forum!
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.
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!
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.
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.
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!
Rel_Number | ⇒ | 4870 | |
Relation Type | ⇒ | Related to |
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.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2014-12-03 07:56:13 |
@test ok, PR resolves my issue