Closed PJK closed 12 years ago
Agreed, I have a local patch already.
D. Sent from my mobile On Feb 25, 2012 9:09 PM, "Pavel J. K." < reply@reply.github.com> wrote:
Xapian supports two special types of queries - MatchAll and MatchNothing ( http://xapian.org/docs/apidoc/html/classXapian_1_1Query.html#9e30b80314359e5ddd7ca4ffa84b6edd). It would be really helpful to have them available via XapianFu. The bindings already support them.
Reply to this email directly or view it on GitHub: https://github.com/johnl/xapian-fu/issues/15
Hrm, I'm now actually not sure about this. Even though these constants have been in the native library for many years, they were only added to the Ruby bindings last year. So most Linux distros don't have new enough packages for this to work - so XapianFu will be broken on most distros until they catch up.
I think this is unacceptable for such a small feature.
I've pushed a fix for this which changes the interface to symbols (:all and :nothing). It's on the branch 15_fix_special_queries so let me know what you think.
I completely understand your point, but your fix introduces even worse BC break: previously, all symbols would get converted to strings. So it was ok to pass a symbol to XapianDb#search. Now they don't, which obviously results in an exception:
/home/pjk/.rvm/gems/ruby-1.9.3-p0/bundler/gems/xapian-fu-5e1fbeb50c8a/lib/xapian_fu/query_parser.rb:84:in `parse_query': Wrong arguments for overloaded method 'QueryParser.parse_query'. (ArgumentError)
Possible C/C++ prototypes are:
Xapian::Query QueryParser.parse_query(std::string const &query_string, unsigned int flags, std::string const &default_prefix)
Xapian::Query QueryParser.parse_query(std::string const &query_string, unsigned int flags)
Xapian::Query QueryParser.parse_query(std::string const &query_string)
from /home/pjk/.rvm/gems/ruby-1.9.3-p0/bundler/gems/xapian-fu-5e1fbeb50c8a/lib/xapian_fu/query_parser.rb:84:in `parse_query'
from /home/pjk/.rvm/gems/ruby-1.9.3-p0/bundler/gems/xapian-fu-5e1fbeb50c8a/lib/xapian_fu/xapian_db.rb:234:in `search'
from test.rb:11:in `<main>'
If you want to maintain compatibility, then we should probably a) add 2 new methods for those special searches or b) add equivalent constant queries to XapianFu itself.
hrm, we didn't have a test for searching using a symbol. Arguably it was an unofficial behaviour :)
Out of interest, what is your use case for using a symbol to do a search?
Feels to me like a symbol is a good thing to use for this kind of thing - it's impossible for a user in a webapp (or via a cli tool or whatever) to actually provide a symbol to the app, so it's a nice differentiator.
It's a similar(ish) kind of behaviour to ActiveRecord too, which uses find(1234) or find(:all) or find(:first).
I think I'd prefer to raise a nicer exception (explaining it was an unknown special search type) and just request that you supply a string if you need.
What do you think?
I don't use symbols for any queries. Why would I? I guess no one does. I was just pointing out that you will break compatibility anyway. But the implicit conversions to strings for any object passed is a part of the interface.
I'm fine with those symbols; it feels very idiomatic. It was the first things I thought of, but because of the implicit conversion I used the special queries to make it more robust and explicit. You maintain, you decide.
Ooops, sorry, wrong button
Thanks for your input Pavel.
So it only breaks compatibility if someone was using an undocumented feature, and we can't think of any real use cases for it really anyway. I think it's a pretty good interface. I'm going to leave it in. Closing this issue now.
Thanks again!
Xapian supports two special types of queries - MatchAll and MatchNothing (http://xapian.org/docs/apidoc/html/classXapian_1_1Query.html#9e30b80314359e5ddd7ca4ffa84b6edd). It would be really helpful to have them available via XapianFu. The bindings already support them.