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

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
8 participants
@reuben-sutton
Contributor

reuben-sutton commented Oct 28, 2014

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.

Fix SearchRequest.templateParams so that it is a Map<String, Object> …
…so that it can take more data-types than just strings, to support Arrays.
@reuben-sutton

This comment has been minimized.

Show comment
Hide comment
@reuben-sutton

reuben-sutton Oct 28, 2014

Contributor

@GaelTadh Can you review please?

Contributor

reuben-sutton commented Oct 28, 2014

@GaelTadh Can you review please?

@clintongormley

This comment has been minimized.

Show comment
Hide comment
@clintongormley

clintongormley Oct 28, 2014

Member

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

Member

clintongormley commented Oct 28, 2014

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

This comment has been minimized.

Show comment
Hide comment
@reuben-sutton

reuben-sutton Oct 28, 2014

Contributor

Thanks @clintongormley - signed.

Contributor

reuben-sutton commented Oct 28, 2014

Thanks @clintongormley - signed.

@GaelTadh

This comment has been minimized.

Show comment
Hide comment
@GaelTadh

GaelTadh Oct 28, 2014

Contributor

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

Contributor

GaelTadh commented Oct 28, 2014

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

@reuben-sutton

This comment has been minimized.

Show comment
Hide comment
@reuben-sutton

reuben-sutton Nov 10, 2014

Contributor

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

Contributor

reuben-sutton commented Nov 10, 2014

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

This comment has been minimized.

Show comment
Hide comment
@GaelTadh

GaelTadh Nov 10, 2014

Contributor

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

Contributor

GaelTadh commented Nov 10, 2014

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

@clintongormley

This comment has been minimized.

Show comment
Hide comment
@clintongormley

clintongormley Nov 10, 2014

Member

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

Member

clintongormley commented Nov 10, 2014

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

This comment has been minimized.

Show comment
Hide comment
@reuben-sutton

reuben-sutton Nov 10, 2014

Contributor

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

Thanks,
Reuben.

Contributor

reuben-sutton commented Nov 10, 2014

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

Thanks,
Reuben.

@clintongormley

This comment has been minimized.

Show comment
Hide comment
@clintongormley

clintongormley Nov 10, 2014

Member

thanks @reuben-sutton - that worked :)

Member

clintongormley commented Nov 10, 2014

thanks @reuben-sutton - that worked :)

@reuben-sutton

This comment has been minimized.

Show comment
Hide comment
@reuben-sutton

reuben-sutton Nov 18, 2014

Contributor

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

Contributor

reuben-sutton commented Nov 18, 2014

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

@GaelTadh

This comment has been minimized.

Show comment
Hide comment
@GaelTadh

GaelTadh Nov 24, 2014

Contributor

I will be looking at this again today

Contributor

GaelTadh commented Nov 24, 2014

I will be looking at this again today

@GaelTadh

This comment has been minimized.

Show comment
Hide comment
@GaelTadh

GaelTadh Nov 24, 2014

Contributor

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

Contributor

GaelTadh commented Nov 24, 2014

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

@dakrone

This comment has been minimized.

Show comment
Hide comment
@dakrone

dakrone Nov 24, 2014

Member

LGTM too!

Member

dakrone commented Nov 24, 2014

LGTM too!

@GaelTadh

This comment has been minimized.

Show comment
Hide comment
@GaelTadh

GaelTadh Nov 24, 2014

Contributor

This has been merged to master and 1.x

Contributor

GaelTadh commented Nov 24, 2014

This has been merged to master and 1.x

@GaelTadh GaelTadh closed this Nov 24, 2014

@lukas-vlcek

This comment has been minimized.

Show comment
Hide comment
@lukas-vlcek

lukas-vlcek Nov 24, 2014

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?

Contributor

lukas-vlcek commented Nov 24, 2014

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

This comment has been minimized.

Show comment
Hide comment
@GaelTadh

GaelTadh Nov 24, 2014

Contributor

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

Contributor

GaelTadh commented Nov 24, 2014

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

@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Nov 24, 2014

Contributor

no don't port it

Contributor

s1monw commented Nov 24, 2014

no don't port it

@reuben-sutton

This comment has been minimized.

Show comment
Hide comment
@reuben-sutton

reuben-sutton Nov 24, 2014

Contributor

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.

Contributor

reuben-sutton commented Nov 24, 2014

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

This comment has been minimized.

Show comment
Hide comment
@dadoonet

dadoonet Dec 2, 2014

Member

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

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

This comment has been minimized.

Show comment
Hide comment
@prateeka

prateeka Jan 16, 2015

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"
]
}
}
}
}
}

prateeka commented Jan 16, 2015

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