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

The parent option on update request should be used for upsert only. #9612

Merged
merged 2 commits into from Mar 28, 2015

Conversation

Projects
None yet
6 participants
@martijnvg
Member

martijnvg commented Feb 9, 2015

PR for #4538 and consists out of two parts:

  • Revert of the removal of the parent option on update
  • Let the parent option on the update request to be used only for upsert requests.
@@ -634,6 +652,7 @@ public void readFrom(StreamInput in) throws IOException {
type = in.readString();
id = in.readString();
routing = in.readOptionalString();
parent = in.readOptionalString();

This comment has been minimized.

@jpountz

jpountz Feb 10, 2015

Contributor

Remember to add version checks on 1.x ?

@jpountz

jpountz Feb 10, 2015

Contributor

Remember to add version checks on 1.x ?

This comment has been minimized.

@martijnvg

martijnvg Feb 10, 2015

Member

I like to keep this change in 2.0 only

@martijnvg

martijnvg Feb 10, 2015

Member

I like to keep this change in 2.0 only

This comment has been minimized.

@martijnvg

martijnvg Feb 27, 2015

Member

When backporting it to 1.x I'll add the version checks.

@martijnvg

martijnvg Feb 27, 2015

Member

When backporting it to 1.x I'll add the version checks.

@jpountz

View changes

Show outdated Hide outdated src/main/java/org/elasticsearch/rest/action/update/RestUpdateAction.java
@jpountz

View changes

Show outdated Hide outdated docs/reference/docs/update.asciidoc
@jpountz

This comment has been minimized.

Show comment
Hide comment
@jpountz

jpountz Feb 10, 2015

Contributor

@martijnvg left some minor comments

Contributor

jpountz commented Feb 10, 2015

@martijnvg left some minor comments

@martijnvg

This comment has been minimized.

Show comment
Hide comment
@martijnvg

martijnvg Feb 10, 2015

Member

@jpountz I've updated the PR.

Member

martijnvg commented Feb 10, 2015

@jpountz I've updated the PR.

@rjernst

View changes

Show outdated Hide outdated docs/reference/docs/update.asciidoc
@rjernst

This comment has been minimized.

Show comment
Hide comment
@rjernst

rjernst Feb 11, 2015

Member

@martijnvg I just have one comment about the docs. I'm also wondering what the behavior should be if they set both routing and parent in the same request? To me this would indicate they are confused, and maybe we should error?

Member

rjernst commented Feb 11, 2015

@martijnvg I just have one comment about the docs. I'm also wondering what the behavior should be if they set both routing and parent in the same request? To me this would indicate they are confused, and maybe we should error?

@martijnvg

This comment has been minimized.

Show comment
Hide comment
@martijnvg

martijnvg Feb 12, 2015

Member

@rjernst Thanks, setting and routing and parent is common when dealing with multi layered parent child mappings. For example the in case of a grandparent > parent > child relationship, when indexing the child doc the routing is set to the grandparent id and the parent is set to the parent id. This will ensure that direct and indirect children of grandparents reside on the same shard.

Member

martijnvg commented Feb 12, 2015

@rjernst Thanks, setting and routing and parent is common when dealing with multi layered parent child mappings. For example the in case of a grandparent > parent > child relationship, when indexing the child doc the routing is set to the grandparent id and the parent is set to the parent id. This will ensure that direct and indirect children of grandparents reside on the same shard.

@martijnvg

This comment has been minimized.

Show comment
Hide comment
@martijnvg

martijnvg Feb 12, 2015

Member

Updated the PR with changes to the docs.

Member

martijnvg commented Feb 12, 2015

Updated the PR with changes to the docs.

@lukens

View changes

Show outdated Hide outdated src/main/java/org/elasticsearch/action/update/UpdateHelper.java

@s1monw s1monw added v1.6.0 and removed v1.6.0 v1.5.0 labels Mar 17, 2015

@s1monw

This comment has been minimized.

Show comment
Hide comment
@s1monw

s1monw Mar 24, 2015

Contributor

@rjernst @jpountz @martijnvg are you guys picking this up?

Contributor

s1monw commented Mar 24, 2015

@rjernst @jpountz @martijnvg are you guys picking this up?

@rjernst

View changes

Show outdated Hide outdated rest-api-spec/api/update.json
@rjernst

This comment has been minimized.

Show comment
Hide comment
@rjernst

rjernst Mar 27, 2015

Member

LGTM, just a couple grammar comments on the rest spec docs.

Member

rjernst commented Mar 27, 2015

LGTM, just a couple grammar comments on the rest spec docs.

@martijnvg martijnvg removed the review label Mar 28, 2015

@martijnvg martijnvg merged commit 6d1a1b3 into elastic:master Mar 28, 2015

1 check passed

CLA Commit author is a member of Elasticsearch
Details

clintongormley added a commit that referenced this pull request Apr 13, 2015

@martijnvg martijnvg deleted the martijnvg:update/parent_option branch May 18, 2015

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