From d72eaa76b2498f070cbb414b376b7566cd8c2d40 Mon Sep 17 00:00:00 2001
From: Jared Camins-Esakov <jcamins@cpbibliography.com>
Date: Tue, 21 Aug 2012 13:53:49 -0400
Subject: [PATCH] Give implicit operators higher precedence in QP

When the implicit ANDs added by QueryParser between terms are combined
with explicit ORs without any explicit grouping, the results can be
unexpected and undesirable. For example, the following query:

  harry potter and the chamber of secrets || sorcerer's stone

Is translated into a query with two branches:
1. harry potter and the chamber of secrets stone
2. sorcerer's stone

This is of course nothing like what the user was expecting, which were
the following two branches:
1. harry potter and the chamber of secrets
2. sorcerer's stone

(Note: of course the user probably wanted to search for "harry potter
and the chamber of secrets" or "harry potter and the sorcerer's stone"
but we have to draw the line on implicit grouping somewhere, and where
it requires reading minds seems like a good place)

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
---
 .../lib/OpenILS/Application/Storage/QueryParser.pm | 37 +++++++++++++++++++---
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/QueryParser.pm b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/QueryParser.pm
index 3afb9d7da9..630925bce0 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/QueryParser.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/Application/Storage/QueryParser.pm
@@ -3,6 +3,7 @@ use warnings;
 
 package QueryParser;
 use OpenSRF::Utils::JSON;
+# Note that the first key must match the name of the package.
 our %parser_config = (
     QueryParser => {
         filters => [],
@@ -642,20 +643,48 @@ sub decompose {
             $last_type = '';
         } elsif (/$and_re/) { # ANDed expression
             $_ = $';
-            next if ($last_type eq 'AND');
-            next if ($last_type eq 'OR');
+            next unless (defined($struct->query_nodes) and scalar(@{$struct->query_nodes}));
             warn "Encountered AND\n" if $self->debug;
 
+            my $lhs = $self->new_plan(level =>
+                    $struct->plan_level + 1, query => $struct->query_nodes);
+            $struct->{query} = [];
+            my ($rhs, $rest) = $self->decompose( $', $current_class, $recursing + 1 );
+            $struct->add_node($lhs);
             $struct->joiner( '&' );
+            $struct->add_node($rhs);
+            foreach (@{$rhs->modifiers}) {
+                $struct->add_modifier($_)
+            }
+            $rhs->{modifiers} = [];
+            foreach (@{$rhs->filters}) {
+                $struct->add_filter($_);
+            }
+            $rhs->{filters} = [];
+            $_ = $rest;
 
             $last_type = 'AND';
         } elsif (/$or_re/) { # ORed expression
             $_ = $';
-            next if ($last_type eq 'AND');
-            next if ($last_type eq 'OR');
+            next unless (defined($struct->query_nodes) and scalar(@{$struct->query_nodes}));
             warn "Encountered OR\n" if $self->debug;
 
+            my $lhs = $self->new_plan(level =>
+                    $struct->plan_level + 1, query => $struct->query_nodes);
+            $struct->{query} = [];
+            my ($rhs, $rest) = $self->decompose( $', $current_class, $recursing + 1 );
+            $struct->add_node($lhs);
             $struct->joiner( '|' );
+            $struct->add_node($rhs);
+            foreach (@{$rhs->modifiers}) {
+                $struct->add_modifier($_)
+            }
+            $rhs->{modifiers} = [];
+            foreach (@{$rhs->filters}) {
+                $struct->add_filter($_);
+            }
+            $rhs->{filters} = [];
+            $_ = $rest;
 
             $last_type = 'OR';
         } elsif ($self->facet_class_count && /$facet_re/) { # changing current class
-- 
2.11.0