http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 --- Comment #2 from Galen Charlton <gmcharlt@gmail.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 mathieu saby <mathieu.saby@univ-rennes2.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mathieu.saby@univ-rennes2.f | |r -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 26208 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=26208&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 Jesse Maseto <jesse@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|gmcharlt@gmail.com |martin.renvoize@ptfs-europe | |.com -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #26251|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #9 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- In my tests the search was no longer blocked, but the suppressed records showed up in OPAC. I have to fail this :( -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #11 from David Cook <dcook@prosentient.com.au> --- 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. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 --- Comment #13 from David Cook <dcook@prosentient.com.au> --- 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. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 --- Comment #15 from David Cook <dcook@prosentient.com.au> --- 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... -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 --- Comment #17 from David Cook <dcook@prosentient.com.au> --- 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. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 --- Comment #20 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- 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? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #29909|0 |1 is obsolete| | --- Comment #22 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 29910 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29910&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #25 from David Cook <dcook@prosentient.com.au> --- 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 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 --- Comment #26 from David Cook <dcook@prosentient.com.au> --- Created attachment 29944 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=29944&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #29944|0 |1 is obsolete| | --- Comment #28 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Created attachment 30479 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=30479&action=edit [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. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #29 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 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 are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 --- Comment #31 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 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 :( -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 --- Comment #33 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- 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. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 --- Comment #35 from David Cook <dcook@prosentient.com.au> --- 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. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |http://bugs.koha-community. | |org/bugzilla3/show_bug.cgi? | |id=12738 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off CC| |tomascohen@gmail.com --- Comment #38 from Tomás Cohen Arazi <tomascohen@gmail.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 --- Comment #40 from Katrin Fischer <katrin.fischer@bsz-bw.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 :) -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #42 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Patches pushed to master. Thanks Martin and David! -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10542 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr -- You are receiving this mail because: You are watching all bug changes.