Skip to content

Add support for distributed frequencies #8144

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

Closed
wants to merge 6 commits into from

Conversation

alexksikes
Copy link
Contributor

Adds distributed frequencies support for the Term Vectors API. A new parameter
called dfs is introduced which defaults to false.

Adds distributed frequencies support for the Term Vectors API. A new parameter
called `dfs` is introduced which defaults to `false`.

@Override
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

For the two aboce methods, can you rather throw an UnsupportedOperationException if it is not supported and document that this dfs-only request/response objects are for internal use only?

try {
sSource = XContentHelper.convertToJson(searchRequest.source(), false);
} catch (Exception e) {
// ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

when can you get an exception here?

@jpountz
Copy link
Contributor

jpountz commented Oct 21, 2014

This looks good to me overall, however I have no experience at all with the transport layer so it would be good if someone else could review this PR before we get it in

}
}
// wrap a search request object
this.searchRequest = client.prepareSearch(indices).setTypes(types).setQuery(boolBuilder).request();
Copy link
Member

Choose a reason for hiding this comment

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

I think the whole client isn't necessary, you can just do this instead:

this.searchRequest = new SearchRequest(indices).types(types).source(new SearchSourceBuilder().query(boolBuilder));

Then you can also remove the client field in ShardTermVectorService.java.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thx that's much nicer indeed.

@martijnvg
Copy link
Member

@alexksikes This looks good. I left a couple of comments.

@martijnvg
Copy link
Member

@alexksikes this look great! LGTM

alexksikes added a commit that referenced this pull request Oct 23, 2014
Adds distributed frequencies support for the Term Vectors API. A new parameter
called `dfs` is introduced which defaults to `false`.

Closes #8144
@alexksikes alexksikes deleted the feature/tv-dfs branch October 23, 2014 12:04
@alexksikes alexksikes removed the v1.5.0 label Oct 23, 2014
@clintongormley clintongormley changed the title Term Vectors: support for distributed frequencies Add support for distributed frequencies Jun 6, 2015
@clintongormley clintongormley added :Search/Search Search-related issues that do not fall into other categories and removed :Term Vectors labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search/Search Search-related issues that do not fall into other categories v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants