In Circulation.pm : GetItemIssue returns informations about issues and items using a LEFT JOIN : SELECT * FROM issues LEFT JOIN items ON issues.itemnumber=items.itemnumber The bug is that the fields "renewals" exists in both issues and items table. With SELECT *, only one of the two fields will be retrieved (mostly issues.renewals). There are other queries using JOIN between issues and items. Two solutions : - Not use SELECT * and define an alias for items.renewals, in each queries. - rename items.renewals in items.itemrenewals, in database model. Regards,
SELECT * is generally frowned on in db circles and issues.renewals and items.renewals describe the contents of these fields accurately. If one changes name theres not really a good way of recalling which one has been aliased (or why if dealing only with that table)
(In reply to comment #1) > SELECT * is generally frowned on in db circles and issues.renewals and > items.renewals describe the contents of these fields accurately. If one changes > name theres not really a good way of recalling which one has been aliased (or > why if dealing only with that table) I agree, the LEFT JOIN is only to get some fields from items. We should change SELECT * in :
SELECT issues.*, items.homebranch, items.holdingbranch, items.notforlaon, ... (end of previous comment)
Created attachment 5225 [details] [review] Proposed patch
Patch solves the problem by always adding "items.renewals AS totalrenewals" in queries where issues and items are joined. This behavior comes from Member.pm : GetPendingIssues and GetAllIssues Please test. Reads,
GBSD#4 taking that.
QA comment : * OK, problem is real, but where can I check the differences ? (please, a test plan !) * There's another problem (that existed before this patch SELECT * FROM issues LEFT JOIN biblioitems LEFT JOIN items will return all xml, that is useless & require memory & CPU. I suggest opening another bug for this (Fridolyn, would you please ?) * why didn't you use the solution you've suggested in comment 3 ? Marking failed QA until you've answered those concerns
Is the issue still exist? Can we have an example?
This issue still exists in that the query in Circulation.pm is still the same, and there are still renewals columns in both items and issues. I think we still have yet to see a test plan which shows how the bug manifests.
GetItemIssue has been fixed by commit 5b4752e027ce32492166c7018d44d4a687afae36 Bug 8365: FIX Return issue values instead of item values GetItemIssues is still buggy, but the only place 'renewals' is used is in catalogue/issuehistory.tt: [% IF ( issue.renewals ) %] And it happens only if the itemnumber parameter is passed to the script. However this parameter is never passed from Koha. That means we could remove the subroutine GetItemIssues as well as the related code in the pl and tt. Would you agree to remove it?
(In reply to Jonathan Druart from comment #10) > Would you agree to remove it? Yep
Obsolete since Bug 17679 and Bug 17680