Mailing List Archive: kinosearch: discuss

KinoSearch: Patch extending query language in 0.15.

Index | Next | Previous | View Threaded

matthew.oconnor at socialtext

Sep 6, 2007, 4:07 PM

Post #1 of 3 (762 views)
Permalink

KinoSearch: Patch extending query language in 0.15.

Marvin, (resending to list)

I have attached a patch which slightly extends the query language in
KinoSearch 0.15. It adds the ability to specify a field on a logical
group. So if I have field "foo" and terms "bar" and "baz" then "foo:
(bar baz)" is equivalent to "(foo:bar foo:baz)". The expression "foo:
(bar baz)" formerly had no semantics, but my implementation seems
like a reasonable interpretation.

The patch contains documentation and tests. Would you consider
including this in a new version of KinoSearch based on the 0.15 code
base?

Thank you!

-matthew

Attachments:

001-field-on-boolean-group.patch (2.73 KB)

marvin at rectangular

Sep 6, 2007, 4:33 PM

Post #2 of 3 (697 views)
Permalink

Re: KinoSearch: Patch extending query language in 0.15. [In reply to]

On Sep 6, 2007, at 4:07 PM, Matthew O'Connor wrote:

> Marvin, (resending to list)

Thanks, I definitely prefer to handle this sort of thing out in the
public forum. :)

> So if I have field "foo" and terms "bar" and "baz" then "foo:(bar
> baz)" is equivalent to "(foo:bar foo:baz)". The expression "foo:
> (bar baz)" formerly had no semantics, but my implementation seems
> like a reasonable interpretation.

I think this is a very nice improvement in terms of consistency.

I see that your patch also adds a second argument to the public API
of QueryParser->parse:

=head1 METHODS
@@ -373,12 +377,16 @@

=head2 parse

- my $query = $query_parser->parse( $query_string );
+ my $query = $query_parser->parse( $query_string, $fields );

Turn a query string into a Query object. Depending on the contents
of the
query string, the returned object could be any one of several
subclasses of
L<KinoSearch::Search::Query|KinoSearch::Search::Query>.

+The default fields to search are taken from the C<$fields>
arrayref. If this
+is not passed in then the default fields from the query parser
object are
+used.
+
=head1 COPYRIGHT

The underlying functionality is needed because of how your patch
works (kudos on its simplicity and elegance, BTW), but I think we
ought to leave that second argument private. It's not needed because
the constructor allows you to specify custom fields already and we're
just duplicating that functionality. If you really need to set up
custom fields, you can create multiple QueryParser objects -- they
aren't cumbersome.

Marvin Humphrey
Rectangular Research
http://www.rectangular.com/

_______________________________________________
KinoSearch mailing list
KinoSearch [at] rectangular
http://www.rectangular.com/mailman/listinfo/kinosearch

matthew.oconnor at socialtext

Sep 6, 2007, 6:10 PM

Post #3 of 3 (700 views)
Permalink

Re: KinoSearch: Patch extending query language in 0.15. [In reply to]

On Sep 6, 2007, at 4:33 PM, Marvin Humphrey wrote:

>
> On Sep 6, 2007, at 4:07 PM, Matthew O'Connor wrote:
>
>> Marvin, (resending to list)
>
> Thanks, I definitely prefer to handle this sort of thing out in the
> public forum. :)
>
>> So if I have field "foo" and terms "bar" and "baz" then "foo:(bar
>> baz)" is equivalent to "(foo:bar foo:baz)". The expression "foo:
>> (bar baz)" formerly had no semantics, but my implementation seems
>> like a reasonable interpretation.
>
> I think this is a very nice improvement in terms of consistency.

While testing this patch at Socialtext we ran into a few surprises,
which I am not sure are wrong. I originally intended that "foo:(bar
baz)" would be the same as "foo:bar foo:baz" regardless of what foo,
bar, and baz were. However, that's certainly not the case when "bar"
or "baz" end in a colon.

In the case of "foo:(bar: baz)" we have that being the same as
"bar:baz" and not "foo:bar: foo:baz" as I originally thought. I
think this is probably okay and that the way it works, even though I
didn't intend that, is probably less surprising on the whole. One
generally expects their "tag:" prefixes to have an effect and having
them not would be weird.

Does that make sense? And if so, do you agree?

> The underlying functionality is needed because of how your patch
> works (kudos on its simplicity and elegance, BTW), but I think we
> ought to leave that second argument private. It's not needed
> because the constructor allows you to specify custom fields already
> and we're just duplicating that functionality. If you really need
> to set up custom fields, you can create multiple QueryParser
> objects -- they aren't cumbersome.

I agree, leaving it private is probably for the best. I was just
being a dutiful documenter. I removed it from the patch and reattched.

-matthew

Attachments:

001-field-on-boolean-group.patch (2.15 KB)

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads