[Xapian-devel] Bug and patch for +terms with wildcards

[Richard Boulton]

>> I've called this operator "OP_MATCH_NOTHING", and it takes no
>> subqueries.
>
> I don't think this should be an operator.  It doesn't operate *on*
> anything, so I think this is an unnatural place to put this.  I know we
> have OP_LEAF internally, but that's an implementation detail and we
> deliberately don't expose it externally, instead we have a constructor
> to create a leaf Query object.

Fair point.

> I agree that Xapian::Query("") isn't totally obvious, but I don't think
> a "nullary" (ick, what an awful word) operator is any better.
>
> We already have a "query which matches nothing" - Xapian::Query().  You
> can run a match on it (and get an empty MSet), but you can't currently
> compose it with anything else.

You can't compose it - but you can test for it (using the "empty()"
method).  The current query parser does this, and composes it in various
ways (when building up lists).

> I think a better approach would be to allow Xapian::Query() to be
> composed with other queries.  I know we used to allow this and it all
> got out of hand, but that was because we tried to make it compose in
> magic ways (e.g. Query() AND X would be X).  I'm just proposing that
> this works in the obvious was for an empty query, and you've already
> implemented this anyway!

I think the current behaviour of Xapian::Query() is good - it can't be
composed, it returns an error if you try and use it, but it is useful as a
placeholder when building up complex queries.  It's also not obvious to me
whether Xapian::Query() should match all documents or no documents - it
depends whether you're thinking of searches as returning all documents
which match a query, or thinking of a search as filtering the database to
remove those documents which don't satisfy a query.

So, I think it would be a mistake to change that.

> And then add two standard constant objects to the API:
>
> Xapian::Query::MatchAll
> Xapian::Query::MatchNothing

I very much like this idea, though.  Having explicit constant queries to
represent MatchAll and MatchNothing would make this much clearer.

> Conceptually these sit much better as constants than operators.
>
>> +	        case '!': {
>> +		    return new
>> Xapian::Query::Internal(Xapian::Query::OP_MATCH_NOTHING, 0);
>> +	        }
>
> I think the remote protocol version probably should be incremented
> because of this change.

Agreed.

I'll rework the patch taking these ideas onboard.

-- 
Richard