Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply the default operator on analyzed wildcard in query_string builder: #17711

Merged
merged 1 commit into from Apr 13, 2016

Conversation

Projects
None yet
4 participants
@jimczi
Copy link
Member

commented Apr 13, 2016

  • Tokens in the same position are grouped in a SynonymQuery.
  • The default operator is applied on tokens in different positions.
  • The wildcard is applied to the terms in the last position only.

Fixes #2183

@jpountz

View changes

core/src/main/java/org/apache/lucene/queryparser/classic/MapperQueryParser.java Outdated
clauses.add(new BooleanClause(super.getPrefixQuery(field, token), BooleanClause.Occur.SHOULD));
for (int i = 0; i < tlist.size(); i++) {
List<String> plist = tlist.get(i);
boolean isLastPos = (i == tlist.size()-1) ? true : false;

This comment has been minimized.

Copy link
@jpountz

jpountz Apr 13, 2016

Contributor

no need for ? true : false

This comment has been minimized.

Copy link
@jimczi

jimczi Apr 13, 2016

Author Member

sure, thanks

@jpountz

View changes

core/src/main/java/org/apache/lucene/queryparser/classic/MapperQueryParser.java Outdated
BooleanClause.Occur.SHOULD));
}
}
posQuery = getBooleanQueryCoordDisabled(innerClauses);

This comment has been minimized.

Copy link
@jpountz

jpountz Apr 13, 2016

Contributor

if all sub queries are term queries with the same boost, it would be nice to build a SynonymQuery instead

This comment has been minimized.

Copy link
@jpountz

jpountz Apr 13, 2016

Contributor

I am fine with doing it in a follow-up PR if that works better for you

This comment has been minimized.

Copy link
@jimczi

jimczi Apr 13, 2016

Author Member

Good idea, I'll add it.

@jpountz

View changes

core/src/test/java/org/elasticsearch/index/query/AbstractQueryTestCase.java Outdated
assertThat(query, instanceOf(PrefixQuery.class));
PrefixQuery prefixQuery = (PrefixQuery) query;
assertThat(prefixQuery.getPrefix().field(), equalTo(field));
assertThat(prefixQuery.getPrefix().text().toLowerCase(Locale.ROOT), equalTo(value.toLowerCase(Locale.ROOT)));

This comment has been minimized.

Copy link
@jpountz

jpountz Apr 13, 2016

Contributor

why the toLowerCase?

This comment has been minimized.

Copy link
@jimczi

jimczi Apr 13, 2016

Author Member

Simple copy/paste ;) This is not needed anyway and I guess it was there because this function is supposed to check queries generated from an analyzer. I'll remove it.

This comment has been minimized.

Copy link
@jpountz

jpountz Apr 13, 2016

Contributor

Then maybe we can just do assertEquals(new PrefixQuery(new Term(field, value)), query)?

@jpountz

This comment has been minimized.

Copy link
Contributor

commented Apr 13, 2016

I left some (minor) comments.

@jimczi

This comment has been minimized.

Copy link
Member Author

commented Apr 13, 2016

Thanks @jpountz . I've pushed another commit to address your comments.

@jimczi

View changes

core/src/main/java/org/apache/lucene/queryparser/classic/MapperQueryParser.java Outdated
import org.apache.lucene.search.MultiPhraseQuery;
import org.apache.lucene.search.PhraseQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.*;

This comment has been minimized.

Copy link
@jimczi

jimczi Apr 13, 2016

Author Member

Oups I forgot to update my intellij configuration.

@jpountz

View changes

core/src/main/java/org/apache/lucene/queryparser/classic/MapperQueryParser.java Outdated
import org.apache.lucene.search.MultiPhraseQuery;
import org.apache.lucene.search.PhraseQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.*;

This comment has been minimized.

Copy link
@jpountz

jpountz Apr 13, 2016

Contributor

beware that wildcard imports will cause the build to fail

@jpountz

This comment has been minimized.

Copy link
Contributor

commented Apr 13, 2016

LGTM

Apply the default operator on analyzed wildcard in query_string builder:
 * Tokens in the same position are grouped into a SynonymQuery..
 * The default operator is applied on tokens in different positions.
 * The wildcard is applied to the terms in the last position only.
Fixes #2183

@jimczi jimczi merged commit 3946d3d into elastic:master Apr 13, 2016

1 check passed

CLA Commit author is a member of Elasticsearch
Details

@jimczi jimczi removed the review label Apr 13, 2016

@jimczi jimczi deleted the jimczi:query_string_wildcard branch Apr 13, 2016

@jimczi jimczi added the v2.4.0 label Apr 18, 2016

@lukapor

This comment has been minimized.

Copy link

commented Jul 12, 2016

Will be this fix (change request) merged in 2.4?

@jpountz

This comment has been minimized.

Copy link
Contributor

commented Jul 13, 2016

Hmm the labels say it was merged in 2.4 but I can't find the commit. @jimferenczi did you forget to backport?

@clintongormley

This comment has been minimized.

Copy link
Member

commented Jul 13, 2016

@jimferenczi I wonder if the 02992ae9d6a4fc42f086e8fab7b2487f868235b4 commit went only into 2.4 and not into 2.x - we replaced the 2.4 branch with 2.x

@jimczi

This comment has been minimized.

Copy link
Member Author

commented Jul 13, 2016

I don't remember :(. I'll check after my vacation next week. Sorry for the delay.

jimczi added a commit that referenced this pull request Jul 20, 2016

Apply the default operator on analyzed wildcard in query_string and s…
…imple_query_string builder

* Tokens in the same position are grouped in a SynonymQuery.
* The default operator is applied on tokens in different positions.
* The wildcard is applied to the terms in the last position only.

Manually merged from #17711
@jimczi

This comment has been minimized.

Copy link
Member Author

commented Jul 20, 2016

I am not sure where the initial merge went but thanks to @lukapor @jpountz and @clintongormley the fix is in 2.4:
02de9cf

synhershko pushed a commit to plugind/builder-elasticsearch-gradle-plugin that referenced this pull request Jan 15, 2018

Apply the default operator on analyzed wildcard in simple_query_strin…
…g builder:

 * This is a followup from elastic/elasticsearch#17711 where we now apply the default operator on analyzed wildcard query in query_string builder.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.