From 35fe5b9cbe10b1323183ad8325696077af2eafa3 Mon Sep 17 00:00:00 2001 From: Lebbeous Fogle-Weekley Date: Fri, 13 Apr 2012 17:18:58 -0400 Subject: [PATCH] Acq: Refactor General Search for more smarts and speed This started in response to problems discussed around https://bugs.launchpad.net/evergreen/+bug/967824 , but it really addresses a distinct issue from that bug. Acq General Search relies on lots of joins to make several tables in Acq searchable by attributes of any of the other tables for rows that happen to be related. This is about 1) making better joins and 2) making fewer joins when you don't need them all for a given search. Much thanks to Mike Rylander for help figuring out how to efficiently implement the joins needed for acq.invoice, which is the tricky part. Less importantly, but still significant for some searches, we also need case insensitivity for searching on user-linked fields (General Search always lets you search by any of username/family name/given name/barcode for these fields) in order to take advantage of some indexes for speed. This diagram of key Acq relationships is helpful: https://docs.google.com/drawings/d/15ExkiYvq0skfobbocvPWxwdZkb7aykEZpLGfbP9PL04/view Signed-off-by: Lebbeous Fogle-Weekley Signed-off-by: Ben Shum --- Open-ILS/examples/fm_IDL.xml | 68 ++++++++++---- .../perlmods/lib/OpenILS/Application/Acq/Search.pm | 102 +++++++++++++++------ 2 files changed, 122 insertions(+), 48 deletions(-) diff --git a/Open-ILS/examples/fm_IDL.xml b/Open-ILS/examples/fm_IDL.xml index 375f93fa5d..9d9b30f830 100644 --- a/Open-ILS/examples/fm_IDL.xml +++ b/Open-ILS/examples/fm_IDL.xml @@ -9657,35 +9657,67 @@ SELECT usr, - + - + + + - - - + + + diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/Search.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/Search.pm index 109284ccf1..cbf4c73c5c 100644 --- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/Search.pm +++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/Search.pm @@ -156,12 +156,16 @@ sub get_fm_links_by_hint { sub gen_au_term { my ($value, $n) = @_; + my $lc_value = { + "=" => { transform => "lowercase", value => lc($value) } + }; + +{ "-or" => [ {"+au$n" => {"usrname" => $value}}, - {"+au$n" => {"first_given_name" => $value}}, - {"+au$n" => {"second_given_name" => $value}}, - {"+au$n" => {"family_name" => $value}}, + {"+au$n" => {"first_given_name" => $lc_value}}, + {"+au$n" => {"second_given_name" => $lc_value}}, + {"+au$n" => {"family_name" => $lc_value}}, {"+ac$n" => {"barcode" => $value}} ] }; @@ -262,12 +266,13 @@ sub prepare_terms { } sub add_au_joins { - my ($from) = shift; + my $graft_map = shift; + my $core_hint = shift; my $n = 0; foreach my $join (@_) { my ($hint, $attr, $num) = @$join; - my $start = $from->{"acqus"}{$hint}; + my $start = $graft_map->{$hint}; my $clause = { "class" => "au", "type" => "left", @@ -282,13 +287,64 @@ sub add_au_joins { } } }; - $start->{"join"} ||= {}; - $start->{"join"}->{"au$num"} = $clause; + + if ($hint eq $core_hint) { + $start->{"au$num"} = $clause; + } else { + $start->{"join"} ||= {}; + $start->{"join"}->{"au$num"} = $clause; + } + $n++; } $n; } +sub build_from_clause_and_joins { + my ($query, $core, $and_terms, $or_terms) = @_; + + my %graft_map = (); + + $graft_map{$core} = $query->{from}{$core} = {}; + + my $join_type = keys(%$or_terms) ? "left" : "inner"; + + my @classes = grep { $core ne $_ } (keys(%$and_terms), keys(%$or_terms)); + my %classes_uniq = map { $_ => 1 } @classes; + @classes = keys(%classes_uniq); + + my $acqlia_join = sub { + return {"type" => "left", "field" => "lineitem", "fkey" => "id"}; + }; + + foreach my $class (@classes) { + if ($class eq 'acqlia') { + if ($core eq 'acqinv') { + $graft_map{acqlia} = + $query->{from}{$core}{acqmapinv}{join}{jub}{join}{acqlia} = + $acqlia_join->(); + } elsif ($core eq 'jub') { + $graft_map{acqlia} = + $query->{from}{$core}{acqlia} = + $acqlia_join->(); + } else { + $graft_map{acqlia} = + $query->{from}{$core}{jub}{join}{acqlia} = + $acqlia_join->(); + } + } elsif ($class eq 'acqinv' or $core eq 'acqinv') { + $graft_map{$class} = + $query->{from}{$core}{acqmapinv}{join}{$class} ||= {}; + $graft_map{$class}{type} = $join_type; + } else { + $graft_map{$class} = $query->{from}{$core}{$class} ||= {}; + $graft_map{$class}{type} = $join_type; + } + } + + return \%graft_map; +} + __PACKAGE__->register_method( method => "unified_search", api_name => "open-ils.acq.lineitem.unified_search", @@ -383,36 +439,22 @@ q/order_by clause must be of the long form, like: } my $query = { - "select" => $select_clause, - from => { - acqus => { - jub => {type => "full"}, - acqpo => {type => "full"}, - acqpl => {type => "full"}, - acqinv => {type => "full"} - } - }, - "order_by" => ($options->{"order_by"} || {$hint => {"id" => {}}}), - "offset" => ($options->{"offset"} || 0) + select => $select_clause, + order_by => ($options->{order_by} || {$hint => {id => {}}}), + offset => ($options->{offset} || 0) }; $query->{"limit"} = $options->{"limit"} if $options->{"limit"}; - # XXX for the future? but it doesn't quite work as is. -# # Remove anything in temporary picklists from search results. -# $and_terms ||= {}; -# $and_terms->{"acqpl"} ||= []; -# push @{$and_terms->{"acqpl"}}, {"name" => "", "__not" => 1}; + my $graft_map = build_from_clause_and_joins( + $query, $hint, $and_terms, $or_terms + ); $and_terms = prepare_terms($and_terms, 1); - $or_terms = prepare_terms($or_terms, 0) and do { - $query->{"from"}{"acqus"}{"jub"}{"join"}{"acqlia"} = { - "type" => "left", "field" => "lineitem", "fkey" => "id", - }; - }; + $or_terms = prepare_terms($or_terms, 0); - my $offset = add_au_joins($query->{"from"}, prepare_au_terms($and_terms)); - add_au_joins($query->{"from"}, prepare_au_terms($or_terms, $offset)); + my $offset = add_au_joins($graft_map, $hint, prepare_au_terms($and_terms)); + add_au_joins($graft_map, $hint, prepare_au_terms($or_terms, $offset)); if ($and_terms and $or_terms) { $query->{"where"} = { -- 2.11.0