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

Fix updating include_in_parent/include_in_root of nested field. #54386

Merged
merged 7 commits into from Apr 16, 2020

Conversation

gaobinlong
Copy link
Contributor

Closes #53792

The main changes are:

  1. Throw an error when updating include_in_parent or include_in_root attribute of nested field dynamically by the PUT mapping API.
  2. Add a test for the change.

@AndyHunt66 AndyHunt66 added the :Data Management/Indices APIs APIs to create and manage indices and templates label Mar 30, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Indices APIs)

@dakrone dakrone added :Search/Mapping Index mappings, including merging and defining field types and removed :Data Management/Indices APIs APIs to create and manage indices and templates labels Mar 30, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Mapping)

@jtibshirani jtibshirani self-requested a review March 30, 2020 17:16
Copy link
Contributor

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

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

Thanks @gaobinlong for the contribution! I left a couple comments.

Copy link
Contributor

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

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

The new structure looks good, I have one last comment about the test. I'll kick off CI to check that all tests pass.

@jtibshirani
Copy link
Contributor

@elasticmachine test this please

@gaobinlong
Copy link
Contributor Author

@jtibshirani, I have pushed a new commit, can you take a look at the change?

Copy link
Contributor

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

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

One last comment, thanks for your patience on this review.

@jtibshirani
Copy link
Contributor

@elasticmachine test this please

Copy link
Contributor

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

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

Looks good to me! I will merge and backport to 7.x, along with a note in the migration docs.

@jtibshirani jtibshirani merged commit 5c66caf into elastic:master Apr 16, 2020
jtibshirani pushed a commit to jtibshirani/elasticsearch that referenced this pull request Apr 16, 2020
elastic#54386)

The main changes are:
1. Throw an error when updating `include_in_parent` or `include_in_root` attribute of nested field dynamically by the PUT mapping API.
2. Add a test for the change.

Closes elastic#53792
yyff pushed a commit to yyff/elasticsearch that referenced this pull request Apr 17, 2020
elastic#54386)

The main changes are:
1. Throw an error when updating `include_in_parent` or `include_in_root` attribute of nested field dynamically by the PUT mapping API.
2. Add a test for the change.

Closes elastic#53792
@jtibshirani jtibshirani changed the title Fix updating include_in_parent/include_in_root of nested field throws… Fix updating include_in_parent/include_in_root of nested field. Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>breaking >bug :Search/Mapping Index mappings, including merging and defining field types v7.8.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change in index_in_parent doesn't work, but throws no error
6 participants