Bug 7846 - get_batch_summary reimplements GROUP BY in perl code
Summary: get_batch_summary reimplements GROUP BY in perl code
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Label/patron card printing (show other bugs)
Version: 3.10
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Dobrica Pavlinusic
QA Contact: Ian Walls
URL:
Keywords:
Depends on:
Blocks: 7119
  Show dependency treegraph
 
Reported: 2012-03-29 13:38 UTC by Dobrica Pavlinusic
Modified: 2013-12-05 20:04 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 7846 - get_batch_summary massive speedup (2.19 KB, patch)
2012-03-29 13:43 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 7864: Reintroduce list of subscribers to a serial alert message (1.77 KB, patch)
2012-04-01 07:56 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7846: Little changes to the display (2.46 KB, patch)
2012-04-01 12:35 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 7846 - get_batch_summary massive speedup (2.25 KB, patch)
2012-04-07 07:04 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 7846 - get_batch_summary massive speedup (2.62 KB, patch)
2012-04-13 10:49 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 7846 - get_batch_summary massive speedup (2.34 KB, patch)
2012-04-13 10:54 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 7846 - get_batch_summary massive speedup (2.40 KB, patch)
2012-05-14 00:43 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Dobrica Pavlinusic 2012-03-29 13:38:02 UTC

    
Comment 1 Dobrica Pavlinusic 2012-03-29 13:43:39 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2012-04-01 07:56:48 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2012-04-01 07:59:42 UTC
Comment on attachment 8739 [details] [review]
Bug 7864: Reintroduce list of subscribers to a serial alert message

Wrong bug
Comment 4 Katrin Fischer 2012-04-01 12:35:48 UTC Comment hidden (obsolete)
Comment 5 Katrin Fischer 2012-04-01 12:37:27 UTC
Comment on attachment 8742 [details] [review]
Bug 7846: Little changes to the display

Wrong bug again, sorry.
Comment 6 Dobrica Pavlinusic 2012-04-02 21:18:37 UTC
I'm still hoping for signoff :-)
Comment 7 Chris Cormack 2012-04-07 07:01:06 UTC
If Katrin stops putting every other patch here I'll be able to sign off :)

(Now I have to hide from Katrin).
Comment 8 Chris Cormack 2012-04-07 07:04:55 UTC Comment hidden (obsolete)
Comment 9 Chris Cormack 2012-04-07 07:06:45 UTC
Works as advertised, with the advertised speed increase
Comment 10 Ian Walls 2012-04-09 17:26:03 UTC
Saves us from running numerous extra queries in a loop of indefinite size.  Good catch!  Marking as Passed QA.
Comment 11 Paul Poulain 2012-04-12 16:23:36 UTC
yikes: on my sample database there is no patch at all and =
* without the patch the page /cgi-bin/koha/patroncards/manage.pl?card_element=batch load smoothly
* with the patch I get a nasty:
Can't use string ("-1") as an ARRAY ref while "strict refs" in use at /home/paul/koha.dev/koha-community/C4/Creators/Lib.pm line 518.

Failed QA sorry !
Comment 12 Dobrica Pavlinusic 2012-04-13 10:47:58 UTC
Good catch. I missed semi-column ; which are embedded at end of query if there is filter parameter. Semi-columns are not needed, so I removed them.
Comment 13 Dobrica Pavlinusic 2012-04-13 10:49:31 UTC Comment hidden (obsolete)
Comment 14 Dobrica Pavlinusic 2012-04-13 10:54:06 UTC Comment hidden (obsolete)
Comment 15 Chris Cormack 2012-05-14 00:43:57 UTC
Created attachment 9545 [details] [review]
Bug 7846 - get_batch_summary massive speedup

current code is using DISTINCT and another SQL query which can be replaced with GROUP BY
for massive speedup. In our case, generating Manage Batches screen DBI time decreased
from 24.762 s to 0.147 s

Aside from correct usage of relational database, this change also cleans up code nicely.

This change removed semi-columns from SQL query which broke Manage Patron batches.

Test scenario:

1. open Manage Batches screen and take note of time needed to generate it
2. apply this patch
3. reload page and check page genration time

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 16 Ian Walls 2012-05-23 15:02:02 UTC
Same content as the patch I Passed QA before, but now with the semi-colon issue fixed.  Marking Passed once again.
Comment 17 Paul Poulain 2012-05-25 15:48:46 UTC
My test database does not have batches, so I can test, but I trust the author. Report any problem !
Comment 18 Jared Camins-Esakov 2012-12-31 01:01:51 UTC
There have been no further reports of problems so I am marking this bug resolved.