From 57bc667d07c7decd314c15e4722c53825c8e057c Mon Sep 17 00:00:00 2001 From: arensb Date: Sat, 12 Oct 2002 05:43:03 +0000 Subject: [PATCH] Got rid of the dependency on Set::Scalar. Yay! One fewer package that the user has to install! Redid the way SQL queries are built up, to make it more readable and maintainable. Removed a couple of unused variables. Got rid of some un-Perl-like bogosity. --- C4/Search.pm | 220 +++++++++++++++++++++++++++------------------------ 1 file changed, 118 insertions(+), 102 deletions(-) diff --git a/C4/Search.pm b/C4/Search.pm index de2d605b79..6870f035d7 100755 --- a/C4/Search.pm +++ b/C4/Search.pm @@ -1,5 +1,4 @@ -package C4::Search; #assumes C4/Search - +package C4::Search; # Copyright 2000-2002 Katipo Communications # @@ -25,7 +24,6 @@ use C4::Context; use C4::Reserves2; # FIXME - C4::Search uses C4::Reserves2, which uses C4::Search. # So Perl complains that all of the functions here get redefined. -use Set::Scalar; use vars qw($VERSION @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS); @@ -299,139 +297,159 @@ C<$num> to 5 will return results 100 through 104 inclusive. =cut #' - -# FIXME - Everything that's being done here with Set::Scalar can be -# done with hashes: to add a result to the set, just use: -# $results{$foo} = 1; -# To get the list of results, just use -# @biblionumbers = sort keys %results; -# This would remove the dependency on Set::Scalar, which means one -# fewer package that people have to install. - sub KeywordSearch { my ($env,$type,$search,$num,$offset)=@_; my $dbh = C4::Context->dbh; $search->{'keyword'}=~ s/ +$//; $search->{'keyword'}=~ s/'/\\'/; my @key=split(' ',$search->{'keyword'}); + # FIXME - Naive users might enter comma-separated + # words, e.g., "training, animal". Ought to cope with + # this. my $count=@key; my $i=1; - my @results; + my %biblionumbers; # Set of biblionumbers returned by the + # various searches. + + # FIXME - Ought to filter the stopwords out of the list of keywords. + # @key = map { !defined($stopwords{$_}) } @key; + + # FIXME - The way this code is currently set up, it looks for all of + # the keywords first in (title, notes, seriestitle), then in the + # subtitle, then in the subject. Thus, if you look for keywords + # "science fiction", this search won't find a book with + # title = "How to write fiction" + # subtitle = "A science-based approach" + # Is this the desired effect? If not, then the first SQL query + # should look in the biblio, subtitle, and subject tables all at + # once. The way the first query is built can accomodate this easily. # Look for keywords in table 'biblio'. - # FIXME - The next 15 lines can be rewritten in somewhat Lisp-ish - # fashion as follows: - # - # my $query = "select biblionumber from biblio where (" . - # join(") or (", - # map { - # my $field = $_; - # join (" and ", - # map { - # "($field like '$_\%' or $field like '\% $_\%')" - # } - # @key) - # } - # qw( title biblio.notes seriestitle )) . - # ")"; - # FIXME - Also, - # field like 'string%' or field like '% string%' - # can be rewritten (in MySQL, at least) as - # field regexp '(^| )string'; - # However, this isn't portable. Though PostgreSQL allows you to use "~" - # instead of "regexp". - my $query="Select biblionumber from biblio - where ((title like '$key[0]%' or title like '% $key[0]%')"; - while ($i < $count){ - $query=$query." and (title like '$key[$i]%' or title like '% $key[$i]%')"; - $i++; - } - $query.= ") or ((biblio.notes like '$key[0]%' or biblio.notes like '% $key[0]%')"; - for ($i=1;$i<$count;$i++){ - $query.=" and (biblio.notes like '$key[$i]%' or biblio.notes like '% $key[$i]%')"; - } - $query.= ") or ((seriestitle like '$key[0]%' or seriestitle like '% $key[0]%')"; - for ($i=1;$i<$count;$i++){ - $query.=" and (seriestitle like '$key[$i]%' or seriestitle like '% $key[$i]%')"; + # Build an SQL query that finds each of the keywords in any of the + # title, biblio.notes, or seriestitle. To do this, we'll build up an + # array of clauses, one for each keyword. + my $query; # The SQL query + my @clauses = (); # The search clauses + + $query = <bind_columns() ? Documented as the most + # efficient way to fetch data. my $sth=$dbh->prepare($query); $sth->execute; - $i=0; - while (my @res=$sth->fetchrow_array){ - $results[$i]=$res[0]; - $i++; + while (my @res = $sth->fetchrow_array) { + for (@res) + { + $biblionumbers{$_} = 1; # Add these results to the set + } } $sth->finish; - my $set1=Set::Scalar->new(@results); # Now look for keywords in the 'bibliosubtitle' table. - $query="Select biblionumber from bibliosubtitle where - ((subtitle like '$key[0]%' or subtitle like '% $key[0]%')"; - for ($i=1;$i<$count;$i++){ - $query.= " and (subtitle like '$key[$i]%' or subtitle like '% $key[$i]%')"; + + # Again, we build a list of clauses from the keywords. + @clauses = (); + $query = "SELECT biblionumber FROM bibliosubtitle WHERE "; + foreach my $keyword (@key) + { + push @clauses, + "subtitle LIKE '\Q$keyword\E%' OR subtitle like '% \Q$keyword\E%'"; } - $query.=" )"; -# print $query; + $query .= "(" . join(") AND (", @clauses) . ")"; + $sth=$dbh->prepare($query); $sth->execute; - $i=0; - while (my @res=$sth->fetchrow_array){ - $results[$i]=$res[0]; - $i++; + while (my @res = $sth->fetchrow_array) { + for (@res) + { + $biblionumbers{$_} = 1; # Add these results to the set + } } $sth->finish; - my $set2=Set::Scalar->new(@results); - if ($i > 0){ - $set1=$set1+$set2; - } # Look for the keywords in the notes for individual items # ('biblioitems.notes') - $query ="Select biblionumber from biblioitems where - ((biblioitems.notes like '$key[0]%' or biblioitems.notes like '% $key[0]%')"; - for ($i=1;$i<$count;$i++){ - $query.=" and (biblioitems.notes like '$key[$i]%' or biblioitems.notes like '% $key[$i]%')"; + + # Again, we build a list of clauses from the keywords. + @clauses = (); + $query = "SELECT biblionumber FROM biblioitems WHERE "; + foreach my $keyword (@key) + { + push @clauses, + "notes LIKE '\Q$keyword\E%' OR notes like '% \Q$keyword\E%'"; } - $query.=" )"; -# print $query; + $query .= "(" . join(") AND (", @clauses) . ")"; + $sth=$dbh->prepare($query); $sth->execute; - $i=0; - while (my @res=$sth->fetchrow_array){ - $results[$i]=$res[0]; - $i++; + while (my @res = $sth->fetchrow_array) { + for (@res) + { + $biblionumbers{$_} = 1; # Add these results to the set + } } $sth->finish; - my $set3=Set::Scalar->new(@results); - if ($i > 0){ - $set1=$set1+$set3; - } # Look for keywords in the 'bibliosubject' table. + + # FIXME - The other queries look for words in the desired field that + # begin with the individual keywords the user entered. This one + # searches for the literal string the user entered. Is this the + # desired effect? + # Note in particular that spaces are retained: if the user typed + # science fiction + # (with two spaces), this won't find the subject "science fiction" + # (one space). Likewise, a search for "%" will return absolutely + # everything. + # If this isn't the desired effect, see the previous searches for + # how to do it. + $sth=$dbh->prepare("Select biblionumber from bibliosubject where subject like '%$search->{'keyword'}%' group by biblionumber"); $sth->execute; - $i=0; - while (my @res=$sth->fetchrow_array){ - $results[$i]=$res[0]; - $i++; + + while (my @res = $sth->fetchrow_array) { + for (@res) + { + $biblionumbers{$_} = 1; # Add these results to the set + } } $sth->finish; - my $set4=Set::Scalar->new(@results); - if ($i > 0){ - $set1=$set1+$set4; - } + my $i2=0; my $i3=0; my $i4=0; my @res2; - my @res = $set1->members; + my @res = keys %biblionumbers; $count=@res; -# print $set1; + $i=0; # print "count $count"; if ($search->{'class'} ne ''){ @@ -493,21 +511,19 @@ sub KeywordSearch { } else { # $search->{'class'} was not specified + + # FIXME - This is bogus: it makes a separate query for each + # biblioitem, and returns results in apparently random order. It'd + # be much better to combine all of the previous queries into one big + # one (building it up a little at a time, of course), and have that + # big query select all of the desired fields, instead of just + # 'biblionumber'. + while ($i2 < $num && $i2 < $count){ my $query="select * from biblio,biblioitems where biblio.biblionumber='$res[$i2+$offset]' and biblio.biblionumber=biblioitems.biblionumber "; - if ($search->{'class'} ne ''){ # FIXME - Ignored: we already know - # that $search->{'class'} eq ''; - my @temp=split(/\|/,$search->{'class'}); - my $count=@temp; - $query.= "and ( itemtype='$temp[0]'"; - for (my $i=1;$i<$count;$i++){ - $query.=" or itemtype='$temp[$i]'"; - } - $query.=")"; - } if ($search->{'dewey'} ne ''){ $query.= "and (dewey like '$search->{'dewey'}%') "; } -- 2.39.5