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

[Java API] IdsQueryBuilder allow merging with list #5089

Closed
nfx opened this issue Feb 11, 2014 · 2 comments · Fixed by #11409
Closed

[Java API] IdsQueryBuilder allow merging with list #5089

nfx opened this issue Feb 11, 2014 · 2 comments · Fixed by #11409
Assignees

Comments

@nfx
Copy link

nfx commented Feb 11, 2014

Currently IdsQueryBuilder does support only appending arrays of identifiers and converting them to lists:

public IdsQueryBuilder addIds(String... ids) {
        values.addAll(Arrays.asList(ids));
        return this;
    }

In cases of huge numbers of IDs submitted to builder it could be more wise of performance to merge with lists directly. Or are there any better ideas?

@spinscale spinscale self-assigned this Feb 12, 2014
@spinscale
Copy link
Contributor

Hey,

I am not sure I follow your request. Can you explain, which performance exactly you intend to improve? Right now the datastructure used there is a list. We could possibly use a set to prevent duplicate or use a HPPC collection to be more garbage collection friendly/faster. Is this what you mean with better merging performance?

Also wondering how much ids you intend to put into a single request. Keep in mind, that you will need to get the data from elasticsearch and return it to the client. It might not be a good idea to get several million entries per request.

Any helping comments are greatly appreciated here! Thanks a lot!

@nfx nfx closed this as completed Feb 13, 2014
@nfx
Copy link
Author

nfx commented Mar 9, 2015

It's just for the ease of use - applications collect ids as lists and it's easier to pass them directly inside of method calls

@nfx nfx reopened this Mar 9, 2015
spinscale added a commit to spinscale/elasticsearch that referenced this issue May 29, 2015
In case a developer gets a list of ids from another data source,
it does not make a lot of sense, to convert it to an array first,
and then internally in IdsQueryBuilder elasticsearch creates a
list out of this.

Closes elastic#5089
spinscale added a commit to spinscale/elasticsearch that referenced this issue Jun 9, 2015
In case a developer gets a list of ids from another data source,
it does not make a lot of sense, to convert it to an array first,
and then internally in IdsQueryBuilder elasticsearch creates a
list out of this.

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

Successfully merging a pull request may close this issue.

2 participants