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

Make template params take arrays #8255

Conversation

reuben-sutton
Copy link
Contributor

Currently the templateParams in SearchRequest is a Map<String, String> - this means that it could not take Arrays as part of the parameters, which is not the case when accessing it through the REST api, and also as documented in the docs for this feature.

This changes it to Map<String, Object> so that it can take other data-types.

…so that it can take more data-types than just strings, to support Arrays.
@reuben-sutton reuben-sutton force-pushed the make_template_params_take_arrays branch from fb2cc29 to d601ba6 Compare October 28, 2014 14:28
@reuben-sutton
Copy link
Contributor Author

@GaelTadh Can you review please?

@clintongormley
Copy link

Hi @reuben-sutton. Thanks for the PR. We'll take a look and get back to you. Meanwhile, could I ask you to sign the CLA please: http://www.elasticsearch.org/contributor-agreement/

thanks

@reuben-sutton
Copy link
Contributor Author

Thanks @clintongormley - signed.

@GaelTadh
Copy link
Contributor

Thanks @reuben-sutton I'll take a look.

@reuben-sutton
Copy link
Contributor Author

Hey @GaelTadh, is there anything I can do to move this forward - would quite like to be able to get rid of the hack in my code for working round it.

Thanks,
Reuben

@GaelTadh
Copy link
Contributor

@reuben-sutton I will get to this today or tomorrow at the latest.

@clintongormley
Copy link

Hi @reuben-sutton - I'm afraid you've signed the CLA with a different email address than the the one you've used on GitHub, so we can't be "sure" that you're the same person :) Sorry for the ask, but please could you sign again: http://www.elasticsearch.org/contributor-agreement/

thanks

@reuben-sutton
Copy link
Contributor Author

Hi @clintongormley - Sorry about that, I've signed with my other email too now.

Thanks,
Reuben.

@clintongormley
Copy link

thanks @reuben-sutton - that worked :)

@reuben-sutton
Copy link
Contributor Author

Hi @GaelTadh, do you mind if I bump this again?

@GaelTadh
Copy link
Contributor

I will be looking at this again today

@GaelTadh
Copy link
Contributor

This LGTM, @dakrone is going to take a quick look before I merge it in.

@dakrone
Copy link
Member

dakrone commented Nov 24, 2014

LGTM too!

@GaelTadh
Copy link
Contributor

This has been merged to master and 1.x

@GaelTadh GaelTadh closed this Nov 24, 2014
@lukas-vlcek
Copy link
Contributor

Great, is this going to be available only from 1.5 or is back-porting to 1.3 or 1.4 planned as well?

@GaelTadh
Copy link
Contributor

As it's not a bug fix I haven't ported it to 1.4 or 1.3. @clintongormley what do you think ?

@s1monw
Copy link
Contributor

s1monw commented Nov 24, 2014

no don't port it

@reuben-sutton
Copy link
Contributor Author

Thanks @GaelTadh!
@s1monw - I think it could be argued that it was a bug that it was possible to do this through the REST API but not the Java API.

@dadoonet
Copy link
Member

dadoonet commented Dec 2, 2014

Someone also hit also this issue: https://groups.google.com/d/msgid/elasticsearch/27cafc64-d21b-4fec-8273-3f92ef92ad8f%40googlegroups.com?utm_medium=email&utm_source=footer

I think it's actually a bug in Java API as explained by @reuben-sutton.

I'd be +1 to backport it at least in 1.4

@prateeka
Copy link

has this been ported to 1.4? else please advise what are my options to use java api for query like below where only query: "XXXX", "YYYY", "ZZZZ" change between different search requests .

GET /cobalt-vehicles/vehicles/_search
{
"query": {
"filtered": {
"query": {
"multi_match": {
"query": "XXXX",
"type": "cross_fields",
"fields": [
"field1",
"field2",
"field3"
]
}
},
"filter": {
"terms": {
"owner": [
"YYYY",
"ZZZZ"
]
}
}
}
}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v1.4.5 v1.5.0 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants