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

WIP: FIX Filtering the version history by a specific date is now more performant in large datasets #197

Conversation

robbieaverill
Copy link
Contributor

Fixes #194

cc @silverstripe/core-team

@robbieaverill robbieaverill changed the title FIX Filtering the version history by a specific date is now more performant in large datasets WIP: FIX Filtering the version history by a specific date is now more performant in large datasets Dec 3, 2018
@robbieaverill
Copy link
Contributor Author

Ok this clearly isn't going to work, will need to come up with something else

@tractorcow
Copy link
Contributor

Good try, but I don't think the logic would be correct trying to duplicate the conditions.

@robbieaverill
Copy link
Contributor Author

The tests that are failing are where the base query conditions include fields from an inner join, so the inner join for latest archived record can't use them. I also tried converting it to a subselect in a "where" condition, but that didn't have the same performance benefits. My next step will be to try and rewrite the query so it doesn't use joins at all, but I'm also happy to put this on the backburner for a while and wait for more input =)

@robbieaverill
Copy link
Contributor Author

I also tried converting it to a subselect in a "where" condition, but that didn't have the same performance benefits

I think I did it wrong, which was why. I've updated the PR and pushed a new commit which correctly moved the join logic into a where condition.

@robbieaverill
Copy link
Contributor Author

Still some bugs with this - will park it for now

@ScopeyNZ
Copy link
Contributor

ScopeyNZ commented Dec 4, 2018

Looks like you missed one other spot in the query. I've pushed an update 🤞

@ScopeyNZ
Copy link
Contributor

ScopeyNZ commented Dec 4, 2018

Still not working 😞

@robbieaverill
Copy link
Contributor Author

Closing, not ready for peer review

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

Successfully merging this pull request may close these issues.

3 participants