I cannot find the root cause of this issue, but multiple libraries that I am aware of have problems searching on particular search terms ( and never the same terms at the same library ). The error they get when they trigger this problem is: Tag "" is not a valid tag. at /home/koha/kohaclone/C4/Biblio.pm line 1849 Something somewhere is adding empty keys to C4::Context->marcfromkohafield, I think it may have something to do with the analytics feature that was added. In the while loop for TransformKohaToMarc, there is a line next unless my $dtm = $db_to_marc->{''}->{$name}; I don't think it's working. If I dump $dtm, for each search, I see the dump twice. It looks like this: $VAR1 = [ '952', 'w' ]; $VAR1 = []; I think the second time, when it is empty is what's breaking this. The next never fails because even though it is empty, it is still a valid arrayref. The solution I have some up with is to skip over the elements where the arrayref is empty.
Created attachment 8219 [details] [review] Bug 7722 - Insidious problem with searching I cannot find the root cause of this issue, but multiple libraries that I am aware of have problems searching on particular search terms ( and never the same terms at the same library ). The error they get when they trigger this problem is: Tag "" is not a valid tag. at /home/koha/kohaclone/C4/Biblio.pm line 1849 Something somewhere is adding empty keys to C4::Context->marcfromkohafield, I think it may have something to do with the analytics feature that was added. In the while loop for TransformKohaToMarc, there is a line next unless my $dtm = $db_to_marc->{''}->{$name}; I don't think it's working. If I dump $dtm, for each search, I see the dump twice. It looks like this: $VAR1 = [ '952', 'w' ]; $VAR1 = []; I think the second time, when it is empty is what's breaking this. The next never fails because even though it is empty, it is still a valid arrayref. The solution I have some up with is to skip over the elements where the arrayref is empty.
Do you have any suggestions how to test this patch?
(In reply to comment #2) > Do you have any suggestions how to test this patch? Unfortunately not, I was never able to find the root cause of the issue. I believe the issue is probably being caused by the analytics feature, but I was never able to pinpoint the exact code that was causing this problem. The error has showed up on multiple Koha installations I have worked on. I can say the patch does indeed fix it, and does no harm. It is just another check to ensure data correctness. The original check for an arrayref is simply insufficient, as an arrayref can be valid, but have no data.
Created attachment 9566 [details] [review] [PASSED QA] Bug 7722 - Insidious problem with searching I cannot find the root cause of this issue, but multiple libraries that I am aware of have problems searching on particular search terms ( and never the same terms at the same library ). The error they get when they trigger this problem is: Tag "" is not a valid tag. at /home/koha/kohaclone/C4/Biblio.pm line 1849 Something somewhere is adding empty keys to C4::Context->marcfromkohafield, I think it may have something to do with the analytics feature that was added. In the while loop for TransformKohaToMarc, there is a line next unless my $dtm = $db_to_marc->{''}->{$name}; I don't think it's working. If I dump $dtm, for each search, I see the dump twice. It looks like this: $VAR1 = [ '952', 'w' ]; $VAR1 = []; I think the second time, when it is empty is what's breaking this. The next never fails because even though it is empty, it is still a valid arrayref. The solution I have some up with is to skip over the elements where the arrayref is empty. Signed-off-by: Ian Walls <koha.sekjal@gmail.com>
This is a consequence of bug 6990, which had a long thread and a proposed solution similar to what you're proposing, Kyle, but no actual follow-up patch written. Testing is... tricky. TransformKohaToMarc isn't used very often.... it's a called as part of GetMarcItem, and in acqui/addorder.pl. Testing would involve editing items, and adding orders. That said, I think Kyle's approach is sound. We're only adding an additional line that will 'next' us if the count of elements returned in $dtm is zero. Adding that check is indeed safe, as there should be never be a circumstance where NULL $tag or $letter should be okay. I'm going to sign off on this and pass it through QA, as I'm quite confident this is a safe and logical addition to the code.
(In reply to comment #5) > I'm going to sign off on this and pass it through QA, as I'm quite confident > this is a safe and logical addition to the code. Agreed, (even if I think it would be better to fix the origin of the empty dtm)
Bug fix, no string changes, will be in 3.8.1
This patch will be in 3.6.6.