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

Consolidate document parsing logic #10802

Merged
merged 1 commit into from Apr 28, 2015

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Apr 25, 2015

The code to parse a document was spread across 3 different classes,
and depended on traversing the ObjectMapper hiearchy. This change
consolidates all the doc parsing code into a new DocumentParser.
This should allow adding unit tests (future issue) for document
parsing so the logic can be simplified. All code was copied
directly for this change with only minor modifications to make
it work within the new location.

@rjernst rjernst added v2.0.0-beta1 :Search/Mapping Index mappings, including merging and defining field types labels Apr 25, 2015
@jpountz
Copy link
Contributor

jpountz commented Apr 25, 2015

This is great! LGTM

Also +1 on handling multi-fields like copy_to

rjernst added a commit to rjernst/elasticsearch that referenced this pull request Apr 28, 2015
The code to parse a document was spread across 3 different classes,
and depended on traversing the ObjectMapper hiearchy. This change
consolidates all the doc parsing code into a new DocumentParser.
This should allow adding unit tests (future issue) for document
parsing so the logic can be simplified.  All code was copied
directly for this change with only minor modifications to make
it work within the new location.

closes elastic#10802
The code to parse a document was spread across 3 different classes,
and depended on traversing the ObjectMapper hiearchy. This change
consolidates all the doc parsing code into a new DocumentParser.
This should allow adding unit tests (future issue) for document
parsing so the logic can be simplified.  All code was copied
directly for this change with only minor modifications to make
it work within the new location.

closes elastic#10802
@rjernst rjernst merged commit 99584de into elastic:master Apr 28, 2015
@clintongormley clintongormley changed the title Mappings: Consolidate document parsing logic Consolidate document parsing logic Jun 8, 2015
brwe added a commit to brwe/elasticsearch that referenced this pull request Dec 3, 2015
Copy to within multi field is ignored from 2.0 on, see elastic#10802.
Instead of just ignoring it, we should throw an exception if this
is found in the mapping when a mapping is added. For already
existing indices we should at least log a warning.

related to elastic#14946
brwe added a commit that referenced this pull request Dec 8, 2015
throw exception if a copy_to is within a multi field

Copy to within multi field is ignored from 2.0 on, see #10802.
Instead of just ignoring it, we should throw an exception if this
is found in the mapping when a mapping is added. For already
existing indices we should at least log a warning.

related to #14946
brwe added a commit that referenced this pull request Dec 8, 2015
throw exception if a copy_to is within a multi field

Copy to within multi field is ignored from 2.0 on, see #10802.
Instead of just ignoring it, we should throw an exception if this
is found in the mapping when a mapping is added. For already
existing indices we should at least log a warning.

related to #14946
@rjernst rjernst deleted the pr/extract-doc-parser branch September 18, 2020 03:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search/Mapping Index mappings, including merging and defining field types v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants