[#14011] - Use simpler db query for admin mod_menu, without GROUP BY
- Fixed in Code Base
- 6 Jul 2017
- Medium
- Build: staging
- # 14011
- Diff
- csthomas:speed_amenu
User tests: Successful: Unsuccessful:
Pull Request for Issue #12983 and in general.
Summary of Changes
Use more simpler and faster query and return the same result.
- return the same result
- do not use GROUP BY
Testing Instructions
- Go to backend and check whether menu items display the same list of items before and after patch.
- It would be good to test it on multi language menu.
Before query:
SELECT a.*, SUM(b.home) AS home,b.language,l.image,l.sef,l.title_native
FROM j37_menu_types AS a
LEFT JOIN j37_menu AS b ON b.menutype = a.menutype AND b.home != 0
LEFT JOIN j37_languages AS l ON l.lang_code = language
WHERE (b.client_id = 0 OR b.client_id IS NULL)
GROUP BY a.id, a.menutype, a.description, a.title, b.menutype,b.language,l.image,l.sef,l.title_native
After query:
SELECT a.id, a.asset_id, a.menutype, a.title, a.description, a.client_id,c.home, c.language, c.image, c.sef, c.title_native
FROM j37_menu_types AS a
LEFT JOIN (
SELECT b.menutype, b.home, b.language, l.image, l.sef, l.title_native
FROM j37_menu AS b
LEFT JOIN j37_languages AS l ON l.lang_code = b.language
WHERE b.home != 0 AND (b.client_id = 0 OR b.client_id IS NULL)
) c ON c.menutype = a.menutype
ORDER BY a.id
Expected result
Works as before but for tables with more rows it works faster.
Documentation Changes Required
None
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Modules Administration |
I'm not sure I understand you correctly.
This PR is not related to single db type.
I have tested it on localhost on mysql and a few minutes ago on mssql too.
This PR replace current joomla query ... GROUP BY ... with simpler one which does not require GROUP BY. Results of query should be the same across all DBs.
I know it's not. But given the group by comment is Sqlsrv change it seemed prudent to check this didn't affect sqlsrv :)
I like the concept for sure!
I assume you talk about ef3bafc#diff-a296558abf749881f99f0297bb5c725bL38
There was change which support ONLY_FULL_GROUP_BY.
What I have read J4 will required it for all queries.
For this PR I changed the entire query to another one which works in different way. I ask for tests on multilingual joomla to be sure that I not make a mistake in my theory:)
| Title |
|
||||||
| Labels |
Added:
?
|
||||||
@ggppdk I have changed the code to fit your needs.
I do not want to change query but one php loop seems to resolve your problem.
It seems that your PHP loop tries to count multiple home menu items inside same menu,
thus achieving the same result as doing it at the SQL Query
sorry no time to test and verify if it works like this:
#12991 (comment)
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14011.
The last commit (add order by a.id) is for types of database other than mysql innodb.
After a discussion in bugsquad, I suggest to let open a future implementation of multiple home pages (when set to a lang else than ALL). This would be done in the libraries.
I.e. I suggest to just concentrate on the faster query if really necessary.
1.7 is a dead fish.
I have removed old workaround for multiple home pages.
Now is only optimised query.
didn't tested on a big #__menu table but EXPLAIN looks far better now
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14011.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14011.
| Status | Pending | ⇒ | Ready to Commit |
RTC after two successful tests.
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2017-07-06 10:31:34 |
| Closed_By | ⇒ | rdeutz | |
| Labels |
Added:
?
|
||
Given you've been doing all the sqlsrv patches I'm assuming you've tested this on there given the group by comment?