Summary: | incremental barcode generation fails when incorrectly converting strings to numbers | ||
---|---|---|---|
Product: | Koha | Reporter: | Pablo AB <pablo.bianchi> |
Component: | Cataloging | Assignee: | David Cook <dcook> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | normal | ||
Priority: | P5 - low | CC: | aleisha, amit.gupta, amitddng135, andrew, catherinem2003, dcook, jonathan.druart, kyle, m.de.rooy, marjorie.barry-vila, victor |
Version: | Main | Keywords: | Academy |
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
22.05.00,21.11.05,21.05.14,20.11.18
|
|
Circulation function: | |||
Attachments: |
Bug 26328: Cast barcode from varchar to integer for incremental barcode
Bug 26328: Cast barcode from varchar to integer for incremental barcode Bug 26328: Cast barcode from varchar to integer for incremental barcode Bug 26328: Add test |
Description
Pablo AB
2020-09-01 00:24:31 UTC
That's an interesting one. Your suggestion returns a value of 9223372036854775807. I would say that is an undesirable outcome as well. It's probably better to use something like this: SELECT MAX(ABS(barcode)) FROM items WHERE barcode REGEXP '^[0-9]+$'; You are right. On my tests: SELECT barcode FROM items ORDER BY ABS(CAST(barcode AS UNSIGNED)) DESC LIMIT 5; 770101E1 → 770101 (CAST on SELECT) SELECT barcode FROM items ORDER BY ABS(barcode) DESC LIMIT 5; 85350E65 → 8.535e69 (ABS on SELECT) Your proposal seems the way to go. I have a long TODO list, but I can look at adding a patch for this down the road. I actually ran into this problem again today, and at the moment the query I like best is actually the following: SELECT max(cast(barcode as unsigned)) from items; I'll write up a patch now... Created attachment 114179 [details] [review] Bug 26328: Cast barcode from varchar to integer for incremental barcode Without this patch, the incremental barcode generation will treat 978e0143019375 as having an exponent and interpret it as a very large number. With this patch, the incremental barcode generation will first cast barcode varchar strings to integers before finding a max() value. In this case 978e0143019375 becomes 978 instead of 1.7976931348623157e308 Test plan: 0. Using koha-testing-docker Before applying patch: 1. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=autobarcode 2. Set to "generated in the form 1, 2, 3" 3. Go to http://localhost:8081/cgi-bin/koha/cataloguing/additem.pl?biblionumber=1#additema&searchid=scs_1607059974968 4. Add item with barcode 978e0143019375 5. Click "p - Barcode" 6. Note the barcode is "Inf" After applying patch: 1. Go to http://localhost:8081/cgi-bin/koha/cataloguing/additem.pl?biblionumber=1#additema&searchid=scs_1607059974968 2. Click "p - Barcode" 3. Note the barcode is "39999000019194" (In reply to David Cook from comment #4) > I actually ran into this problem again today, and at the moment the query I > like best is actually the following: > > SELECT max(cast(barcode as unsigned)) from items; Why you like the most this one (except for the ABS identical to my first suggestion), instead of your alternative con comment 1? I remember that at the time your suggestion seems better. (In reply to Pablo AB from comment #6) > Why you like the most this one (except for the ABS identical to my first > suggestion), instead of your alternative con comment 1? I remember that at > the time your suggestion seems better. Option 1: SELECT MAX(ABS(barcode)) FROM items WHERE barcode REGEXP '^[0-9]+$'; This query won't generate any warnings, so that's good. But it's also fairly long and a little complex. Option 2: SELECT max(cast(barcode as unsigned)) from items; This query will show "Truncated incorrect..." warnings, but it's simpler/easier to read/maintain. -- In the end, it doesn't really matter too much I think. Is this ready for testing? If not can you please update the status appropriately? (In reply to Aleisha Amohia from comment #8) > Is this ready for testing? If not can you please update the status > appropriately? It was never properly signed off, so I think it's still in Needs Signoff, unfortunately. I followed the test plan, however it did not work accordingly. (In reply to Catherine from comment #10) > I followed the test plan, however it did not work accordingly. Can you describe how it did not work? It worked great last time I looked at it. Hi David, after applying the patch, I did step 2 however "39999000019194" did not appear. Nothing changed when I did step 2. Possible that I made an error during the test plan, so someone else should test again. (In reply to Catherine from comment #12) > Hi David, > after applying the patch, I did step 2 however "39999000019194" did not > appear. Nothing changed when I did step 2. Possible that I made an error > during the test plan, so someone else should test again. Hmm weird. I'll give it another go sometime too. Were you using a fresh koha-testing-docker? Or were you testing it with a different Koha dev setup? Created attachment 131592 [details] [review] Bug 26328: Cast barcode from varchar to integer for incremental barcode Without this patch, the incremental barcode generation will treat 978e0143019375 as having an exponent and interpret it as a very large number. With this patch, the incremental barcode generation will first cast barcode varchar strings to integers before finding a max() value. In this case 978e0143019375 becomes 978 instead of 1.7976931348623157e308 Test plan: 0. Using koha-testing-docker Before applying patch: 1. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=autobarcode 2. Set to "generated in the form 1, 2, 3" 3. Go to http://localhost:8081/cgi-bin/koha/cataloguing/additem.pl?biblionumber=1#additema&searchid=scs_1607059974968 4. Add item with barcode 978e0143019375 5. Click "p - Barcode" 6. Note the barcode is "Inf" After applying patch: 1. Go to http://localhost:8081/cgi-bin/koha/cataloguing/additem.pl?biblionumber=1#additema&searchid=scs_1607059974968 2. Click "p - Barcode" 3. Note the barcode is "39999000019194" Signed-off-by: Marjorie <marjorie.barry-vila@collecto.ca> Hi, I tested on sandbox https://sandbox.bywatersolutions.com/signoff/bz26328 and for me test plan worked well. Regards Marjorie Hey, you forgot the tests! Created attachment 131976 [details] [review] Bug 26328: Cast barcode from varchar to integer for incremental barcode Without this patch, the incremental barcode generation will treat 978e0143019375 as having an exponent and interpret it as a very large number. With this patch, the incremental barcode generation will first cast barcode varchar strings to integers before finding a max() value. In this case 978e0143019375 becomes 978 instead of 1.7976931348623157e308 Test plan: 0. Using koha-testing-docker Before applying patch: 1. Go to http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=autobarcode 2. Set to "generated in the form 1, 2, 3" 3. Go to http://localhost:8081/cgi-bin/koha/cataloguing/additem.pl?biblionumber=1#additema&searchid=scs_1607059974968 4. Add item with barcode 978e0143019375 5. Click "p - Barcode" 6. Note the barcode is "Inf" After applying patch: 1. Go to http://localhost:8081/cgi-bin/koha/cataloguing/additem.pl?biblionumber=1#additema&searchid=scs_1607059974968 2. Click "p - Barcode" 3. Note the barcode is "39999000019194" Signed-off-by: Marjorie <marjorie.barry-vila@collecto.ca> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 131977 [details] [review] Bug 26328: Add test Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to master for 22.05, thanks to everybody involved [U+1F984] Pushed to 21.11.x for 21.11.05 Pushed to 21.05.x for 21.05.14 Backported: Pushed to 20.11.x branch for 20.11.18 |