jpountz commented on a change in pull request #1394: LUCENE-9300: Fix field infos update on doc values update URL: https://github.com/apache/lucene-solr/pull/1394#discussion_r401619000
########## File path: lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java ########## @@ -543,27 +543,30 @@ public synchronized boolean writeFieldUpdates(Directory dir, FieldInfos.FieldNum try { // clone FieldInfos so that we can update their dvGen separately from - // the reader's infos and write them to a new fieldInfos_gen file - FieldInfos.Builder builder = new FieldInfos.Builder(fieldNumbers); - // cannot use builder.add(reader.getFieldInfos()) because it does not - // clone FI.attributes as well FI.dvGen + // the reader's infos and write them to a new fieldInfos_gen file. + Map<String, FieldInfo> newFields = new HashMap<>(); for (FieldInfo fi : reader.getFieldInfos()) { - FieldInfo clone = builder.add(fi); - // copy the stuff FieldInfos.Builder doesn't copy - for (Entry<String,String> e : fi.attributes().entrySet()) { - clone.putAttribute(e.getKey(), e.getValue()); - } - clone.setDocValuesGen(fi.getDocValuesGen()); + // cannot use builder.add(fi) because it does not preserve + // the local field number. Field numbers can be different from the global ones + // if the segment was created externally (with IndexWriter#addIndexes(Directory)). + FieldInfo clone = new FieldInfo(fi); + newFields.put(clone.name, clone); } // create new fields with the right DV type + FieldInfos.Builder builder = new FieldInfos.Builder(fieldNumbers); for (List<DocValuesFieldUpdates> updates : pendingDVUpdates.values()) { DocValuesFieldUpdates update = updates.get(0); - FieldInfo fieldInfo = builder.getOrAdd(update.field); - fieldInfo.setDocValuesType(update.type); + FieldInfo fi = newFields.get(update.field); + if (fi == null) { + // the field is not present in this segment so we can fallback to the global fields. + fi = builder.getOrAdd(update.field); Review comment: Had a quick discussion about this line with Jim. This isn't good enough as this might return a field number that already exists in the reader if the field exists in the writer with a number that it less than the number of fields in the reader. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org