LUCENE-8594: DV update are broken for updates on new field #516
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
this was found in elastic/elasticsearch#36286 |
jpountz
approved these changes
Dec 6, 2018
lucene/backward-codecs/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java
Show resolved
Hide resolved
lucene/backward-codecs/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java
Outdated
Show resolved
Hide resolved
|
@jpountz I pushed changes |
mikemccand
approved these changes
Dec 6, 2018
| @@ -66,7 +67,7 @@ public PostingsFormat getPostingsFormatForField(String field) { | |||
| private final DocValuesFormat docValuesFormat = new PerFieldDocValuesFormat() { | |||
| @Override | |||
| public DocValuesFormat getDocValuesFormatForField(String field) { | |||
| throw new IllegalStateException("This codec should only be used for reading, not writing"); | |||
| return defaultDVFormat; | |||
There was a problem hiding this comment.
Ahh this is because the DV updates must also be written with the old codec?
There was a problem hiding this comment.
I don't think it's a requirement, but I like using the old format better:
- it's probably easier to debug, as all fields would be using the same format
- given that PFDVFormat creates different files per format, forcing the new format would increase the number of files for old segments
A segmemnt written with Lucene70Codec failes if it ties to update a DV field that didn't exist in the index before it was upgraded to Lucene80Codec. We bake the DV format into the FieldInfo when it's used the first time and therefor never go to the codec if we need to update. yet on a field that didn't exist before and was added during an indexing operation we have to consult the coded and get an exception. This change fixes this issue and adds the relevant bwc tests.
s1monw
force-pushed
the
fix_dv_update_on_old_index
branch
from
7cb255d to
aaa64d7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A segmemnt written with Lucene70Codec failes if it ties to update
a DV field that didn't exist in the index before it was upgraded to
Lucene80Codec. We bake the DV format into the FieldInfo when it's used
the first time and therefor never go to the codec if we need to update.
yet on a field that didn't exist before and was added during an indexing
operation we have to consult the coded and get an exception.
This change fixes this issue and adds the relevant bwc tests.