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.

Sign up for GitHub

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

Jump to bottom

Give precedence to index creation when mixing typed templates with typeless index creation and vice-versa. #37871

Merged
merged 8 commits into from
Jan 30, 2019
Merged

Give precedence to index creation when mixing typed templates with typeless index creation and vice-versa. #37871

merged 8 commits into from
Jan 30, 2019

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Jan 25, 2019 •
edited
Loading

Currently if you mix typed templates and typeless index creation or typeless
templates and typed index creation then you will end up with an error because
Elasticsearch tries to create an index that has multiple types: _doc and
the explicit type name that you used.

This commit proposes to give precedence to the index creation call so that
the type from the template will be ignored if the index creation call is
typeless while the template is typed, and the type from the index creation
call will be used if there is a typeless template.

This is consistent with the fact that index creation already "wins" if a field
is defined differently in the index creation call and in a template: the
definition from the index creation call is used in such cases.

Closes #37773

@jpountz
Give precedence to index creation when mixing typed templates with ty…
69f0b1e 
…peless index creation and vice-versa.

Currently if you mix typed templates and typeless index creation or typeless
templates and typed index creation then you will end up with an error because
Elasticsearch tries to create an index that has multiple types: `_doc` and
the explicit type name that you used.

This commit proposes to give precedence to the index creation call so that
the type from the template will be ignored if the index creation call is
typeless while the template is typed, and the type from the index creation
call will be used if there is a typeless template.

This is consistent with the fact that index creation already "wins" if a field
is defined differently in the index creation call and in a template: the
definition from the index creation call is used in such cases.

Closes elastic#37773
@jpountz jpountz added >feature :Search Foundations/Mapping Index mappings, including merging and defining field types v7.0.0 v6.7.0 labels Jan 25, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@jpountz jpountz changed the title Give precedence to index creation when mixing typed templates with ty… Give precedence to index creation when mixing typed templates with typeless index creation and vice-versa. Jan 25, 2019
@jpountz jpountz requested a review from jtibshirani January 25, 2019 15:31
@jpountz
Copy link
Contributor Author

jpountz commented Jan 28, 2019

@elasticmachine run elasticsearch-ci/2, run elasticsearch-ci/oss-distro-docs

jtibshirani
jtibshirani reviewed Jan 29, 2019
View reviewed changes
Copy link
Contributor

@jtibshirani jtibshirani left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks a lot @jpountz for this PR!

I originally had a different intuition for the behavior: I expected that given a template with a custom type, and a typeless create index request, the final index would contain the custom type. This is because I think of create index requests with no mappings as being 'typeless', and these requests won't influence the type of the final index. The overall principle is something like 'a custom type always wins out over having no type (including the dummy _doc type)', because it indicates a desire to break with the default behavior.

However, I can see the benefits of this approach -- it fits with the overall idea of the index request 'winning out', and users have more flexibility to convert each index creation to a typeless one. In any case, since the right behavior is not obvious, I think it would be worth updating the templates documentation with an explanation.

rest-api-spec/src/main/resources/rest-api-spec/test/indices.create/20_mix_typeless_typeful.yml
@@ -0,0 +1,77 @@
---
"Create a typeless index while there is a typed template":
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the change is a bit subtle, I wonder if it'd be worth adding some unit tests in IndexCreationTaskTests? In particular, reading the code I had to think a bit about what would happen if no mappings were provided in the create index requests, or if the template or create request had more than one type.

jtibshirani
jtibshirani approved these changes Jan 29, 2019
View reviewed changes
docs/reference/mapping/removal_of_types.asciidoc

In case of implicit index creation, because of documents that get indexed in
an index that doesn't exist yet, the template is always honored. This is
usually not a problem due to the fact that typless index calls work on typed
Copy link
Contributor

Choose a reason for hiding this comment

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

Small typo (typless) but it's so hard to get a green build, I can just fix this when we do another pass over this documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will open a follow-up PR.

