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 GetField behavior more consitent for multivalued fields. #3015

Merged
merged 1 commit into from May 9, 2013

Conversation

imotov
Copy link
Contributor

@imotov imotov commented May 8, 2013

Before this change, the GetField#getValue() method was returning a list of values of a multivalued fields if the field values were obtained from source or if the field was stored and real-time get was used. If the field was stored but non-realtime get was used, GetField#getValue() was returning only the first element and the GetField#getValues() was returning a list of elements. This change makes behavior consistent. GetField#getValue() now always returns only the first value of the field and GetField#getValues() returns the entire list.

@kimchy
Copy link
Member

kimchy commented May 8, 2013

Looks good!, lets backport to 0.90 as well.

Before this change, the GetField#getValue() method was returning a list of values of a multivalued fields if the field values were obtained from source or if the field was stored and real-time get was used. If the field was stored but non-realtime get was used, GetField#getValue() was returning only the first element and the GetField#getValues() was returning a list of elements. This change makes behavior consistent. GetField#getValue() now always returns only the first value of the field and GetField#getValues() returns the entire list.
@imotov imotov merged commit 4d66575 into elastic:master May 9, 2013
@btiernay
Copy link

btiernay commented Jun 6, 2013

For nested types, what should be the expected behavior here? After upgrading to 0.90.1 our ES client code broke semantically since now getValue only returns the first nested doc. However, there is now no way to determine at query time if the value should be single value or not without knowing the mapping.

Even if the mapping is known, there is no way to distinguish an array object type from an "object" object type. This makes it very difficult to proxy elasticsearch queries dynamically (ex. dynamically created queries for which you wouldn't know until runtime which fields are returned ), and requires access to the mapping and manually maintained metadata to know which method to call for a single vs multivalue field.

@btiernay
Copy link

btiernay commented Jun 6, 2013

I thought about this a little more. Would it be possible to add a new method to GetField such as isMultiValued to return whether or not the field is mapped as having multiple values? This would be similar to LongValues, DoubleValues, BytesValues, etc. This would be a suitable compromise for both consistency and general usage.

@btiernay
Copy link

btiernay commented Jul 2, 2013

@imotov: Just curious to your thoughts on this. We don't really have a good solution for working around this right now. Is there another mechanism I'm overlooking? Thank again.

@imotov
Copy link
Contributor Author

imotov commented Jul 2, 2013

@btiernay I don't think I fully understand the problem. In elasticsearch any field can be multivalued. There is no "array" type because any field can accept and array of values. Are you trying to distinguish between an array that contains one element and a single element that wasn't passed to elasticsearch as an array? If not, could you provide some examples?

@btiernay
Copy link

btiernay commented Jul 2, 2013

@imotov: I'm trying to distinguish x (non-nested) in the following document:

{x: [1, 2, 3]}

from:

{x: 1}

So that when a query requesting x be returned, it is known that getValue or getValues should be called. At design time this is known, but at query time it is not.

What I am getting from you is that since ES makes no distinction between the two, it is up to the application to manage this information. Is that correct?

@imotov
Copy link
Contributor Author

imotov commented Jul 2, 2013

There is a difference between {"x": [1,2,3]} and {"x": 1}, for the first record GetField#getValues() returns a list with 3 elements and for the second with only 1. However, there is currently no way to distinguish between {"x": [1]} and {"x": 1}.

@kimchy
Copy link
Member

kimchy commented Jul 2, 2013

@btiernay there isn't a way today, but we are going to support it pretty soon. In search we actually support it with teh concept of partial fields, and we plan to add it to get, and actually simplify partial fields even more.

@btiernay
Copy link

btiernay commented Jul 2, 2013

@imotov: Thanks for the explanation.
@kimchy: Thanks for the update. Is there a ticket I can follow to be sure that I don't miss it?

@kimchy
Copy link
Member

kimchy commented Jul 2, 2013

@btiernay I don't think we have a ticket open yet, there should be one open pretty soon where we can brainstorm if needed.

@btiernay
Copy link

btiernay commented Jul 2, 2013

@kimchy: Awesome thanks again for such a great product :)

@btiernay
Copy link

@kimchy Was the feature you mentioned in your previous message ever added? We are struggling with this issue now that we are upgrading from 0.90.1 to 1.4. Cheers.

jpountz added a commit to jpountz/elasticsearch that referenced this pull request Nov 21, 2014
…ey produce a list.

This change is essentially the same as elastic#3015 but on script fields.
@bleskes
Copy link
Contributor

bleskes commented Nov 22, 2014

jpountz added a commit that referenced this pull request Nov 24, 2014
…ey produce a list.

This change is essentially the same as #3015 but on script fields.

Close #8592
@imotov imotov deleted the fix-get-field-inconsistency branch May 1, 2020 22:10
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

4 participants