Skip to content

Add documentation for JSON fields. #35281

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
Merged
merged 7 commits into from
Nov 9, 2018
Merged

Add documentation for JSON fields. #35281

merged 7 commits into from
Nov 9, 2018

Conversation

@jtibshirani
Copy link
Contributor

@jtibshirani jtibshirani commented Nov 6, 2018

No description provided.

@jtibshirani jtibshirani added >docs General docs changes WIP :Search Foundations/Mapping Index mappings, including merging and defining field types labels Nov 6, 2018
@elasticmachine
Copy link
Collaborator

elasticmachine commented Nov 6, 2018

Pinging @elastic/es-search-aggs

@jtibshirani jtibshirani force-pushed the json-field-docs branch 4 times, most recently from d4184d2 to 1d79ad2 Compare November 6, 2018 07:31
colings86
colings86 reviewed Nov 6, 2018
View reviewed changes
Copy link
Contributor

@colings86 colings86 left a comment

Choose a reason for hiding this comment

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

I left a couple of comments. Thanks for writing this up, its really useful to have documentation early on

docs/reference/mapping/types/json.asciidoc Outdated
Copy link
Contributor

@colings86 colings86 Nov 6, 2018

Choose a reason for hiding this comment

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

I think it would be useful to explain a bit more about the trade offs of using this vs explicitly mapping the object. For example one advantage of this is that we expect the resulting index to be smaller with the disadvantage of limited query support and no support for aggregations and sorting.

Copy link
Contributor Author

@jtibshirani jtibshirani Nov 6, 2018

Choose a reason for hiding this comment

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

Makes a lot of sense, I'll expand this section.

docs/reference/mapping/types/json.asciidoc Outdated
Copy link
Contributor

@colings86 colings86 Nov 6, 2018

Choose a reason for hiding this comment

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

Maybe we should just state that the default is to not skip values of any length rather than mentioning the explicit value here?

Copy link
Contributor Author

@jtibshirani jtibshirani Nov 6, 2018

Choose a reason for hiding this comment

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

Will fix, I copied this over from the keyword documentation but reading it over again it is quite strange!

@jtibshirani jtibshirani mentioned this pull request Nov 6, 2018
Closed
11 tasks
@jtibshirani
Copy link
Contributor Author

jtibshirani commented Nov 8, 2018

@colings86 I think this is ready for another look.

@jtibshirani jtibshirani removed the WIP label Nov 8, 2018
colings86
colings86 approved these changes Nov 8, 2018
View reviewed changes
Copy link
Contributor

@colings86 colings86 left a comment

Choose a reason for hiding this comment

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

LGTM but could you make sure all lines are wrapped to 80 characters before merging?

nik9000
nik9000 reviewed Nov 8, 2018
View reviewed changes
docs/reference/mapping/types/json.asciidoc Outdated
Copy link
Member

@nik9000 nik9000 Nov 8, 2018

Choose a reason for hiding this comment

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

It might be better to mark this // TESTSETUP then you don't need to mark the others // TEST[continued].

Copy link
Contributor Author

@jtibshirani jtibshirani Nov 8, 2018

Choose a reason for hiding this comment

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

👍

@jtibshirani
Copy link
Contributor Author

jtibshirani commented Nov 8, 2018

@elasticmachine run sample packaging tests

@jtibshirani
Copy link
Contributor Author

jtibshirani commented Nov 8, 2018

@elasticmachine run gradle build tests

@jtibshirani jtibshirani merged commit ffe70cb into elastic:object-fields Nov 9, 2018
@jtibshirani jtibshirani deleted the json-field-docs branch November 9, 2018 00:56
jtibshirani added a commit to jtibshirani/elasticsearch that referenced this pull request Nov 9, 2018
@jtibshirani
Add documentation for JSON fields. (elastic#35281)
a90bb64 
* Add documentation for JSON fields.
jtibshirani added a commit that referenced this pull request Nov 30, 2018
@jtibshirani
Add documentation for JSON fields. (#35281)
9315a4a 
* Add documentation for JSON fields.
jtibshirani added a commit that referenced this pull request Dec 5, 2018
@jtibshirani
Add documentation for JSON fields. (#35281)
87134c3 
* Add documentation for JSON fields.
jtibshirani added a commit that referenced this pull request Dec 13, 2018
@jtibshirani
Add documentation for JSON fields. (#35281)
9ca3aba 
* Add documentation for JSON fields.
jtibshirani added a commit that referenced this pull request Mar 6, 2019
@jtibshirani
Add documentation for JSON fields. (#35281)
a47c7cd 
* Add documentation for JSON fields.
jtibshirani added a commit that referenced this pull request Mar 7, 2019
@jtibshirani
Add documentation for JSON fields. (#35281)
686f72b 
* Add documentation for JSON fields.
jtibshirani added a commit that referenced this pull request Mar 9, 2019
@jtibshirani
Add documentation for JSON fields. (#35281)
2fcb5ba 
* Add documentation for JSON fields.
jtibshirani added a commit that referenced this pull request Mar 11, 2019
@jtibshirani
Add documentation for JSON fields. (#35281)
fe5ada6 
* Add documentation for JSON fields.
jtibshirani added a commit that referenced this pull request Mar 14, 2019
@jtibshirani
Add documentation for JSON fields. (#35281)
df75c6e 
* Add documentation for JSON fields.
jtibshirani added a commit that referenced this pull request Mar 19, 2019
@jtibshirani
Add documentation for JSON fields. (#35281)
9fb427d 
* Add documentation for JSON fields.
jtibshirani added a commit to jtibshirani/elasticsearch that referenced this pull request Mar 19, 2019
@jtibshirani
Add documentation for JSON fields. (elastic#35281)
5deac67 
* Add documentation for JSON fields.
jtibshirani added a commit that referenced this pull request Mar 28, 2019
@jtibshirani
Add documentation for JSON fields. (#35281)
8041e23 
* Add documentation for JSON fields.
jtibshirani added a commit that referenced this pull request Apr 10, 2019
@jtibshirani
Add documentation for JSON fields. (#35281)
d76b94d 
* Add documentation for JSON fields.
jtibshirani added a commit that referenced this pull request Apr 17, 2019
@jtibshirani
Add documentation for JSON fields. (#35281)
7a217bd 
* Add documentation for JSON fields.
jtibshirani added a commit that referenced this pull request May 1, 2019
@jtibshirani
Add documentation for JSON fields. (#35281)
9e28ed6 
* Add documentation for JSON fields.
jtibshirani added a commit that referenced this pull request May 24, 2019
@jtibshirani
Add documentation for JSON fields. (#35281)
80b59eb 
* Add documentation for JSON fields.
jtibshirani added a commit that referenced this pull request May 29, 2019
@jtibshirani
Add documentation for JSON fields. (#35281)
fbadb62 
* Add documentation for JSON fields.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nik9000 nik9000 nik9000 left review comments

@colings86 colings86 colings86 approved these changes

Assignees

No one assigned

Labels

>docs General docs changes :Search Foundations/Mapping Index mappings, including merging and defining field types

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jtibshirani @elasticmachine @nik9000 @colings86