@jtibshirani jtibshirani mentioned this pull request Jan 29, 2019
Closed
@jpountz jpountz merged commit b63b50b into elastic:master Jan 30, 2019
@jpountz jpountz deleted the mix_typed_typeless_template_index_creation branch January 30, 2019 09:28
jpountz added a commit to jpountz/elasticsearch that referenced this pull request Jan 30, 2019
@jpountz
Fix typo in docs.
de15a7f 
This has been introduced in elastic#37871.
@jpountz jpountz mentioned this pull request Jan 30, 2019
Merged
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 30, 2019
@jasontedor
Merge branch 'master' into retention-lease-stats
ad66152 
* master: (29 commits)
  Fix limit on retaining sequence number (elastic#37992)
  Docs test fix, wait for shards active.
  Revert "Revert "Documented default values for index follow request parameters. (elastic#37917)""
  Revert "Documented default values for index follow request parameters. (elastic#37917)"
  Ensure date parsing BWC compatibility (elastic#37929)
  SQL: Skip the nested and object field types in case of an ODBC request (elastic#37948)
  Use mappings to format doc-value fields by default. (elastic#30831)
  Give precedence to index creation when mixing typed templates with typeless index creation and vice-versa. (elastic#37871)
  Add classifier to tar.gz in docker compose (elastic#38011)
  Documented default values for index follow request parameters. (elastic#37917)
  Fix fetch source option in expand search phase (elastic#37908)
  Restore a noop _all metadata field for 6x indices (elastic#37808)
  Added ccr to xpack usage infrastructure (elastic#37256)
  Fix exit code for Security CLI tools  (elastic#37956)
  Streamline S3 Repository- and Client-Settings (elastic#37393)
  Add version 6.6.1 (elastic#37975)
  Ensure task metadata not null in follow test (elastic#37993)
  Docs fix - missing callout
  Types removal - deprecate include_type_name with index templates (elastic#37484)
  Handle completion suggestion without contexts
  ...
jpountz added a commit to jpountz/elasticsearch that referenced this pull request Jan 30, 2019
@jpountz
Give precedence to index creation when mixing typed templates with ty…
0abce95 
…peless index creation and vice-versa.

This is a backport of elastic#37871 and elastic#38032.
@jpountz jpountz mentioned this pull request Jan 30, 2019
Merged
jpountz added a commit that referenced this pull request Jan 31, 2019
@jpountz
Fix typo in docs. (#38018)
3332e33 
This has been introduced in #37871.
jpountz added a commit that referenced this pull request Feb 1, 2019
@jpountz
Give precedence to index creation when mixing typed templates with ty…
81a6c1d 
…peless index creation and vice-versa. (#38055)

This is a backport of #37871 and #38032.
jpountz added a commit to jpountz/elasticsearch that referenced this pull request Feb 1, 2019
@jpountz
Enable bw tests for elastic#37871 and elastic#38032.
c254a6c 
Mixed-version clusters tests had been disabled initially since they wouldn't
work until the functionality would be backported.
jpountz added a commit that referenced this pull request Feb 1, 2019
@jpountz
Enable bw tests for #37871 and #38032. (#38167)
2229e72 
Mixed-version clusters tests had been disabled initially since they wouldn't
work until the functionality would be backported.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Feb 1, 2019
@jasontedor
Merge branch 'master' into retention-leases-version
7cf145c 
* master: (36 commits)
  Ensure joda compatibility in custom date formats (elastic#38171)
  Do not compute cardinality if the `terms` execution mode does not use `global_ordinals` (elastic#38169)
  Do not set timeout for IndexRequests in GatewayIndexStateIT (elastic#38147)
  Zen2ify testMasterFailoverDuringIndexingWithMappingChanges (elastic#38178)
  SQL: [Docs] Add limitation for aggregate functions on scalars (elastic#38186)
  Add elasticsearch-node detach-cluster command (elastic#37979)
  Add tests for fractional epoch parsing (elastic#38162)
  Enable bw tests for elastic#37871 and elastic#38032. (elastic#38167)
  Clear send behavior rule in CloseWhileRelocatingShardsIT (elastic#38159)
  Fix testCorruptedIndex (elastic#38161)
  Add finalReduce flag to SearchRequest (elastic#38104)
  Forbid negative field boosts in analyzed queries (elastic#37930)
  Remove AtomiFieldData#getLegacyFieldValues (elastic#38087)
  Universal cluster bootstrap method for tests with autoMinMasterNodes=false (elastic#38038)
  Fix FullClusterRestartIT.testHistoryUUIDIsAdded (elastic#38098)
  Replace joda time in ingest-common module (elastic#38088)
  Fix eclipse config for ssl-config (elastic#38096)
  Don't load global ordinals with the `map` execution_hint (elastic#37833)
  Relax fault detector in some disruption tests (elastic#38101)
  Fix java time epoch date formatters (elastic#37829)
  ...
@dnhatn dnhatn mentioned this pull request Feb 1, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtibshirani jtibshirani jtibshirani approved these changes

Assignees
No one assigned
Labels
>feature :Search Foundations/Mapping Index mappings, including merging and defining field types v6.7.0 v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jpountz @elasticmachine @jtibshirani @colings86 @danielmitterdorfer