reduces number of db queries in cart validations #680
Conversation
|
Cart#validate_items now runs constant time queries (about a 1000% improvement using a cart with a date range of 10 days and 3 items) Cart#validate_dates_and_items runs 2x(number of e models) + 4 queries (about a 300% improvement with the same cart setup detailed above) Changing dates now takes <1000ms in basically all use cases and adding/remove items to the cart take around 500ms (on my system) |
| errors = [] | ||
| relevant = Reservation.for_reserver(self.reserver_id).not_returned | ||
| relevant = Reservation.for_reserver(self.reserver_id).not_returned.all |
There was a problem hiding this comment.
Out of curiosity, why was this necessary?
There was a problem hiding this comment.
I think it's because the previous version just specified the Relation but didn't actually load anything. On the upside, that enabled scope-chaining; on the downside, (probably) multiple loads. Am I understanding it right, @squidgetx?
|
Two inline comments, otherwise this looks really good. It passes all specs when I just run |
|
I just noticed that all the validations should be retriggered whenever the reserver id is changed, so it no longer really makes sense to separate by validate_items and validate_date. I've reorganized the file into units as @shippy suggested in #644 and adjusted the comments as necessary but not much of the code has really changed (although each individual validation is now much more accessible from the outside, which will be important when dealing with renewals) |
reduces number of db queries in cart validations
| errors << check_start_date_blackout | ||
| errors << check_due_date_blackout | ||
| errors << check_overdue_reservations | ||
| errors << check_max_items |
There was a problem hiding this comment.
Just a sanity check, but all these validation units return an array of errors, right? Will the << operator extend the errors array (i.e. merge the two arrays), or append the return array of errors to it?
Queries don't work the way I thought they work, unfortunately.
This PR reduces the query count of cart#validate_items to O(1) and reduces the query count of cart#validate_dates_and_items by about half (though this is still O(n) queries, mostly due to a faulty num_available method).