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

Update API doesn't support both script and doc #2967

Closed
akahn opened this Issue May 1, 2013 · 7 comments

Comments

Projects
None yet
3 participants
@akahn
Copy link
Contributor

akahn commented May 1, 2013

In my application I want to update a document by incrementing a counter using script and by passing a partial document to be merged in using doc. However, it appears that only one of these is applied:

curl -XPOST http://localhost:9200/foo/bar/aoeu -d '{"counter": 0, "foo": "bar"}'
curl -XPOST http://localhost:9200/foo/bar/aoeu/_update -d '{"script": "ctx._source.counter += 1", "doc": {"foo": "barbaz"}}'
curl  http://localhost:9200/foo/bar/aoeu
# Returns: {"_index":"foo","_type":"bar","_id":"aoeu","_version":2,"exists":true, "_source" : {"counter":1,"foo":"bar"}}

The counter is incremented but the foo key's value isn't updated.

I expected that my document would be merged and that my script would be executed. But it seems that if doc is passed, then script (as well as upsert) is disregarded. The documentation doesn't mention this and the server doesn't provide a warning (as far as I can tell).

This should either be described in the documentation or combinations of doc, script, and upsert should be accepted and applied.

@martijnvg

This comment has been minimized.

Copy link
Member

martijnvg commented May 2, 2013

I'm not sure if this should be supported, it can lead to unexpected behaviour (like what executes first). It is not a common use case. I think the best way to handle this is to just add your partial document to the script itself.

If both script and doc is specified, then doc is ignored. I agree the documentation should be clearer about this.

@akahn

This comment has been minimized.

Copy link
Contributor Author

akahn commented May 2, 2013

Thanks. Adding my document to the script itself is the workaround I chose.

What do you think of elasticsearch returning a client error when multiple update methods are provided?

@martijnvg

This comment has been minimized.

Copy link
Member

martijnvg commented May 2, 2013

Returning a client error makes sense when both script and doc is specified. This should be added.

Btw: you can also add your partial document as a parameter to the script. This is a nice optimisation because it allows Elasticsearch to cache the compiled version of the script.

@ghost ghost assigned martijnvg May 2, 2013

@akahn

This comment has been minimized.

Copy link
Contributor Author

akahn commented May 2, 2013

Cool. I will take a look at that and try to submit a patch.

Can you explain what you mean by adding a partial document as a parameter to the script? Right now I'm just doing ctx._source.count += 1; ctx._source.foo = 'barbaz';….

@akahn

This comment has been minimized.

Copy link
Contributor Author

akahn commented May 9, 2013

@martijnvg The above patch makes elasticsearch consider an update request invalid if it contains both both doc and script.

@martijnvg

This comment has been minimized.

Copy link
Member

martijnvg commented May 9, 2013

@akahn Looks good, I will pull this in soon.

I meant that 1 and barbaz are provided as argument to the script like this:

"script" : "ctx._source.counter += count; ctx._source.foo = foo;",
    "params" : {
        "count" : 4,
        "foo" : "barbaz"
    },

@martijnvg martijnvg closed this in 2be23d2 May 10, 2013

martijnvg added a commit that referenced this issue May 10, 2013

Added test that checks if a validation error is thrown when both doc …
…and script provided in a update request.

Closes #2967

mute pushed a commit to mute/elasticsearch that referenced this issue Jul 29, 2015

@thefourtheye

This comment has been minimized.

Copy link
Contributor

thefourtheye commented Nov 14, 2017

I think the best way to handle this is to just add your partial document to the script itself.

How do I do that? Could someone give an example?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.