[Xapian-devel] Re: [Xapian-discuss] Xapian 1.0.1 released

[Daniel Ménard]

Olly Betts a écrit :
> On Mon, Jun 11, 2007 at 08:11:11PM +0200, Daniel M?nard wrote:
>   
>> 1. In xapian.php (php5), class XapianQuery, constants OP_AND and 
>> OP_ELITE_SET are defined as integers (0 and 10)
>>     
> So while this looks odd, it's harmless really (...)
>   
that's ok for me,
>> 2. On windows, from the php bindings, I can't use SimpleStopper (apache 
>> hangs).
> Do you have a complete (but ideally not too large) script which
> demonstrates this?
>   
Trying to reproduce the bug, I think I found where it comes from...
Here is a simple script to reproduce the problem (I hope it's OK to post 
here, if not, please tell me about other ways):

---[start]---
require_once dirname(__FILE__) . '/lib/xapian/xapian.php'; // the php5 
OO wrapper
class Test
{
    private $parser=null;
       
    public function setup()
    {
        $this->parser=new XapianQueryParser();
        $stopper=new XapianSimpleStopper(); // no need to add stopwords 
to reproduce the bug
        $this->parser->set_stopper($stopper);
    }
   
    public function parse($request)
    {   
        return $this->parser->parse_Query($request); // hangs here when 
qp tries to access stopper
    }
}
$t=new test();
$t->setup();
echo $t->parse('a test of the query parser')->get_description();
echo "done\n"; // not reached
---[end]---

This script hangs under apache but also if you run it from the command 
line (I'm using php 5.2.2). However, on my box, php cli gives no clue 
about the problem : the only "symptoms" are that the final 'echo done' 
is never reached and that an event is logged in the windows event viewer.

n.b. if the two functions are rewritten as a single one (setting up the 
parser and using it just after), there's no problem.

Here is my guess about what happens:

The important point is that in setup(), $stopper is a local variable.
At the end of setup() the variable is deleted which, I think, causes 
Xapian to free the underlying c++ object
(that's probably not correct as the parser still references it).
Then, $this->parser has a reference to a stopper which does no longer 
exists and hangs the first time it tries to use it (in parse)...

Is-it a bug? My scenario is probably not the common usage (setting up 
the query parser in one function, using it in another one).
On the other hand, the internal stopper should not be deleted if another 
object still uses it, no?

A simpler way to demonstrate the problem could be:
---[start]---
$parser=new XapianQueryParser();
$parser->set_stopper(new XapianSimpleStopper());
$parser->parse_Query('a test for the query parser'); // hangs
---[end]---

Last, if my assumption is correct, perhaps there are other scenarios 
where this problem can appear (like setting the weighting scheme for an 
enquire in one place and using it elsewhere)? I can't easily test this 
one...
>> 3. I was surprised by the precedence of the "NOT" operator:
>> QueryParser parses "a OR b NOT c" as "(a OR b) AND_NOT c"
>> I thought it would have been "a OR (b AND_NOT c)" but I'm probably wrong 
>> as queryparser.lemony states that NOT has lower priority than OR...
>>     
> I think you've misunderstood the comment - I believe it gives equal
> priority to NOT, OR, and XOR.  All these are lower than AND.
>   
you're right, I mixed up precedence and associativity... All operators 
between %left and the next '.' have the same priority.
> Perhaps that's wrong - it seems like NOT should probably bind more
> tightly (...)
>
> So either most people don't notice (or care), or it's working as most
> people expect.  I suspect the former, so it's probably OK to change
> this.
>   
I also think that "not" should be given higher precedence, even if I was 
not able to find doc from google/yahoo/others stating explicitly on that 
point.
However, I'm not sure it is worth changing this if everyone is happy 
with that (I was not asking for a change, just saying I was surprised)...

Cheers,

-- 

Daniel Ménard