Comment 2 for bug 1989740

Been stalking this one for a while and I'm going to try to put down my thoughts as understandably as possible.

Technically, this problem lies in the SQL function action.copy_related_hold_stats(copy_id BIGINT) at /Open-ILS/src/sql/Pg/100.circ_matrix.sql - but the scope of addressing it keeps zooming out the more I think.
action.copy_related_hold_stats() is given the ID of a row in asset.copy and then attempts to return the number of available copies of that item divided by the number of holds targeting that item.
So all we need to do is add a check to see whether or not the item is age protected to the statement to the lines tallying up the available copies, right?

Wrong. Just because the item HAS age protection enabled doesn't mean it WOULD be age protected - it might have expired. And... it depends on where you are. Age protection not only cares how long it has been, but also whether the item would go to an org_unit outside of the range of its protection - there are different values to allow items to go different distances for different durations. The concerto settings allow you to restrict items to 3 months within the same library or 6 months within the same library system, and different consortiums could use different distances and durations as well.

Okay, well, then I'll just take the logic for age protection from... hold_matrix.sql? Age protection, as it currently is, is used to prevent a hold from being placed, not to prevent circulations from happening. This makes sense - if a patron physically found the item, it shouldn't have gotten to a library different than its home library unless it had been shipped there - and the only reason (I think) items are shipped AWAY from their home libraries is because they've been placed on hold.

So, I try to extract the logic for checking if age protection would stop a hold. It's in 110.hold_matrix_sql in the function action.hold_request_permit_test(). Hopefully, I can make it check all the things it needs to know in any context, to make copy_related_hold_stats() and hold_request_permit_test use only one function. I named it action.does_age_protection_allow_use_at() Except...

Age protection needs to know how to calculate distance.
When an item is part of a floating collection, one might measure the distance of how far one org_unit is from another using a different library than where it stays: its owning library. Where is this stored? On the hold_matrix, with all the rules about when a hold is allowed. If I pulled the check for age protection out, it would need to find the hold matchpoint again.

At this point, I take a step back and think about the big picture. In order to figure out if ONE renewal is allowed to happen, you would need to search the entire hold_matrix table as many times as there are holds on that item in order to find the rules that apply to each hold on that item. Agh, my performance! That's O(n^2) on something that should be O(n)!

But that's... kinda the expected behavior, right? It sounds like the purpose in the ticket is that everyone who wants a turn gets a turn. That only the people who COULD get their hands on this item are counted. That means that the copy_related_hold_stats() in general needs to run a permit test on every hold in order for it to be counted against letting the patron renew their book now.

So the things I feel like I need input on are
[1] Could we afford a performance hit for this increase in correctness (find rules for every hold, then permit test on all of them)
[2] Is there a more performant way to store an age protection check, or make it easier to fetch the matchpoint?

Branch in progress.