Comment 9 for bug 1496522

Revision history for this message
Galen Charlton (gmc) wrote :

I've looked at it, and while it appears that it would work as expected for single-branch systems, it doesn't look like it will handle multi-branch systems and consortia as well as it ought to.

In particular:

- there's no way to aggregate user balances at an OU and its descendants, meaning that a patron who uses more than one branch in a system would be getting a billing notice from each branch
- the template bases the from address on the user's home OU, but doesn't include anything to indicate that the billing may be from an entirely different system

Also, some testing of the new views in a test database with 30 million circulation rows gives me pause:

# explain analyze SELECT COUNT(*) FROM money.usr_summary_per_org_unit WHERE billing_ou = 123; QUERY PLAN
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Aggregate (cost=7798960.10..7798960.11 rows=1 width=0) (actual time=59222.777..59222.777 rows=1 loops=1)
   -> HashAggregate (cost=7797444.95..7798118.35 rows=67340 width=8) (actual time=59221.012..59222.440 rows=6253 loops=1)
         Group Key: mmbts.usr, located_xact.billing_ou
         CTE located_xact
           -> Unique (cost=6323431.77..6559224.81 rows=31439072 width=12) (actual time=39596.034..51380.077 rows=31436345 loops=1)
                 -> Sort (cost=6323431.77..6402029.45 rows=31439072 width=12) (actual time=39596.032..46771.153 rows=31436345 loops=1)
                       Sort Key: circulation.id, circulation.circ_lib
                       Sort Method: external merge Disk: 676072kB
                       -> Append (cost=0.00..1640755.44 rows=31439072 width=12) (actual time=0.031..8541.612 rows=31436345 loops=1)
                             -> Seq Scan on circulation (cost=0.00..1315601.01 rows=30938501 width=12) (actual time=0.029..6779.568 rows=30935769 loops=1)
                             -> Seq Scan on grocery (cost=0.00..10762.41 rows=500541 width=12) (actual time=0.019..79.894 rows=500546 loops=1)
                             -> Seq Scan on reservation (cost=0.00..1.30 rows=30 width=12) (actual time=0.007..0.014 rows=30 loops=1)
         -> Nested Loop (cost=0.56..1237434.17 rows=157195 width=8) (actual time=39851.844..59120.777 rows=435150 loops=1)
               -> CTE Scan on located_xact (cost=0.00..707379.12 rows=157195 width=12) (actual time=39851.793..57277.208 rows=435150 loops=1)
                     Filter: (billing_ou = 123)
                     Rows Removed by Filter: 31001195
               -> Index Scan using materialized_billable_xact_summary_pkey on materialized_billable_xact_summary mmbts (cost=0.56..3.36 rows=1 width=12) (actual time=0.004..0.004 rows=1 loops=435150)
                     Index Cond: (id = located_xact.id)
 Planning time: 0.495 ms
 Execution time: 59442.586 ms
(20 rows)

In the context of this specific A/T event definition, the sequential scans on action.circulation and action.circulation aren't necessarily a huge deal, but it would (a) add up if a lot of libraries in a consortium activate the billing statement and (b) would be a problem if those views start getting used in other contexts. I suspect they can be made to perform better.

Upshot: a good start, but I think this needs more work.