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

Allow _doc as a type. #27816

Merged
merged 3 commits into from
Dec 14, 2017
Merged

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Dec 14, 2017

Allowing _doc as a type will enable users to make the transition to 7.0
smoother since the index APIs will be PUT index/_doc/id and POST index/_doc.
This also moves same tests and most of the documentation to _doc as a type name.

Closes #27750
Closes #27751

Allowing `_doc` as a type will enable users to make the transition to 7.0
smoother since the index APIs will be `PUT index/_doc/id` and `POST index/_doc`.
This also moves most of the documentation to `_doc` as a type name.

Closes elastic#27750
Closes elastic#27751
@jpountz jpountz added :Search Foundations/Mapping Index mappings, including merging and defining field types >enhancement labels Dec 14, 2017
Copy link
Contributor

@clintongormley clintongormley left a comment

Choose a reason for hiding this comment

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

I couldn't check all 196(!) files, but it looks like you've got most things, thank you.

Added some comments about the removal of types page.

@@ -122,7 +122,7 @@ PUT twitter
"email": { "type": "keyword" }
}
},
"tweet": {
"_doc": {
Copy link
Contributor

Choose a reason for hiding this comment

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

this one shouldn't change

@@ -187,7 +187,7 @@ PUT twitter/doc/user-kimchy

Copy link
Contributor

Choose a reason for hiding this comment

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

doc in lines 167 and 180 could become _doc

Copy link
Contributor

Choose a reason for hiding this comment

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

Line 167 still needs changing

@@ -187,7 +187,7 @@ PUT twitter/doc/user-kimchy

PUT twitter/doc/tweet-1
{
"type": "tweet", <1>
"type": "_doc", <1>
Copy link
Contributor

Choose a reason for hiding this comment

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

this one shouldn't change

@@ -204,7 +204,7 @@ GET twitter/_search
},
"filter": {
"match": {
"type": "tweet" <1>
"type": "_doc" <1>
Copy link
Contributor

Choose a reason for hiding this comment

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

this one shouldn't change

@@ -320,7 +320,7 @@ PUT tweets
"index.mapping.single_type": true
},
"mappings": {
Copy link
Contributor

Choose a reason for hiding this comment

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

In line 242 above, under changes coming in 6.x, mention addition of _doc as an acceptable type name to ease the transition to new URLs

@@ -351,7 +351,7 @@ POST _reindex
{
"source": {
"index": "twitter",
"type": "tweet"
"type": "_doc"
Copy link
Contributor

Choose a reason for hiding this comment

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

this is the source, so the type name should remain

@@ -351,7 +351,7 @@ POST _reindex
{
"source": {
"index": "twitter",
"type": "tweet"
"type": "_doc"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

in lines 375 and 413 below, use _doc instead of doc

@jpountz
Copy link
Contributor Author

jpountz commented Dec 14, 2017

@clintongormley done

Copy link
Contributor

@clintongormley clintongormley left a comment

Choose a reason for hiding this comment

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

One missed change and a couple of suggestions, otherwise LGTM

@@ -187,7 +187,7 @@ PUT twitter/doc/user-kimchy

Copy link
Contributor

Choose a reason for hiding this comment

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

Line 167 still needs changing

can be used for the type, but there can be only one.
can be used for the type, but there can be only one. The preferred type name
is `_doc`, so that index APIs have the same path as they will have in 7.0:
`PUT index/_doc/id` and `POST index/_doc`
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps PUT {index}/_doc/{id} instead?

a document no longer requires a document `type`.
a document no longer requires a document `type`. The new index APIs
are `PUT index/_doc/id` in case of explicit ids and `POST index/_doc`
for auto-generated ids.
Copy link
Contributor

Choose a reason for hiding this comment

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

same here. perhaps PUT {index}/_doc/{id} instead?

@jpountz jpountz merged commit 1b66082 into elastic:master Dec 14, 2017
@jpountz jpountz deleted the enhancement/_doc_as_a_type branch December 14, 2017 16:49
jpountz added a commit that referenced this pull request Dec 19, 2017
Allowing `_doc` as a type will enable users to make the transition to 7.0
smoother since the index APIs will be `PUT index/_doc/id` and `POST index/_doc`.
This also moves most of the documentation to `_doc` as a type name.

Closes #27750
Closes #27751
jpountz added a commit to jpountz/elasticsearch that referenced this pull request Feb 1, 2019
The test assumes that all 6.x indices support `_doc` as a type name but support
for it was actually only added in 6.2 via elastic#27816.
jpountz added a commit to jpountz/elasticsearch that referenced this pull request Feb 18, 2019
The test assumed that it could run against all 6.x indices but it actually
requires 6.2+ since 6.2 is the first version that allowed `_doc` as a type
name (elastic#27816).

Closes elastic#38202
jpountz added a commit that referenced this pull request Feb 20, 2019
…`. (#39054)

The test assumed that it could run against all 6.x indices but it actually
requires 6.2+ since 6.2 is the first version that allowed `_doc` as a type
name (#27816).

Closes #38202
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types v6.2.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants