Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0] Improve article list performance on multilingual sites #26465

Merged
merged 7 commits into from Oct 6, 2019
Merged

[4.0] Improve article list performance on multilingual sites #26465

merged 7 commits into from Oct 6, 2019

Conversation

SharkyKZ
Copy link
Contributor

@SharkyKZ SharkyKZ commented Oct 4, 2019

Pull Request for Issue #25770.

Summary of Changes

Replaces JOIN and GROUP clauses in query with a subquery.

Testing Instructions

Set up a multilingual site.
Enable associations.
Create some articles and add associations.
View article list in backend.

Expected result

Works like before.

Documentation Changes Required

No.

@richard67
Copy link
Member

richard67 commented Oct 4, 2019

I have tested this item ✅ successfully on 10687ec

Tested with MySQL 5.7 and PostgreSQL 10.10.

About performance I can't say anything because my test data set is too small.

But I can confirm everything works as well as before, carefully watching PHP error log with PHP error reporting set to maximum and watching database log.

There is a PHP notice "Undefined index: link in /layouts/joomla/content/associations.php on line 18" at least when using PostgreSQL before and after having applied this PR, so not related to this PR. This just as info for other testers.


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

@alikon
Copy link
Contributor

alikon commented Oct 4, 2019

What language have you used in the installation step ?
i've tryied persian , french always got this trying to add multilanguage content sample data
Screenshot from 2019-10-04 19-09-51

@richard67
Copy link
Member

Sometimes content languages are not added after language install. I've helped myself then with uninstalling languages and installing again.

@alikon
Copy link
Contributor

alikon commented Oct 4, 2019

seems that German - DE allows me to run sample multilanguage data (just for note on postgresql now)

@richard67
Copy link
Member

I'm using German (DE), French and for RTC sometimes Persian.

@SharkyKZ
Copy link
Contributor Author

SharkyKZ commented Oct 5, 2019

@alikon you may have to create a Content Language manually.

@richard67
Copy link
Member

I have tested this item ✅ successfully on 89e6b32

Tested with MySQL 5.7 and PostgreSQL 10.10.

About performance I can't say anything because my test data set is too small.

But I can confirm everything works as well as before, carefully watching PHP error log with PHP error reporting set to maximum and watching database log.


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

@alikon
Copy link
Contributor

alikon commented Oct 5, 2019

@SharkyKZ ok maybe another issue to open if it will be still there when we will have languages pack ready for 4, anyway PR seems fine & all works as before , despite i don't have a good big dataset to do some real case performance check... i cannot trust my EXPLAIN with my short data , but it looks fine and i cannot guess how it can works worst than now...

@alikon
Copy link
Contributor

alikon commented Oct 5, 2019

I have tested this item ✅ successfully on 89e6b32


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

@alikon
Copy link
Contributor

alikon commented Oct 5, 2019

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 5, 2019
@wilsonge wilsonge merged commit 42a24f4 into joomla:4.0-dev Oct 6, 2019
@wilsonge
Copy link
Contributor

wilsonge commented Oct 6, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants