When QueryParser is activated and OpacSuppression is used, you don't find any results for "All libraries" (pull down next to simple search in OPAC).
Here is the error message that gets logged when I try such a search with both UseQueryParser and OpacSuppression on: opac-search.pl: WARNING: query problem with (@attr 2=102 @or @or @attr 1=1016 @attr 5=1 @attr 4=6 "history" @attr 9=20 @attr 2=102 @attr 5=1 @attr 4=6 "history" @attr 9=34 @attr 2=102 @attr 5=1 @attr 4=6 "history") not Suppress=1 ZOOM error 20003 "can't set prefix query" (addinfo: "(@attr 2=102 @or @or @attr 1=1016 @attr 5=1 @attr 4=6 "history" @attr 9=20 @attr 2=102 @attr 5=1 @attr 4=6 "history" @attr 9=34 @attr 2=102 @attr 5=1 @attr 4=6 "history") not Suppress=1") from diag-set 'ZOOM' at /home/gmc/koha/C4/Search.pm line 378., referer: http://opac.zadi.librarypolice.com/cgi-bin/koha/opac-search.pl?idx=&q=history&branch_group_limit= I'm bumping up the severity to critical as this would be a blocker to making QueryParser turned on by default for new installations.
Poking at this more, the immediate cause of the problem is that tacking on 'not Suppress=1' to the end of a PQF query generated by QP is a syntax error. That can be addressed by munging the input to buildQuery(), but I've run into a roadblock: the PQF QueryParser driver needs to be taught how to use the negation operator.
Hi Galen, this doesn't sound good: >PQF QueryParser driver needs to be taught how to use the negation operator. How bad is it?
Created attachment 26208 [details] [review] Bug 10542: Fix QueryParser with OpacSupression OpacSupressions manipulates the query string after the buildQuery call and so breaks with queryParser enabled. This patch adds checks for queryParser and manipulates the query before passing it to buildQuery if it is enabled, but leaves the post buildQuery manipultation when queryParser is disabled
Many thanks for the hints form Jared on this one. I'm sure this can be more cleanly implimented, but wanted to post up a patch asap for others to comment upon and test.
Created attachment 26251 [details] [review] Attachment to Bug 10542 - QueryParser + OpacSuppression doesn't allow search in 'all libraries' [SIGNED-OFF] Bug 10542: Fix QueryParser with OpacSupression OpacSupressions manipulates the query string after the buildQuery call and so breaks with queryParser enabled. This patch adds checks for queryParser and manipulates the query before passing it to buildQuery if it is enabled, but leaves the post buildQuery manipultation when queryParser is disabled Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Created attachment 26283 [details] [review] Bug 10542: Fix QueryParser with OpacSupression OpacSupressions manipulates the query string after the buildQuery call and so breaks with queryParser enabled. This patch adds checks for queryParser and manipulates the query before passing it to buildQuery if it is enabled, but leaves the post buildQuery manipultation when queryParser is disabled
Created attachment 26311 [details] [review] Bug 10542: Fix QueryParser with OpacSupression OpacSupressions manipulates the query string after the buildQuery call and so breaks with queryParser enabled. This patch adds checks for queryParser and manipulates the query before passing it to buildQuery if it is enabled, but leaves the post buildQuery manipultation when queryParser is disabled Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>
In my tests the search was no longer blocked, but the suppressed records showed up in OPAC. I have to fail this :(
Just an Update: My approach was way to simplistic, and I'm still looking into this, but it may take a little longer than hoped.
Here's the start of the solution: Add the following line of code to the OpacSuppression code block: $query = '@not '.$query.' @attr 1=9011 1'; if (C4::Context->preference('OpacSuppression')) { # OPAC suppression by IP address if (C4::Context->preference('OpacSuppressionByIPRange')) { my $IPAddress = $ENV{'REMOTE_ADDR'}; my $IPRange = C4::Context->preference('OpacSuppressionByIPRange'); if ($IPAddress !~ /^$IPRange/) { $query = "($query) not Suppress=1"; } } else { #$query = "($query) not Suppress=1"; $query = '@not '.$query.' @attr 1=9011 1'; } } Now...we can't comment out [$query = "($query) not Suppress=1";]. That's just for demonstration purposes. Unfortunately, we can't use the QP syspref to detect whether we should use the CCL version or the PQF version, because buildQuery can output both depending on the query that is input into it. So...the hack here is to use some regex to detect "@attr" in the outbound query probably. You can get fancier than that. Even better...changing buildQuery, so that it returns what query language it's using! Then no hacking necessary. I can probably do a patch for this sometime when I'm less pressed for time.
Hmm, now I'm unsure about my answer due to the order of operations... Example from IndexData (http://www.indexdata.com/yaz/doc/tools.html#PQF) @or "dylan" "zimmerman" @and @or dylan zimmerman when @and when @or dylan zimmerman It seems like you always need to have your operators at the beginning...and that it figures out the order on its own. In the 3rd case, not all the operators are at the beginning, but they are before the "pair" of operands on which they're operating. Here's my query that worked: @not @or @or @or @attr 1=1016 @attr 5=1 @attr 4=6 "e" @attr 9=75 @attr 2=102 @attr 5=1 @attr 4=6 "e" @attr 9=20 @attr 2=102 @attr 5=1 @attr 4=6 "e" @attr 9=34 @attr 2=102 @attr 5=1 @attr 4=6 "e" @attr 1=9011 1 Yet...consider the following: prefixQuery("dinosaur"); CCLQuery("dinosaur"); prefixQuery("@and complete dinosaur"); CCLQuery("complete and dinosaur"); prefixQuery("@and complete @or dinosaur pterosaur"); CCLQuery("complete and (dinosaur or pterosaur)"); prefixQuery("@attr 1=7 0253333490"); CCLQuery("isbn=0253333490"); Even these examples don't make sense but it's probably something like... "e and not e or e or e or suppress=1" Maybe... "e and not (e or e or e or suppress=1)" Which...will hide the suppressed records...but it'll also kill those other "e" subqueries...
I'm looking again at this example: prefixQuery("@and complete @or dinosaur pterosaur"); CCLQuery("complete and (dinosaur or pterosaur)"); And considering the PQF grammar: PQF Grammar complex ::= operator query-struct query-struct. http://www.indexdata.com/yaz/doc/tools.html#PQF If it is "reverse polish notation" (ie RPN)... Then the following: @not @or @or @or @attr 1=1016 @attr 5=1 @attr 4=6 "e" @attr 9=75 @attr 2=102 @attr 5=1 @attr 4=6 "e" @attr 9=20 @attr 2=102 @attr 5=1 @attr 4=6 "e" @attr 9=34 @attr 2=102 @attr 5=1 @attr 4=6 "e" @attr 1=9011 1 Becomes: @attr 1=1016 @attr 5=1 @attr 4=6 "e" @not @attr 9=75 @attr 2=102 @attr 5=1 @attr 4=6 "e" @or @attr 9=20 @attr 2=102 @attr 5=1 @attr 4=6 "e" @or @attr 9=34 @attr 2=102 @attr 5=1 @attr 4=6 "e" @or @attr 1=9011 1 OR kw,wrdl,rt=e and not (kw,wrdl,rt=e or (kw,wrdl,rt=e or (kw,wrdl,rt=e or suppress=1))) Well...there is also relevance ranking in there but I got lazy... I'm not sure how to make the query structure into: (kw,wrdl,rt=e or kw,wrdl,rt=e or kw,wrdl,rt=e or kw,wrdl,rt=e) and not (suppress=1) I know you can do grouping with CCL but how that works with PQF...I have no idea.
I see this too in the grammar, but I don't understand how to read grammars well enough to understand: query-struct ::= attr-spec | simple | complex | '@term' term-type query
I wonder if these are supposed to be equivalent... @and when @or dylan zimmerman becomes... when and (dylan or zimmerman) So what does that mean for the following? @and @or dylan zimmerman when Does it become: 1) "dylan" and "zimmerman" or "when" 2) "dylan" and ("zimmerman" or "when") 3) ("dylan" or "zimmerman") and "when" I suppose the only way is to read YAZ source code or to do more tests...
So I have 5 records: "A", "B", "C", "D", and "E". [As of this writing, all of them return 1 record except D which returns all 5...should've picked a different letter. Oh well.] Every one of them has the value "test" in them as well. -- TEST: @and "a" "b" = No records "a" and "b" -- TEST: @and @or "a" "b" "c" = No records [no] ("a" and "b") or "c" ["a" and "b" = 0. "c" = 1.] [?] "a" and ("b" or "c") ["a" and "b" = 0. "a" and "c" = 0.] [?] ("a" or "b") and "c" ["a" and "c" = 0. "b" and "c" = 0.] [no] "a" or ("b" and "c") ["a" = 1. "b" and "c" = 0.] -- TEST: @and @or "a" "b" "test" = 2 records ("A","B") [?] ("a") or ("b" and "test") ["a" = 1. "b" and "test" = 1. Grand total of 2. [?] ("a" or "b") and "test" ["a" and "test" = 1. "b" and "test" = 1. Grand total of 2. [no] "a" and ("b" or "test") ["a" and "b" = 0. "a" and "test" = 5.] [no] ("a" and "b") or "test" ["a" and "b" = 0. "test" = 5.] -- TEST: @and @or "test" "B" "C" = 1 record ("C") [no] ("test" and "B") or "C" ["test and "B" = 1. "C" = 1. Grand total 2 records of "B" and "C"] [no] "test" and ("B" or "C") ["test" and "B" = 1. "test" and "C" = 1. Grand total 2 records of "B" and "C"] [YES] ("test" or "B") and "C" ["test" and "C" = 1. "B" and "C" = 0.] [no] "test" or ("B" and "C") ["test" = 5. "B" and "C" = 0] -- So...it looks like queries are expanded from the inside out... @and @or "test" "B" "C" ("test" or "b") and "C" Let's do some more tests to double-check this... -- TEST: @not "test" "b" = 4 records ("A","C","D",E") "test" and not "b" -- TEST: @not @or "b" "test" "b" = 4 records ("A","C","D","E") ("b" or "test") and not "b" -- TEST: @not @or "b" "c" "b" = 1 record ("C") ("b" or "c") and not "b" -- TEST: @not @or @or "b" "c" "a" "b" = 2 records ("A","C") (("b" or "c") or "a") and not "b" -- TEST: @not @or @or "b" "c" "a" "e" = 3 records ("A","B","C") (("b" or "c") or "a") and not "e" -- TEST: @not @or @and "b" "c" "a" "e" = 1 record ("A") (("b" and "c") or "a") and not "e" ["b" and "c" = 0. "a" = 1". Not "E"..."A" isn't "E" so it goes through. -- TEST: @not @or @and "b" "c" "a" "a" (("b" and "c") or "a") and not "a" ["b" and "c" = 0. "a" = 1". Not "A"..."A" is "A" so it doesn't make it to the result set.] -- TEST: @or @not @or "b" "c" "c" "e" = 2 records ("B","E") (("b" or "c") and not "c") or "e" ["b" = 1. "c" = 1. But not C...so only B. Or "E". Ergo, "B","E" returned) -- TEST: @or @not "test" "c" "e" = 4 records ("A","B","D","E") ("test" and not "C") or "e" -- TEST: @or @not "test" "c" "c" = 4 records ("A","B","C","D","E") ("test" and not "C") or "C" -- TEST: @not "test" @or "c" "e" = 3 records ("A","B","D") ("test") and not ("c" or "e") -- So...it appears that the Zebra parser takes the right-most operator and puts it after the first operand it encounters to its right... So...let's look at my query again... @not @or @or @or @attr 1=1016 @attr 5=1 @attr 4=6 "e" @attr 9=75 @attr 2=102 @attr 5=1 @attr 4=6 "e" @attr 9=20 @attr 2=102 @attr 5=1 @attr 4=6 "e" @attr 9=34 @attr 2=102 @attr 5=1 @attr 4=6 "e" @attr 1=9011 1 This would become (((@attr 1=1016 @attr 5=1 @attr 4=6 "e" @or @attr 9=75 @attr 2=102 @attr 5=1 @attr 4=6 "e") @or @attr 9=20 @attr 2=102 @attr 5=1 @attr 4=6 "e") @or @attr 9=34 @attr 2=102 @attr 5=1 @attr 4=6 "e") @not @attr 1=9011 1 That should be OK... That means that it will perform 5 sub-queries...4 of them will have OR between them...which produces the result set we want... Then the final sub-query returns a result set for records we don't want, and it acts as a filter to make sure that no suppressed records get through. Voila!
If I want to check my Koha query again... -- TEST: @not @or @or @or "a" "b" c" "e" "b" = 3 records("A","C","E") ((("a" or "b") or "c") or "e") and not "b" -- Ok...now that we've got this basic concept down...let's try something more advanced. Let's consider the parsing logic again... prefixQuery("@and complete @or dinosaur pterosaur"); CCLQuery("complete and (dinosaur or pterosaur)"); (In hindsight, the above example quite clearly illustrates how operators are applied to operands. The tests helped clarify that though...at least in my mind and hopefully for anyone else reading this who didn't understand before.) So let's look at the grammar again (http://www.indexdata.com/yaz/doc/tools.html#PQF): "simple ::= result-set | term." So a simple query may just be a single term like "A". "complex ::= operator query-struct query-struct." A complex query can have an operator and two query structures...but a query structure is not the same thing as just a term. "query-struct ::= attr-spec | simple | complex | '@term' term-type query" A query structure can itself be a complex query! So we should be able to join multiple complex queries together to form a new complex query. -- TEST: @or @or "a" "b" @or "c" "e" = 4 records ("A","B","C","E") ("a" or "b") or ("c" or "e") -- TEST: @not "test" @or "c" "e" = 3 records ("A","B","D") ("test") and not ("c" or "e") -- So let's add some more complex data... I'm going to add "bull" to "B". -- TEST: @not @or "a" @or @and "b" "bull" "c" "a" = 2 records ("B","C") ("a" or ("b" and "bull") or "c") and not "a" -- TEST: @or "a" @or @and "b" "bull" "c" = 3 records ("A","B","C") ("a" or ("b" and "bull") or "c") -- So something like the following is probably valid in CCL... (kw,wrdl,rt=e or kw,wrdl,rt=e or kw,wrdl,rt=e or kw,wrdl,rt=e) But I bet when it's parsed into RPN it is broken up into pairs like so: (((kw,wrdl,rt=e or kw,wrdl,rt=e) or kw,wrdl,rt=e) or kw,wrdl,rt=e) Because there has to be an order of operations... For instance ("b" and "bull" or "c") would probably be parsed as (("b" and "bull") or "c") rather than ("b" and ("bull" or "c")). -- TEST: (After using "querytype ccl2rpn" and "set_cclfile /koha/etc/zebradb/ccl.properties" in yaz-client) ("b" and "bull" or "c") = 2 records ("B","C") (("b" and "bull") or "c") -- TEST: ("b" and ("bull" or "c")) = 1 record ("B") ------------ So...I'm sure that was more than anyone ever wanted to know about CCL and PQF syntax. But...it proves that my solution will work. Ideally, we should be adding OpacSuppression when the query is being built, rather than after the fact. Ah, we do pass $query_type back from buildQuery. While it's usually null, if it is a PQF query, it will say "pqf". That means it won't be too hard to change our current code to work with QP. At the moment, I'm inclined to agree with Galen about QueryParser not being able to handle negation...although I'll have to look some more. That said, we could add OpacSuppression into the QP query building without requiring it to parse it from a query we put in, so it might not matter too much.
Ah, I believe that Galen was incorrect. QueryParser indeed does appear to handle negation. QueryParser: [Negation via query_plan.pm] (test) && !(suppress:1) OR (test) && -(suppress:1) OR [Negation via node.pm] (test) && (suppress:!"1") OR (test) && (suppress:-"1") (N.B. When negating at the "node" level, you have to wrap the 1 in quotes to treat it as a phrase or else the ! or - appear to be parsed as part of the operand.) CCL: (test) not (suppress:1) If you think about it, "not" really is "and not", so CCL's "not" is equivalent to "&& !" in QP. ------ Ahh...so I've finally read through Martin's patch. tl;dr Martin, if you change the following: $query = "$query -suppress:1"; to: $query = "($query) && -(suppress:1)"; It should work. [Note: If we wanted to add other fields to disallow, we could throw them inside that disallowed group when using QP as well...]
Created attachment 29909 [details] [review] Bug 10542: Fix QueryParser with OpacSupression OpacSupressions manipulates the query string after the buildQuery call and so breaks with queryParser enabled. This patch adds checks for queryParser and manipulates the query before passing it to buildQuery if it is enabled, but leaves the post buildQuery manipultation when queryParser is disabled
dcook, thanks for all your efforts on this one. I've made the suggested changes, I think, and am currently proving the process to myself too, but thought it worth posting up this trivial amendment for others to start testing sooner rather than later. I'm now trying to work out all the ways in which this should be tested to repvent a regression... I'm thinking. Testing with and without QP turned on, with and without OpacSupressions turned on, and testing from both the front page search and the advanced search pages. Combinations of all those should close the door on a regression right? Is there anywhere I've missed in terms of sending different queries down the pipe?
Bear with me, I'm adapting the patch to be a bit cleaner, and hopefully catch a few more cases as per dcooks other suggestions.
Created attachment 29910 [details] [review] Bug 10542: Fix QueryParser with OpacSupression OpacSupressions manipulates the query string after the buildQuery call and so breaks with queryParser enabled. This patch adds checks for queryParser and manipulates the query before passing it to buildQuery if it is enabled, but leaves the post buildQuery manipultation when queryParser is disabled
Next attempt ready.. go forth and destroy.. I mean test
Created attachment 29942 [details] [review] Bug 10542: Fix QueryParser with OpacSupression OpacSupressions manipulates the query string after the buildQuery call and so breaks with queryParser enabled. This patch adds checks for queryParser and manipulates the query before passing it to buildQuery if it is enabled, but leaves the post buildQuery manipultation when queryParser is disabled Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
I realized last night after getting off IRC that I forgot to mention that we would need to use PQF syntax rather than QP syntax, if using the $query_type to know what type of query we're working with. The following syntax would have to be used "before" buildQuery/parseQuery, as this is QueryParser syntax which is parsed and then transformed into PQF. $query = "($query) && -(suppress:1)" Since the latest patch is adding the OpacSuppression after the query has already been returned from buildQuery as PQF, we need to use the following PQF syntax: $query = '@not '."$query".' @attr 1=9011 1'; -- Otherwise we generate an invalid query that will return 0 records when using QueryParser: ZOOM error 20003 "can't set prefix query" (addinfo: "(@or @or @or @attr 1=1016 @attr 5=1 @attr 4=6 "test" @attr 9=75 @attr 2=102 @attr 5=1 @attr 4=6 "test" @attr 9=20 @attr 2=102 @attr 5=1 @attr 4=6 "test" @attr 9=34 @attr 2=102 @attr 5=1 @attr 4=6 "test") && -(suppress:1)") from diag-set 'ZOOM' -- We also can't use parentheses as those aren't used in PQF either. ZOOM error 20003 "can't set prefix query" (addinfo: "@not (@or @or @or @attr 1=1016 @attr 5=1 @attr 4=6 "test" @attr 9=75 @attr 2=102 @attr 5=1 @attr 4=6 "test" @attr 9=20 @attr 2=102 @attr 5=1 @attr 4=6 "test" @attr 9=34 @attr 2=102 @attr 5=1 @attr 4=6 "test") @attr 1=9011 1") from diag-set 'ZOOM' -- The following query will work as expected though: @not @or @or @or @attr 1=1016 @attr 5=1 @attr 4=6 "test" @attr 9=75 @attr 2=102 @attr 5=1 @attr 4=6 "test" @attr 9=20 @attr 2=102 @attr 5=1 @attr 4=6 "test" @attr 9=34 @attr 2=102 @attr 5=1 @attr 4=6 "test" @attr 1=9011 1 I had to look over those past comments of mine to make sure but if this were CCL it should look like this: (((kw,wrdl,rt=test OR kw,wrdl,rt=test) OR kw,wrdl,rt=test) OR kw,wrdl,rt=test) NOT suppress=1
Created attachment 29944 [details] [review] Bug 10542 [Follow-up] - QueryParser + OpacSuppression doesn't allow search in 'all libraries' Since we're using the $query_type variable to detect if this pre-built query is PQF, we need to use PQF syntax (rather than QueryParser syntax) when adding to the query. I've made a lot of notes of somewhat incoherent notes on Bugzilla talking about PQF, CCL, and QP syntaxes, but I'm hoping to refine these notes on a wiki page for future reference. _TEST PLAN_ 1) Set 'Suppress in Opac' (ie 942$n) to 1 for one record 2) Re-index Zebra 3) Set 'OpacSuppression' to 'Hide' 4) Set 'UseQueryParser' to 'Do not try' 5) In the staff client, do a search that will return that suppressed record as well as a few records that are NOT suppressed 6) Note that you can return that suppressed record in the staff client 7) Do the same search in the OPAC 8) Note that the suppressed record doesn't appear 9) Set 'UseQueryParser' to 'Try' && re-run the OPAC search 10) Note that no results appear (the logs will probably mention a ZOOM error) 11) Apply patch 12) Re-run the OPAC search 13) Note that the suppressed record doesn't appear, and that the not suppressed records are showing (it's important that you are getting some results...as ZOOM errors are silent in the UI). 14) Set 'UseQueryParser' to 'Do not try' 15) Re-run the search 16) Note that the suppressed record doesn't appear, and that the not suppressed records are showing
Created attachment 30478 [details] [review] [SIGNED-OFF] Bug 10542: Fix QueryParser with OpacSupression OpacSupressions manipulates the query string after the buildQuery call and so breaks with queryParser enabled. This patch adds checks for queryParser and manipulates the query before passing it to buildQuery if it is enabled, but leaves the post buildQuery manipultation when queryParser is disabled Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Adding a sing after test
Created attachment 30479 [details] [review] [SIGNED-OFF] Bug 10542: QueryParser + OpacSuppression doesn't allow search in 'all libraries' Since we're using the $query_type variable to detect if this pre-built query is PQF, we need to use PQF syntax (rather than QueryParser syntax) when adding to the query. I've made a lot of notes of somewhat incoherent notes on Bugzilla talking about PQF, CCL, and QP syntaxes, but I'm hoping to refine these notes on a wiki page for future reference. _TEST PLAN_ 1) Set 'Suppress in Opac' (ie 942$n) to 1 for one record 2) Re-index Zebra 3) Set 'OpacSuppression' to 'Hide' 4) Set 'UseQueryParser' to 'Do not try' 5) In the staff client, do a search that will return that suppressed record as well as a few records that are NOT suppressed 6) Note that you can return that suppressed record in the staff client 7) Do the same search in the OPAC 8) Note that the suppressed record doesn't appear 9) Set 'UseQueryParser' to 'Try' && re-run the OPAC search 10) Note that no results appear (the logs will probably mention a ZOOM error) 11) Apply patch 12) Re-run the OPAC search 13) Note that the suppressed record doesn't appear, and that the not suppressed records are showing (it's important that you are getting some results...as ZOOM errors are silent in the UI). 14) Set 'UseQueryParser' to 'Do not try' 15) Re-run the search 16) Note that the suppressed record doesn't appear, and that the not suppressed records are showing Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Work as described following test plan. No koha-qa errors.
I am sorry, but it looks like we are not quite there yet. I am using "der" (similar to "the") as a search term. Without OpacSuppression this gives 34 results. I hid one record, without QueryParser the search gives 33 results now. OK With QueryParser I see some weird behaviour - sometimes the results are 21, then 33, then a specific recod is pulled up or no results are found... I am puzzled. The URL is: http://localhost/cgi-bin/koha/opac-search.pl?idx=&q=der Reloaded the page using F5. I reindexed my database after hiding the record.
(In reply to Katrin Fischer from comment #29) > I am sorry, but it looks like we are not quite there yet. I am using "der" > (similar to "the") as a search term. Without OpacSuppression this gives 34 > results. > > I hid one record, without QueryParser the search gives 33 results now. OK > > With QueryParser I see some weird behaviour - sometimes the results are 21, > then 33, then a specific recod is pulled up or no results are found... I am > puzzled. > > The URL is: > http://localhost/cgi-bin/koha/opac-search.pl?idx=&q=der > > Reloaded the page using F5. > > I reindexed my database after hiding the record. You always manage to find the strangest behaviour, Katrin :p. When you say sometimes the results are 21, then 33, then a specific record is pulled up or no results are found...is that all with the same query? Could you explain a bit more about the process you take to get to those results?
It's my secret super power... It's all with the same query - just hitting F5 or setting the cursor back into the search field and submitting. I have NO idea how this is possible sadly :(
Might this have something to do with the QP bug regarding hash order? Wonder if caits perl version is one of those that forcefully randomizes hash order and thus 'breaks' QP a certain amount? I'm not fully up on how it affects stuff, but it feels like a random enough symptom that it may match?
At least it would be some explanation - because that was really crazy behaviour. I am using the latest Ubuntu - can check the exact version when I am back home.
Latest Ubuntu would give you the affected perl version, so it's certailny feeling like my hunch may be correct.
Intriguing! I hadn't heard about this problem. Is there a bug number for it? All the more reason to get Katrin to print out the queries that are being generated each time. Katrin, if you do a warn "Query = $query" just after this line (http://repo.or.cz/w/koha.git/blob/HEAD:/opac/opac-search.pl#l521), we should be able to see the different queries that are being produced. Of course, if it is a problem with randomizing hash order...that problem would exist before this patch is applied as well, so that's worth checking too.
Tomas just opened a bug for exactly what I was talking about! Must have been reading my mind: bug 12738
OK, I've now tested with and without this patch, on Debian and Ubuntu (i.e. the differing perl versions) I can verify that it works perfectly as expected on Deb (older perl) with the patch and breaks in the expected way before the patch. On Ubuntu, QP is broken before and after the patch. If we mask the bug this patch addresses (by disabling OpacSupression) then pre patch and post patch breakage matches. I think this should get passed qa really now. Other QP issues need addressing separately.
I haven't read the patch, but please test this one on top of bug 12738 's patch if you're dealing with QP's PQF output.
Created attachment 30660 [details] [review] [PASSED QA] Bug 10542: Fix QueryParser with OpacSupression OpacSupressions manipulates the query string after the buildQuery call and so breaks with queryParser enabled. This patch adds checks for queryParser and manipulates the query before passing it to buildQuery if it is enabled, but leaves the post buildQuery manipultation when queryParser is disabled Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Adding a sing after test Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
(In reply to Tomás Cohen Arazi from comment #38) > I haven't read the patch, but please test this one on top of bug 12738 's > patch if you're dealing with QP's PQF output. I did - worked perfectly :)
Created attachment 30661 [details] [review] [PASSED QA] Bug 10542: QueryParser + OpacSuppression doesn't allow search in 'all libraries' Since we're using the $query_type variable to detect if this pre-built query is PQF, we need to use PQF syntax (rather than QueryParser syntax) when adding to the query. I've made a lot of notes of somewhat incoherent notes on Bugzilla talking about PQF, CCL, and QP syntaxes, but I'm hoping to refine these notes on a wiki page for future reference. _TEST PLAN_ 1) Set 'Suppress in Opac' (ie 942$n) to 1 for one record 2) Re-index Zebra 3) Set 'OpacSuppression' to 'Hide' 4) Set 'UseQueryParser' to 'Do not try' 5) In the staff client, do a search that will return that suppressed record as well as a few records that are NOT suppressed 6) Note that you can return that suppressed record in the staff client 7) Do the same search in the OPAC 8) Note that the suppressed record doesn't appear 9) Set 'UseQueryParser' to 'Try' && re-run the OPAC search 10) Note that no results appear (the logs will probably mention a ZOOM error) 11) Apply patch 12) Re-run the OPAC search 13) Note that the suppressed record doesn't appear, and that the not suppressed records are showing (it's important that you are getting some results...as ZOOM errors are silent in the UI). 14) Set 'UseQueryParser' to 'Do not try' 15) Re-run the search 16) Note that the suppressed record doesn't appear, and that the not suppressed records are showing Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> Work as described following test plan. No koha-qa errors. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Works as described - hidden records are hidden. Passes tests and QA script.
Patches pushed to master. Thanks Martin and David!