The SQL query to find the database max value for annual barcodes is: 42 my $query = "SELECT max(substring_index(barcode,'-',-1)) AS chunk,barcode FROM items WHERE barcode LIKE ? GROUP BY barcode"; 43 # FIXME: unreasonably expensive query on large datasets The 'max' in the first column creates confusion and disrupts the normal pairing of the db. To recreate: 1 - Add two 'annual' type barcodes to the system (2016-0003, 2016-0006) 2 - Run the query: SELECT max(substring_index(barcode,'-',-1)) AS chunk,barcode FROM items WHERE barcode LIKE '2016-%' GROUP BY barcode 3 - Note that returned values may not pair up, in my case I got things like: chunk | barcode 0006 | 2016-0003
Created attachment 48363 [details] [review] Bug 15877 - C4::Barcodes does not correctly calculate db_max for 'annual' barcodes This patch alters the MySQL query to remove a max statement which incorrectly groups full annual barcodes with numeric values.
Created attachment 48365 [details] [review] Bug 15877 - Unit tests
Test plan: 1 - prove t/db_dependent/Barcodes.t 2 - All should pass 3 - Apply second patch (unit tests update) 4 - Test 3 should fail (annual) db_max equals greatest value when more than 1 present 5 - Apply first patch 6 - All tests should now pass
Created attachment 48367 [details] [review] Bug 15877 - Unit tests
Created attachment 49197 [details] [review] [SIGNED-OFF] Bug 15877 - Unit tests Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Created attachment 49198 [details] [review] [SIGNED-OFF] Bug 15877 - C4::Barcodes does not correctly calculate db_max for 'annual' barcodes This patch alters the MySQL query to remove a max statement which incorrectly groups full annual barcodes with numeric values. Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Comment on attachment 49197 [details] [review] [SIGNED-OFF] Bug 15877 - Unit tests Review of attachment 49197 [details] [review]: ----------------------------------------------------------------- A warning is raised by C4::Barcodes, use warning_is to catch it. ::: t/db_dependent/Barcodes.t @@ +53,5 @@ > + barcode => $barcodevalue > + } > +}); > + > +ok($barcodevalue eq $barcodeobj->db_max(), "(annual) First barcode saved to db is equal to db_max" ); use is instead of ok. @@ +59,5 @@ > +$barcodevalue = $barcodeobj->next_value(); > +$barcodevalue = $barcodeobj->next_value($barcodevalue); > +$barcodevalue = $barcodeobj->next_value($barcodevalue); > +$barcodevalue = $barcodeobj->next_value($barcodevalue); > +$barcodevalue = $barcodeobj->next_value($barcodevalue); Why are you calling 4x this method?
Created attachment 49600 [details] [review] Bug 15877 - Unit tests Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
(In reply to Jonathan Druart from comment #7) > Review of attachment 49197 [details] [review] [review]: > use is instead of ok. ok (or should I say is ;-) ) > @@ +59,5 @@ > > +$barcodevalue = $barcodeobj->next_value(); > > +$barcodevalue = $barcodeobj->next_value($barcodevalue); > > +$barcodevalue = $barcodeobj->next_value($barcodevalue); > > +$barcodevalue = $barcodeobj->next_value($barcodevalue); > > +$barcodevalue = $barcodeobj->next_value($barcodevalue); > Why are you calling 4x this method? To set the value higher without using any constants. It's arbitrary to use 4, I just knew that triggered the problem for me each time and it felt more illustrative than 1
Created attachment 49926 [details] [review] Bug 15877 - Unit tests Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Created attachment 49927 [details] [review] Bug 15877 - Unit tests Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Created attachment 49928 [details] [review] Bug 15877 - Unit tests Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Edited the: $dbh->do(q|DELETE FROM items|); And replaced with: $builder->clear( { source => 'Item' } ); Same for issues, a test crash wiped my database of all items so not an ideal construction
Created attachment 49961 [details] [review] Bug 15877 - C4::Barcodes does not correctly calculate db_max for 'annual' barcodes This patch alters the MySQL query to remove a max statement which incorrectly groups full annual barcodes with numeric values. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 49962 [details] [review] Bug 15877 - Unit tests Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed to Master - Should be in the May 2016 release. Thanks!
Patches pushed to 3.22.x, will be in 3.22.7