This logic was removed by #1122348: Rework query class to provide uniform methods of getting and setting params, but we need it.

CommentFileSizeAuthor
#5 1154770-5.patch773 bytesjpmckinney
#3 1154770-3.patch742 bytesjpmckinney
AddDoubleQuotesIfNecessary.patch651 bytesjpmckinney

Comments

jpmckinney’s picture

jpmckinney commented

Just saw this @todo:

    // @todo - escape the value if it has spaces in it and is not a range query or parenthesized.

Not sure the code should be in addFilter and not makeFilterQuery as in the patch. makeFilterQuery seems appropriate as it is what prepares queries for Solr.

pwolanin’s picture

pwolanin commented

Well, the output of makeFilterQuery() should be something we can feed back in to addFilter(), but in any case I think we should indeed add the escaping when doing the make filter operation.

jpmckinney’s picture

jpmckinney commented
StatusFileSize
new 1154770-3.patch742 bytes

If the output of makeFilterQuery() may be run through makeFilterQuery() again, then we should check to make sure there are not already double quotes.

pwolanin’s picture

pwolanin commented

Why \A \Z ? Does this handle parenthesized values?

jpmckinney’s picture

jpmckinney commented
StatusFileSize
new 1154770-5.patch773 bytes

I wasn't aware you could parenthesize values, but apparently you can (e.g. "Field Grouping" on this page http://lucene.apache.org/java/2_4_0/queryparsersyntax.html).

In a case like "title:(+foo +bar)", we would not want to wrap the parameter value in double quotes, even though it contains a space, as that would change the semantics. And for "title:(+foo +bar bbq)", we can't be sure whether to wrap "bar bbq" in double quotes or not. If parentheses are present, I don't think it's possible to figure out where and when to wrap in double quotes.

Wrapping in double quotes is to protect non-power users from hitting frustrating roadblocks when, for example, they try to assign a value containing a space to an fq param. I assume only power users would be writing code that wraps values in parentheses. They then carry the burden of properly quoting their values, since we can't do it for them.

This new patch will not touch the value if it is already wrapped in double quotes or in parentheses.

pwolanin’s picture

pwolanin commented

for the sake of readability, let's use ^ and $ instead of \A and \Z. A value should not be multi-line.

jpmckinney’s picture

jpmckinney commented
Status: Needs review » Fixed

Ok. Committed http://drupalcode.org/project/apachesolr.git/commit/e45488e

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

msamavi’s picture

msamavi commented

Hi,

I am using 1.x dev version of 2012-Apr-13 but have got the above problem (though it seems that the patch has already been implemented)!
Any help please?

David_Rothstein’s picture

David_Rothstein commented

See related issue at #1803512: The OR operator returns no results for facets with colons in the value (such as entity reference facets) (especially if you're still experiencing a similar bug).