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

Enhancement/terms lookup fixes #12870

Closed

Conversation

javanna
Copy link
Member

@javanna javanna commented Aug 14, 2015

Relates to #12042 (comment) .

  • Java api: remove support for lookup cache in TermsLookupBuilder: TermsQueryParser doesn't support the cache field anymore, so if it gets set through java api, the subsequent parsing of that query will throw error
  • Java api: restore support for minimumShouldMatch and disableCoord in TermsQueryBuilder: TermsQueryParser still parses those values although deprecated. These need to be present in the java api as well to get ready for the query refactoring, where the builders are the intermediate query format that we parse our json queries into. Whatever the parser supports need to be supported by the builder as well.

* @deprecated use [bool] query instead
*/
@Deprecated
public TermsQueryBuilder disableCoord(boolean disableCoord) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this, can you also make the parser use a ParseField that marks it as deprecated?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, done

@javanna javanna force-pushed the enhancement/terms_lookup_fixes branch from 86dce9a to f8986b4 Compare August 14, 2015 09:05
@jpountz
Copy link
Contributor

jpountz commented Aug 14, 2015

LGTM

 TermsQueryParser doesn't support the cache field anymore, so if it gets set through java api, the subsequent parsing of that query will throw error
…TermsQueryBuilder

 TermsQueryParser still parses those values although deprecated. These need to be present in the java api as well to get ready for the query refactoring, where the builders are the intermediate query format that we parse our json queries into. Whatever the parser supports need to be supported by the builder as well.
@javanna javanna force-pushed the enhancement/terms_lookup_fixes branch from f8986b4 to 64bfbd2 Compare August 14, 2015 09:15
javanna added a commit to javanna/elasticsearch that referenced this pull request Aug 14, 2015
 TermsQueryParser doesn't support the cache field anymore, so if it gets set through java api, the subsequent parsing of that query will throw error

Relates to elastic#12870
@javanna javanna closed this in 4010e7e Aug 14, 2015
@javanna javanna removed the review label Aug 14, 2015
@javanna
Copy link
Member Author

javanna commented Aug 14, 2015

Marking this as breaking as the removal of the setter for lookupCache from TermsLookupBuilder breaks the java api.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants