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

Allow scripts to handle missing mappings gracefully #22056

Closed
Bargs opened this issue Dec 8, 2016 · 11 comments · Fixed by #29036
Closed

Allow scripts to handle missing mappings gracefully #22056

Bargs opened this issue Dec 8, 2016 · 11 comments · Fixed by #29036
Assignees
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >docs General docs changes

Comments

@Bargs
Copy link

Bargs commented Dec 8, 2016

Elasticsearch version: 6.0, 5.x
Plugins installed: none
JVM version: 1.8.0_60
OS version: OSX El Capitan

Description of the problem including expected versus actual behavior:

If a script attempts to access a field that does not exist in the mappings of one or more target indices, you'll get an error saying "Field [foo] does not exist in mappings".

Steps to reproduce:

  1. Execute a query with a script field accessing a non-existent field
  2. Observe error response

When querying wildcard index patterns, it's quite possible that some indices will contain mappings that others do not. It would be nice if the user had the option to handle the missing fields gracefully within the script instead of blowing up the entire request.

@rjernst
Copy link
Member

rjernst commented Dec 8, 2016

You can use the containsKey method on the doc to see if the field exists (doc implements Map):

doc.containsKey('foo') ? doc['foo'] : /* some default for indexes without foo */

@Bargs
Copy link
Author

Bargs commented Dec 8, 2016

Ah interesting, is that documented anywhere? My natural inclination was to try a null check (doc['foo'] === null ? ...) but that still throws an error. I suspect other devs would expect a null check to suffice as well, it's odd for a missing key to throw an error instead of simply returning null. Is that something you might consider changing?

Also, is there an equivalent to containsKey when using lucene expressions?

@nik9000
Copy link
Member

nik9000 commented Dec 8, 2016

Is that something you might consider changing?

The nice thing about throwing the exception is that you know why the you got null. I think this one is probably best solved with documented examples or a change to the error message in the exception or both.

I don't know about expressions though.

@jdconrad
Copy link
Contributor

jdconrad commented Dec 8, 2016

I can't remember off the top of my head for expressions, but if I had to guess it will just be 0.0 since expressions assumes all values are doubles and can't represent null.

Edit: I take that back. I think it will be a run-time error where it can't bind the appropriate field.

@Bargs
Copy link
Author

Bargs commented Dec 8, 2016

@jdconrad I'm not sure I follow. When I write an expression script that accesses a field with no mapping in the index, I get the same error, not a value of 0.0 or null.

@nik9000 Re-reading Ryan's comment I realized he mentioned that doc implements Map. Out of curiosity I tried a script like doc.get('foo') where foo has no mapping. I got the same error about missing mappings. Doesn't this break Map's contract, which specifies that get should return null if the map contains no mapping for a given key?

@jdconrad
Copy link
Contributor

jdconrad commented Dec 8, 2016

Yes, my first comment was incorrect about expressions, hence the edit :)

All the 'doc' classes extend Map, and the get method is overridden to have that error instead of returning null. Using containsKey will use the default Map method, so the error won't appear.

@Bargs
Copy link
Author

Bargs commented Dec 9, 2016

Ah, I missed the edit :)

Well, I'd be ok with just improving the docs, throwing an error on a missing key just felt a little strange to me. It would be nice if we had a way to handle this gracefully with expressions as well though.

@clintongormley clintongormley added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache help wanted adoptme >docs General docs changes labels Dec 9, 2016
@jdconrad
Copy link
Contributor

jdconrad commented Dec 9, 2016

Agree completely about the documentation. It's on a long list of things to help with scripting.

@zyong812
Copy link

zyong812 commented Aug 8, 2017

Nice, this has solved my problem.

@jdconrad jdconrad self-assigned this Mar 13, 2018
@jdconrad jdconrad removed the help wanted adoptme label Mar 13, 2018
@jdconrad
Copy link
Contributor

This just needs to be part of the documentation update.

rjernst added a commit to rjernst/elasticsearch that referenced this issue Mar 13, 2018
This commit adds a documentation note about the behavior when trying to
access docvalues for a field which does not exist in mappings.

closes elastic#22056
rjernst added a commit that referenced this issue Mar 21, 2018
This commit adds a documentation note about the behavior when trying to
access docvalues for a field which does not exist in mappings.

closes #22056
rjernst added a commit that referenced this issue Mar 21, 2018
This commit adds a documentation note about the behavior when trying to
access docvalues for a field which does not exist in mappings.

closes #22056
@kripafixstream
Copy link

Thanks so much this really help solve my problem !!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >docs General docs changes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants