Skip to content

Conversation

@vladimirdolzhenko
Copy link
Contributor

@vladimirdolzhenko vladimirdolzhenko commented Nov 5, 2018

UpdateRequest in 6.x could have doc, script and other valuable commands on any level of depth.

All of following requests
{"doc":{"message":"set by update:doc"}}
{"whatever":{"doc":{"message":"set by update:doc"}}}
{"aaa":{"whatever":{"doc":{"message":"set by update:doc"}}}}

are like {"doc":{"message":"set by update:doc"}} as all unknown fields ignored but parsing continues to look up for commands in depth.

Actually parsing of nested object is broken and it leads to issues like #34069.

This PR fixes parsing of nested objects for UpdateRequest for 6.x branch.

#29293 is a proper fix for 7.0 (it forbids such kind of behaviour) but it is breaking change that we could not apply for 6.x branch.

Closes #34069

@vladimirdolzhenko vladimirdolzhenko added :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v6.6.0 labels Nov 5, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Crazy that we allow this, but we don't want to break it in a minor. And what you've got makes 6.x better. Crazy. But better.

Do you know if we have a breaking change warning in master for no longer allowing this? I don't think we realized fully what we were fixing when we did this. If we don't have one, could you add one?

@vladimirdolzhenko
Copy link
Contributor Author

vladimirdolzhenko commented Nov 5, 2018

@nik9000 Deprecation message has to be added in 6.x as master already dropped support for fields and unknown fields (it throws XContentParseException for any unknown fields ). Could you pls have another look ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v6.6.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants