money.transaction_billing_summary view displays incorrect billing_type and billing_note for the actual last transaction

Thank you to Mike Rylander who proposed this solution to the problem with that particular view:

CREATE OR REPLACE VIEW money.transaction_billing_summary AS
    SELECT DISTINCT xact,
        LAST_VALUE(billing_type) OVER w AS last_billing_type,
        LAST_VALUE(note) OVER w AS last_billing_note,
        MAX(billing_ts) OVER w AS last_billing_ts,
        SUM(COALESCE(amount,0)) OVER w AS total_owed
      FROM money.billing
      WHERE voided IS FALSE
      WINDOW w AS (PARTITION BY xact ORDER BY billing_ts ROWS BETWEEN UNBOUNDED PRECEDING AND UNBOUNDED FOLLOWING);

As he mentioned, there are many others to convert to using window functions. I have identified a list:

-- money.billable_xact_summary_new
-- money.billable_xact_with_void_summary
-- money.open_transaction_billing_summary
-- money.open_transaction_billing_type_summary
-- money.open_transaction_payment_summary
-- money.transaction_billing_type_summary
-- money.transaction_billing_with_void_summary
-- money.transaction_payment_summary
-- money.transaction_payment_with_void_summary

I'm not sure how involved changing all of those would be, but it sounds like project.

working branch on this: http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/csharp/update_money_views_with_window_functions

I have tested the upgrade script on a PINES test DB, but not installation.

Pulling this out of limbo for review during the next 2.6 rc1.

Chris, it looks like the first hunk of the patch at commit 765aa89f6 is missing a "LAST() OVER w" wrapper on last_billing_type. Is that correct?

So is there additional work that Chris needs to do before this branch is ready for testing?

Changing to High priority because it relates pretty directly to patrons and it has to do with money. Both very important and potentially painful things.

That said, trying the correct view resulted in terrible performance on our 9.1 database:

Here's the incorrect, original view:

EXPLAIN ANALYZE SELECT * FROM money.transaction_billing_summary WHERE xact = 1055743;
                                                              QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------
 Sort (cost=27.31..27.32 rows=1 width=72) (actual time=0.427..0.427 rows=1 loops=1)
   Sort Key: (max(billing.billing_ts))
   Sort Method: quicksort Memory: 25kB
   -> GroupAggregate (cost=0.00..27.30 rows=1 width=72) (actual time=0.412..0.412 rows=1 loops=1)
         -> Index Scan using m_b_xact_idx on billing (cost=0.00..17.65 rows=19 width=72) (actual time=0.059..0.145 rows=32 loops=1)
               Index Cond: (xact = 1055743)
               Filter: (voided IS FALSE)
 Total runtime: 0.514 ms
(8 rows)

Here's the correct, windowed view:

EXPLAIN ANALYZE SELECT * FROM money.transaction_billing_summary WHERE xact = 1055743;
                                                                   QUERY PLAN
-------------------------------------------------------------------------------------------------------------------------------------------------
 Subquery Scan on transaction_billing_summary (cost=389206.01..397046.79 rows=1742 width=112) (actual time=16954.467..17031.868 rows=1 loops=1)
   Filter: (transaction_billing_summary.xact = 1055743)
   -> HashAggregate (cost=389206.01..392690.80 rows=348479 width=72) (actual time=16930.022..17007.854 rows=135940 loops=1)
         -> WindowAgg (cost=302984.69..362261.85 rows=2155533 width=72) (actual time=8766.521..14677.489 rows=2161543 loops=1)
               -> Sort (cost=302984.69..308373.53 rows=2155533 width=72) (actual time=8766.454..10138.737 rows=2161543 loops=1)
                     Sort Key: billing.xact, billing.billing_ts
                     Sort Method: external merge Disk: 170368kB
                     -> Seq Scan on billing (cost=0.00..76226.79 rows=2155533 width=72) (actual time=0.014..1353.749 rows=2161543 loops=1)
                           Filter: (voided IS FALSE)
 Total runtime: 17091.181 ms
(10 rows)

After creating an index such as CREATE INDEX CONCURRENTLY m_b_xact_time_idx ON money.billing (xact, billing_ts);, that drops to the following:

EXPLAIN ANALYZE SELECT * FROM money.transaction_billing_summary WHERE xact = 1055743;
                                                                            QUERY PLAN
------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Subquery Scan on transaction_billing_summary (cost=234806.53..242647.30 rows=1742 width=112) (actual time=8516.624..8587.78...

Alternate approach suggested by Mike Rylander: build on the materialized summary table, which will have way fewer rows, and also not require gnarly SQL. So for example:

CREATE OR REPLACE VIEW money.transaction_billing_summary AS SELECT id AS xact, last_billing_type, last_billing_note, last_billing_ts, total_owed FROM money.materialized_billable_xact_summary;

Performance:

EXPLAIN ANALYZE SELECT * FROM money.transaction_billing_summary WHERE xact = 1055743;
                                                                                 QUERY PLAN
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Index Scan using materialized_billable_xact_summary_pkey on materialized_billable_xact_summary (cost=0.00..6.45 rows=1 width=72) (actual time=0.070..0.070 rows=1 loops=1)
   Index Cond: (id = 1055743)
 Total runtime: 0.094 ms
(3 rows)

Seems good!

Removing milestone targets while this one is put into a working branch for proper review and inclusion. Looks good though dbs! (faster is better!)

I've created a signed-off working branch based on Mike's and Dan's approach, which I've confirmed works on PINES test data. While this doesn't address the consistency issue I brought up in comment #1 above, it does solve the problem reported in the original bug report.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/csharp/lp1206936_correct_last_billing_info_mtbs

Added a (somewhat convoluted) test and pushed to master through 2.8. Thanks, Chris et al.!

Heads up.. Pushed working/user/berick/lp1206936-base-schema-repair to fix a problem with the ordering in the base SQL schema.

money.materialized_billable_xact_summary was referenced before it was defined.

And I've pushed Bill's fix to master. Thanks Bill and Dan and Chris!