Acq: Refactor General Search for more smarts and speed
authorLebbeous Fogle-Weekley <lebbeous@esilibrary.com>
Fri, 13 Apr 2012 21:18:58 +0000 (17:18 -0400)
committerLebbeous Fogle-Weekley <lebbeous@esilibrary.com>
Tue, 17 Apr 2012 13:19:30 +0000 (09:19 -0400)
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 <lebbeous@esilibrary.com>
Signed-off-by: Ben Shum <bshum@biblio.org>

Open-ILS/examples/fm_IDL.xml
Open-ILS/src/perlmods/lib/OpenILS/Application/Acq/Search.pm

index 375f93f..9d9b30f 100644 (file)
@@ -9657,35 +9657,67 @@ SELECT  usr,
                        </actions>
                </permacrud>
        </class>
-       <class id="acqus" controller="open-ils.cstore" oils_obj:fieldmapper="acq::unified_search" reporter:label="Acq Unified Search View" oils_persist:readonly="true">
+       <class id="acqmapinv" controller="open-ils.cstore" oils_obj:fieldmapper="acq::map_to_invoice" reporter:label="Acq Map to Invoice View" oils_persist:readonly="true">
                <oils_persist:source_definition><![CDATA[
                SELECT
+                       poi.purchase_order AS purchase_order,
+                       ii.invoice AS invoice,
+                       NULL AS lineitem,
+                       poi.id AS po_item,
+                       NULL AS picklist
+               FROM
+                       acq.po_item poi
+                       JOIN acq.invoice_item ii ON (ii.po_item = poi.id)
+               UNION SELECT
+                       jub.purchase_order AS purchase_order,
+                       ie.invoice AS invoice,
+                       jub.id AS lineitem,
+                       NULL AS po_item,
+                       jub.picklist AS picklist
+               FROM
+                       acq.lineitem jub
+                       JOIN acq.invoice_entry ie ON (ie.lineitem = jub.id)
+               UNION SELECT
+                       ii.purchase_order AS purchase_order,
+                       ii.invoice AS invoice,
+                       NULL AS lineitem,
+                       NULL AS po_item,
+                       NULL AS picklist
+               FROM
+                       acq.invoice_item ii
+               WHERE ii.po_item IS NULL
+               UNION SELECT
+                       ie.purchase_order AS purchase_order,
+                       ie.invoice AS invoice,
+                       NULL AS lineitem,
+                       NULL AS po_item,
+                       NULL AS picklist
+               FROM
+                       acq.invoice_entry ie
+               WHERE ie.lineitem IS NULL
+               UNION SELECT
+                       NULL AS purchase_order,
+                       NULL AS invoice,
                        jub.id AS lineitem,
-                       po.id AS purchase_order,
-                       pl.id AS picklist,
-                       inv.id AS invoice
-               FROM acq.purchase_order po
-               FULL JOIN acq.lineitem jub ON (jub.purchase_order = po.id)
-               FULL JOIN acq.picklist pl ON (pl.id = jub.picklist)
-               LEFT JOIN acq.po_item poi ON (poi.purchase_order = po.id)
-               LEFT JOIN acq.invoice_item ii
-                       ON (ii.po_item = poi.id OR ii.purchase_order = po.id)
-               LEFT JOIN acq.invoice_entry ie
-                       ON (ie.lineitem = jub.id OR ie.purchase_order = po.id)
-               LEFT JOIN acq.invoice inv
-                       ON (ie.invoice = inv.id OR ii.invoice = inv.id)
+                       NULL AS po_item,
+                       jub.picklist AS picklist
+               FROM
+                       acq.lineitem jub
+               WHERE jub.purchase_order IS NULL
                ]]></oils_persist:source_definition>
                <fields>
-                       <field reporter:label="Lineitem ID" name="lineitem" reporter:datatype="link"/>
                        <field reporter:label="Purchase Order ID" name="purchase_order" reporter:datatype="link"/>
+                       <field reporter:label="Lineitem ID" name="lineitem" reporter:datatype="link"/>
+                       <field reporter:label="Invoice ID" name="invoice" reporter:datatype="link"/>
+                       <field reporter:label="Purchase Order Item ID" name="po_item" reporter:datatype="link"/>
                        <field reporter:label="Picklist ID" name="picklist" reporter:datatype="link"/>
-                       <field reporter:label="Invoice" name="invoice" reporter:datatype="link"/>
                </fields>
                <links>
-                       <link field="lineitem" reltype="has_a" key="id" map="" class="jub" />
                        <link field="purchase_order" reltype="has_a" key="id" map="" class="acqpo" />
-                       <link field="picklist" reltype="has_a" key="id" map="" class="acqpl" />
+                       <link field="lineitem" reltype="has_a" key="id" map="" class="jub" />
                        <link field="invoice" reltype="has_a" key="id" map="" class="acqinv" />
+                       <link field="po_item" reltype="has_a" key="id" map="" class="acqpoi" />
+                       <link field="picklist" reltype="has_a" key="id" map="" class="acqpl" />
                </links>
        </class>
        <class id="cbc" controller="open-ils.cstore open-ils.pcrud" oils_obj:fieldmapper="config::barcode_completion" oils_persist:tablename="config.barcode_completion" reporter:label="Barcode Completions">
index 109284c..cbf4c73 100644 (file)
@@ -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"} = {