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

[DOCS] Reorg field data types page #61117

Merged
merged 13 commits into from
Aug 26, 2020
Merged

[DOCS] Reorg field data types page #61117

merged 13 commits into from
Aug 26, 2020

Conversation

jrodewig
Copy link
Contributor

@jrodewig jrodewig commented Aug 13, 2020

Reorganizes the field data types page based on use case.

Also incorporates type families and standardizes usage.

Relates to #57548

Preview

https://elasticsearch_61117.docs-preview.app.elstc.co/guide/en/elasticsearch/reference/master/mapping-types.html

@jrodewig jrodewig marked this pull request as ready for review August 18, 2020 16:43
@jrodewig jrodewig added :Search Foundations/Mapping Index mappings, including merging and defining field types >docs General docs changes v7.10.0 v7.9.1 v8.0.0 labels Aug 18, 2020
@elasticmachine
Copy link
Collaborator

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

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (>docs)

@elasticmachine elasticmachine added Team:Search Meta label for search team Team:Docs Meta label for docs team labels Aug 18, 2020
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.

This looks like a nice start! I left a couple preliminary comments.

docs/reference/mapping/types.asciidoc Outdated Show resolved Hide resolved
docs/reference/mapping/types.asciidoc Outdated Show resolved Hide resolved
The following sections organize the available field types based on their
intended use. Except where noted, the type family is the same as the field type.

<<binary,`binary`>>::
Copy link
Contributor

Choose a reason for hiding this comment

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

For me this organization is a bit unclear. Some types like binary and boolean don't belong to an overall section. And the sections don't relate to a consistent concept -- some describe use cases like 'full text search' while others are broad type categories like 'dates'.

It's actually quite challenging to come up with a good grouping! I'll reach out to you through another channel with a brainstorming document.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome! Thanks!

@jrodewig jrodewig requested a review from giladgal August 20, 2020 23:15
@jrodewig
Copy link
Contributor Author

@jtibshirani @markharwood @giladgal

I updated the categorization of the data types based on feedback from the other channel. Please take a look at the preview and leave any additional feedback you have. All feedback welcome!


<<binary>>:: `binary`
<<boolean>>:: `boolean`
keyword:: <<keyword,`keyword`>>,
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 out of scope for the current PR, but seeing this made me think that each 'field type family' should live on a single sub-page. We could have a short description of the family at the top, and then a subsection for each type. This would help users directly compare the different options, and could make wildcard more discoverable.

@jrodewig @markharwood what would you think about this idea as a potential follow-up?

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 like this idea. I've created #61441 to track.

docs/reference/mapping/types.asciidoc Outdated Show resolved Hide resolved
docs/reference/mapping/types.asciidoc Outdated Show resolved Hide resolved
docs/reference/mapping/types.asciidoc Outdated Show resolved Hide resolved
docs/reference/mapping/types.asciidoc Outdated Show resolved Hide resolved
docs/reference/mapping/types.asciidoc Outdated Show resolved Hide resolved
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.

This looks good to me. I understand if you'd like to wait to merge to give others some time to weigh in though.

docs/reference/mapping/types.asciidoc Outdated Show resolved Hide resolved
docs/reference/mapping/types.asciidoc Outdated Show resolved Hide resolved
[[object-types]]
==== Objects and relational types

<<object,`object`>>:: A single JSON object.
Copy link
Contributor

Choose a reason for hiding this comment

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

I just noticed that the existing description of object + nested is quite confusing. Suggested update:

  • object: A JSON object.
  • nested: A JSON object that preserves the relationship between its subfields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is much clearer. Thanks for the suggestion.

@jrodewig
Copy link
Contributor Author

jrodewig commented Aug 21, 2020

Thank you so much for your reviews and the work on this reorg @jtibshirani. You did most of the heavy lifting!🙇

I'll leave this PR open for a few more days to get any additional feedback.

@markharwood
Copy link
Contributor

markharwood commented Aug 24, 2020

I wonder if "quantifiers" might be a useful word to help describe the purpose of our numerics? A JSON long shouldn't be mapped as an elasticsearch long field if the data is something like a customer ID - that's better mapped as keyword. Our numeric indexing fields are optimised for range queries and range queries are used on:

  • weights
  • sizes
  • counts
  • money etc

... all quantities of something.
In a previous life/product we talked about quantitifiers vs identifiers to clearly distinguish the two use cases for numeric fields.

@jrodewig
Copy link
Contributor Author

I wonder if "quantifiers" might be a useful word to help describe the purpose of our numerics?

I've added this to the Numbers description with e672593.

I think relabelling "Numbers" as "Quantifiers" entirely may simply obfuscate things rather than help users trying to map IDs. I think that topic has a bit more nuance than can probably be captured here (e.g., "Do you need to do math on these IDs?", "Do you need to search the IDs based on a range?", "Do you frequently sort or filter on this ID?", etc.).

@markharwood
Copy link
Contributor

, "Do you need to do math on these IDs?",

Agreed, it's not always 100% clear-cut but generally range queries, sorting and math apply to amounts more than unique IDs. Exceptions might be IDs like order numbers that increment.

I've added this to the Numbers description with e672593.
Quantifiers and numeric types, such as long

That sounds a little like "quantifiers" and "numeric types" are two different things. How about

 "Numeric types such as long etc when used to express amounts (rather than IDs)."

@jrodewig
Copy link
Contributor Author

Thanks for the suggestion @markharwood! I've added a slightly modified version to the docs.

Keyword:: The keyword family, including <<keyword, `keyword`>>,
<<constant-keyword,`constant_keyword`>>, and
<<wildcard, `wildcard`>>.
<<number,Numbers>>:: Numeric types, such as `long` and `double`, used to
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for nitpicking -- the 'amounts' part is really helpful, but the 'rather than IDs' part feels confusing, and as @markharwood mentioned there are actually some use cases for mapping ordered IDs as numbers. We could remove 'rather than IDs' (since we already have a warning/ tip about it in the numeric types docs)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem. Removed with c4beb42. Thanks for raising.

@jrodewig
Copy link
Contributor Author

@elasticmachine update branch

@jrodewig jrodewig merged commit 49350dd into elastic:master Aug 26, 2020
@jrodewig
Copy link
Contributor Author

Went ahead and merged this in. Please feel free to reach out if you have any further iteration. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes :Search Foundations/Mapping Index mappings, including merging and defining field types Team:Docs Meta label for docs team Team:Search Meta label for search team v7.9.1 v7.10.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants