update fails silently if script and doc update are both in one line

[robin13]

Elasticsearch version (bin/elasticsearch --version): 6.4.1

Plugins installed: []

JVM version (java -version): OpenJDK 64-bit Server VM 10.0.2

OS version (uname -a if on a Unix-like system): Linux 4.15.0-34-generic #37-Ubuntu SMP

Description of the problem including expected versus actual behavior:

If you (accidentally) include both a script and an update:doc action in a bulk update, it does not always return the appropriate error and can fail to execute a script silently.

Steps to reproduce:

Setup:

PUT /test
PUT /test/_doc/test
{  "message":"hello world" }

The expected behaviour - when both script and update:doc actions are used in this order:

POST _bulk
{"update":{"_index":"test","_type":"_doc","_id":"test"}}
{"script":{"source":"ctx._source.message = 'set by script'"},"update":{"doc":{"message":"set by update:doc"}}}

Elasticsearch correctly responds with the error:

{
  "error": {
    "root_cause": [
      {
        "type": "action_request_validation_exception",
        "reason": "Validation Failed: 1: can't provide both script and doc;"
      }
    ],
    "type": "action_request_validation_exception",
    "reason": "Validation Failed: 1: can't provide both script and doc;"
  },
  "status": 400
}

But if you switch the order putting update:doc first, then script:

POST _bulk
{"update":{"_index":"test","_type":"_doc","_id":"test"}}
{"update":{"doc":{"message":"set by update:doc"}},"script":{"source":"ctx._source.message = 'set by script'"}}

It returns successfully:

{
  "took": 9,
  "errors": false,
  "items": [
    {
      "update": {
        "_index": "test",
        "_type": "_doc",
        "_id": "test",
        "_version": 5,
        "result": "updated",
        "_shards": {
          "total": 2,
          "successful": 1,
          "failed": 0
        },
        "_seq_no": 4,
        "_primary_term": 1,
        "status": 200
      }
    }
  ]
}

But only the update has been done, and the script section has been silently ignored:

GET /test/_doc/test
{
  "_index": "test",
  "_type": "_doc",
  "_id": "test",
  "_version": 5,
  "found": true,
  "_source": {
    "message": "set by update:doc"
  }
}

It would be good to

1. Fail consistently when both update:doc and script are provided regardless of order
2. Add explicit details to the documentation noting that only one action can be used per update

[rjernst]

The bulk api uses a hand rolled parser that is quite complex. Only the main actions (delete, create, index, update) are looked for. My hunch is the success you got is not silently ignore script failure, but never even parsing the script portion, likely due to the scope of the slice parser used to parse the update portion (see BulkRequest line ~419).

[elasticmachine]

Pinging @elastic/es-distributed

[vladimirdolzhenko]

#29293 changed behaviour in 7.0 - it is not possible to specify bulk request as

{"update":{"_index":"test","_type":"_doc","_id":"test"}}
{"update":{"doc":{"message":"set by update:doc"}},"script":{"source":"ctx._source.message = 'set by script'"}}

unnecessary update leads to a parser exception like [UpdateRequest] unknown field [update], parser not found

for 7.0 both following cases throw validation exception like you've got can't provide both script and doc:

{"update":{"_index":"test","_type":"_doc","_id":"test"}}
{"doc":{"message":"set by update:doc"},"script":{"source":"ctx._source.message = 'set by script'"}}

and

{"update":{"_index":"test","_type":"_doc","_id":"test"}}
{"script":{"source":"ctx._source.message = 'set by script'"}, "doc":{"message":"set by update:doc"}}

[vladimirdolzhenko]

@rjernst problem in UpdateRequest#fromXContent and how it parses {"update":{"doc":{"message":"set by update:doc"}}, - everything that goes after comma is ignored.

[vladimirdolzhenko]

#29293 is a proper fix for 7.0 already applied.

#35257 is a fix for 6.x branch.