[Koha-bugs] [Bug 26587] detail.pl is very slow for records with many items

[bugzilla-daemon at bugs.koha-community.org]

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26587

--- Comment #14 from Björn Nylén <bjorn.nylen at ub.lu.se> ---
(In reply to David Cook from comment #11)
> Hey Björn, have you tested these patches? 
> 
> I haven't tested them, but at a glance it looks like they contain errors.
> 
> 1)
> For instance, in the first one, $av_cache doesn't look like it's declared
> anywhere. I think that will probably throw a fatal error (or at best a
> warning). You should be declaring that before the loop in GetItemsInfo. 
> 
> You're also using fairly deep hash structures which could break. For
> instance:
> $av_cache->{$data->{frameworkcode}}->{'items.stack'}->{$data->{stack}}-
> >{lib} = $data->{stack};
> 
> If $data->{frameworkcode} or more likely $data->{stack} don't exist , then
> you're going to probably get a fatal error. You might want to wrap some of
> this cache code in a function with more checks/conditions in it.
> 
> 2)
> I think that the second patch will generate either fatal errors or warnings
> as well since you're not testing for the existence of $self->{branches}
> before tyring $self->{branches}->{$branchcode}. 
> 
> You can either create $self->{branches} in a "sub new" function or just test
> for it first. Something like "$self->{branches} = {} unless
> $self->{branches}". Then You can test for $self->{branches}->{$branchcode}
> because you've initialized $self->{branches} as a hashref.

Yeah, I tested and developed in a separate repo and tried to just copy paste
the code to the one with Koha-master code and missed the variable declaration. 

About GetItemsInfo:
Removed the deep hashes as they were unneccesary. (Framwork is the same for all
items in biblio) Autovivification should do the trick with the undefined
hash-keys but added them more explicitly. It's more obvious what's done that
way.The only warnings I got were the item-values that are NULL in the database
so I check for those.

Branches:
Added a "new" sub to init the cache in a more explicit manner. Autovivification
did it before but is a bit more obscure.

-- 
You are receiving this mail because:
You are watching all bug changes.