-
Notifications
You must be signed in to change notification settings - Fork 492
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
7398 saved search performance #7447
Conversation
@@ -60,31 +42,15 @@ | |||
|
|||
public List<Dataverse> findLinkingDataverses(Long datasetId) { | |||
List<Dataverse> retList = new ArrayList<>(); | |||
for (DatasetLinkingDataverse dld : findDatasetLinkingDataverses(datasetId)) { | |||
retList.add(dld.getLinkingDataverse()); | |||
Query query = em.createQuery("select object(o) from DatasetLinkingDataverse as o where o.dataset.id =:datasetId order by o.id"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason this wasn't added as a named query?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, just because I was only changing the ones related specifically to saved search. I did think about it. And sure, I can add the others too, while I'm at it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. passing to QA.
What this PR does / why we need it:
Adds some optimization for saved searches and some code cleanup
Which issue(s) this PR closes:
None for sure - it may fix #7398if the performance is improved enough
Special notes for your reviewer:
While I was at it, I made some Named queries and did some cleanup.
Suggestions on how to test this:
just make sure saved searches work as expected. You could also compare the timing of how long it takes to update a saved search before and after. (I will be doing some timing test with prod once released)
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No
Is there a release notes update needed for this change?:
No
Additional documentation: