#139: Fix Failed to Update Child Documents

[bstst]

This fix ensures the routing is used properly when updating parent-child related documents.

[divyanshu013]

@bstst Will this also work for ES 6.x?

[bstst]

@divyanshu013, I haven't tested it on 6.x. But this fix only appends additional parameters to the update request body when routing is specified as required in the index mapping. If no one else will test this I'll pull a 6.x image some time later and will test it myself.

[siddharthlatest]

@bstst Yes, this would append a parent query string value. I am not that familiar with the parent-child / joins and I couldn't find any mention on the Elasticsearch official docs for 6.x if this would be allowed (or have the desired effect).

It would be awesome if you can check on that - as I feel this is one of things they have made breaking changes to since v5.

[bstst]

@siddharthlatest, since there is no parent-child mapping in v6 — my changes don't seem to affect the way dejavu works with v6.

The way dejavu DOES or DOESN'T work with v6 is another question. That is out of scope of this issue, but there seem to be quite a bit of editing issues in there. I suppose they should be addressed in another issue.

[siddharthlatest]

You are checking for mapping._routing and mapping._routing._required and based on it, adding a parent query string param with the routing path to the request. As long as doing this doesn't result in a failed index request on ES 6, I am okay with merging this PR. @bstst Do you know if this will hold true? I can't find anything in their docs about it.

I agree that we should address the parent-child join relationship in ES 6 in a different issue.

[siddharthlatest]

I tried specifying a parent query string on ES v6.2.3 without an existing parent-child mapping and it threw this:

{  
   "error":{  
      "root_cause":[  
         {  
            "type":"illegal_argument_exception",
            "reason":"can't specify parent if no parent field has been configured"
         }
      ],
      "type":"illegal_argument_exception",
      "reason":"can't specify parent if no parent field has been configured"
   },
   "status":400
}

I suspect this would fail on ES v5 as well.

It would be great if we can add an additional condition to verify if there indeed is a parent-child mapping before applying the parent query string.

Edit: This change also makes this PR modification inert to ES v6 as a parent-child mapping doesn't exist at the type level there and the parent query string wouldn't get added. (It would result in a failure, but at least it would be transparent to the user)

[bstst]

@siddharthlatest, could you show me the mapping output of the index? or just a snippet of the data I could test upon?

[siddharthlatest]

@bstst I didn't set a parent-child relationship, neither did I set routing path with the mappings. I only tried to make an index request with a parent present in the query string and it complained that I can't specify parent if no parent field has been configured.

[bstst]

@siddharthlatest, exactly — my change will only add parent to the query if it exists in the mapping routing. and since v6 works differently, my change does not affect the way dejavu works with v6.

[siddharthlatest]

@bstst I am requesting an additional condition to ensure that the parent querystring does get applied only when _parent object is present within the type's mapping.

[siddharthlatest]

@bstst Here, we should also add a check for the presence of this.props._mapping._parent to verify that the document in the type being indexed indeed has a parent mapping.

[siddharthlatest]

@bstst I missed this earlier, but it seems ES has completely removed support for adding path inside _routing starting v2. See elastic/elasticsearch#6730 (comment).

And also this https://stackoverflow.com/questions/36123647/elasticsearch-routing-on-specific-field.

In which version of ES are you able to set the _routing.path?

I still want to merge this, so perhaps we can add a check to ensure that _routing.path is present.

[siddharthlatest]

Thank you for contributing @bstst! 🎉 These changes will be out in the next minor release.