[GitHub] [lucene-solr] jimczi commented on a change in pull request #1394: LUCENE-9300: Fix field infos update on doc values update
jimczi 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_r402936952
##
File path: lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java
##
@@ -543,27 +543,37 @@ 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.
+int maxFieldNumber = -1;
+Map perName = new HashMap<>();
Review comment:
https://github.com/apache/lucene-solr/pull/1394/commits/9fed4319409a4d6c3a70e500cc51a58766116e21
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
[GitHub] [lucene-solr] jimczi commented on a change in pull request #1394: LUCENE-9300: Fix field infos update on doc values update
jimczi 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_r402404730
##
File path: lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java
##
@@ -543,27 +543,39 @@ 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.
+int maxFieldNumber = -1;
+Map newFields = new HashMap<>();
for (FieldInfo fi : reader.getFieldInfos()) {
- FieldInfo clone = builder.add(fi);
- // copy the stuff FieldInfos.Builder doesn't copy
- for (Entry 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 = cloneFieldInfo(fi, fi.number);
+ newFields.put(clone.name, clone);
+ maxFieldNumber = Math.max(clone.number, maxFieldNumber);
}
// create new fields with the right DV type
+FieldInfos.Builder builder = new FieldInfos.Builder(fieldNumbers);
for (List updates : pendingDVUpdates.values()) {
DocValuesFieldUpdates update = updates.get(0);
- FieldInfo fieldInfo = builder.getOrAdd(update.field);
- fieldInfo.setDocValuesType(update.type);
+ FieldInfo globalFieldInfo = builder.getOrAdd(update.field);
+ globalFieldInfo.setDocValuesType(update.type);
+ FieldInfo segmentFieldInfo = newFields.get(update.field);
+ if (segmentFieldInfo == null) {
+// the field is not present in this segment so we can fallback to
the global fields.
+if (globalFieldInfo.number <= maxFieldNumber) {
+ // the global field number is already used in this segment for a
different field so we force a new one locally.
+ globalFieldInfo = cloneFieldInfo(globalFieldInfo,
++maxFieldNumber);
Review comment:
we should, thanks. I rewrote the logic in my last commits. I thought that
`addOrGet` would preserve the dv type but it doesn't.
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
[GitHub] [lucene-solr] jimczi commented on a change in pull request #1394: LUCENE-9300: Fix field infos update on doc values update
jimczi 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_r402403738
##
File path: lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java
##
@@ -543,27 +543,39 @@ 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.
+int maxFieldNumber = -1;
+Map newFields = new HashMap<>();
for (FieldInfo fi : reader.getFieldInfos()) {
- FieldInfo clone = builder.add(fi);
- // copy the stuff FieldInfos.Builder doesn't copy
- for (Entry 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)).
Review comment:
++
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
[GitHub] [lucene-solr] jimczi commented on a change in pull request #1394: LUCENE-9300: Fix field infos update on doc values update
jimczi 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_r402403553
##
File path: lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java
##
@@ -543,27 +543,39 @@ 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.
+int maxFieldNumber = -1;
+Map newFields = new HashMap<>();
for (FieldInfo fi : reader.getFieldInfos()) {
- FieldInfo clone = builder.add(fi);
- // copy the stuff FieldInfos.Builder doesn't copy
- for (Entry 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 = cloneFieldInfo(fi, fi.number);
+ newFields.put(clone.name, clone);
+ maxFieldNumber = Math.max(clone.number, maxFieldNumber);
}
// create new fields with the right DV type
+FieldInfos.Builder builder = new FieldInfos.Builder(fieldNumbers);
for (List updates : pendingDVUpdates.values()) {
DocValuesFieldUpdates update = updates.get(0);
- FieldInfo fieldInfo = builder.getOrAdd(update.field);
- fieldInfo.setDocValuesType(update.type);
+ FieldInfo globalFieldInfo = builder.getOrAdd(update.field);
+ globalFieldInfo.setDocValuesType(update.type);
+ FieldInfo segmentFieldInfo = newFields.get(update.field);
+ if (segmentFieldInfo == null) {
+// the field is not present in this segment so we can fallback to
the global fields.
+if (globalFieldInfo.number <= maxFieldNumber) {
+ // the global field number is already used in this segment for a
different field so we force a new one locally.
+ globalFieldInfo = cloneFieldInfo(globalFieldInfo,
++maxFieldNumber);
+}
Review comment:
Thanks, I reworked the logic to always clone and reassign the field number.
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
[GitHub] [lucene-solr] jimczi commented on a change in pull request #1394: LUCENE-9300: Fix field infos update on doc values update
jimczi 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_r402403634
##
File path: lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java
##
@@ -543,27 +543,39 @@ 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.
+int maxFieldNumber = -1;
+Map newFields = new HashMap<>();
for (FieldInfo fi : reader.getFieldInfos()) {
- FieldInfo clone = builder.add(fi);
- // copy the stuff FieldInfos.Builder doesn't copy
- for (Entry 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 = cloneFieldInfo(fi, fi.number);
+ newFields.put(clone.name, clone);
+ maxFieldNumber = Math.max(clone.number, maxFieldNumber);
}
// create new fields with the right DV type
+FieldInfos.Builder builder = new FieldInfos.Builder(fieldNumbers);
for (List updates : pendingDVUpdates.values()) {
DocValuesFieldUpdates update = updates.get(0);
- FieldInfo fieldInfo = builder.getOrAdd(update.field);
- fieldInfo.setDocValuesType(update.type);
+ FieldInfo globalFieldInfo = builder.getOrAdd(update.field);
+ globalFieldInfo.setDocValuesType(update.type);
+ FieldInfo segmentFieldInfo = newFields.get(update.field);
+ if (segmentFieldInfo == null) {
+// the field is not present in this segment so we can fallback to
the global fields.
+if (globalFieldInfo.number <= maxFieldNumber) {
+ // the global field number is already used in this segment for a
different field so we force a new one locally.
+ globalFieldInfo = cloneFieldInfo(globalFieldInfo,
++maxFieldNumber);
+}
+newFields.put(update.field, globalFieldInfo);
+ } else {
+segmentFieldInfo.setDocValuesType(update.type);
+newFields.put(update.field, segmentFieldInfo);
Review comment:
++, removed
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
[GitHub] [lucene-solr] jimczi commented on a change in pull request #1394: LUCENE-9300: Fix field infos update on doc values update
jimczi 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_r402403065
##
File path: lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java
##
@@ -543,27 +543,39 @@ 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.
+int maxFieldNumber = -1;
+Map newFields = new HashMap<>();
for (FieldInfo fi : reader.getFieldInfos()) {
- FieldInfo clone = builder.add(fi);
- // copy the stuff FieldInfos.Builder doesn't copy
- for (Entry 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 = cloneFieldInfo(fi, fi.number);
+ newFields.put(clone.name, clone);
+ maxFieldNumber = Math.max(clone.number, maxFieldNumber);
}
// create new fields with the right DV type
+FieldInfos.Builder builder = new FieldInfos.Builder(fieldNumbers);
for (List updates : pendingDVUpdates.values()) {
DocValuesFieldUpdates update = updates.get(0);
- FieldInfo fieldInfo = builder.getOrAdd(update.field);
- fieldInfo.setDocValuesType(update.type);
+ FieldInfo globalFieldInfo = builder.getOrAdd(update.field);
+ globalFieldInfo.setDocValuesType(update.type);
Review comment:
++,
https://github.com/apache/lucene-solr/pull/1394/commits/0b5c6c896bcc67f8ec6d9206d5dc9caa5cc5ee81
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
[GitHub] [lucene-solr] jimczi commented on a change in pull request #1394: LUCENE-9300: Fix field infos update on doc values update
jimczi 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_r402402939
##
File path: lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java
##
@@ -543,27 +543,39 @@ 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.
+int maxFieldNumber = -1;
+Map newFields = new HashMap<>();
for (FieldInfo fi : reader.getFieldInfos()) {
- FieldInfo clone = builder.add(fi);
- // copy the stuff FieldInfos.Builder doesn't copy
- for (Entry 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 = cloneFieldInfo(fi, fi.number);
+ newFields.put(clone.name, clone);
+ maxFieldNumber = Math.max(clone.number, maxFieldNumber);
}
// create new fields with the right DV type
+FieldInfos.Builder builder = new FieldInfos.Builder(fieldNumbers);
for (List updates : pendingDVUpdates.values()) {
DocValuesFieldUpdates update = updates.get(0);
- FieldInfo fieldInfo = builder.getOrAdd(update.field);
- fieldInfo.setDocValuesType(update.type);
+ FieldInfo globalFieldInfo = builder.getOrAdd(update.field);
+ globalFieldInfo.setDocValuesType(update.type);
Review comment:
I added an
[assertion](https://github.com/apache/lucene-solr/pull/1394/commits/0b5c6c896bcc67f8ec6d9206d5dc9caa5cc5ee81)
to check the dv type of the global field but we still need to set the dv type
on the cloned field since it is not preserved by the `getOrAdd` call.
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
[GitHub] [lucene-solr] jimczi commented on a change in pull request #1394: LUCENE-9300: Fix field infos update on doc values update
jimczi 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_r402394140
##
File path:
lucene/core/src/test/org/apache/lucene/index/TestNumericDocValuesUpdates.java
##
@@ -1483,6 +1484,83 @@ public void testAddIndexes() throws Exception {
IOUtils.close(dir1, dir2);
}
+ public void testUpdatesAfterAddIndexes() throws Exception {
Review comment:
++, I pushed
https://github.com/apache/lucene-solr/pull/1394/commits/00d755888d4744ff6a1beba17e20580f43841f12
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
[GitHub] [lucene-solr] jimczi commented on a change in pull request #1394: LUCENE-9300: Fix field infos update on doc values update
jimczi 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_r402394008
##
File path: lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java
##
@@ -644,6 +656,13 @@ public synchronized boolean writeFieldUpdates(Directory
dir, FieldInfos.FieldNum
return true;
}
+ private FieldInfo cloneFieldInfo(FieldInfo fi, int fieldNumber) {
+return new FieldInfo(fi.name, fieldNumber, fi.hasVectors(),
fi.omitsNorms(), fi.hasPayloads(),
+fi.getIndexOptions(), fi.getDocValuesType(), fi.getDocValuesGen(), new
HashMap<>(fi.attributes()),
+fi.getPointDimensionCount(), fi.getPointIndexDimensionCount(),
fi.getPointNumBytes(), fi.isSoftDeletesField());
+
Review comment:
++,
https://github.com/apache/lucene-solr/pull/1394/commits/34718cb64e747e56ebc5341f9180d2f12aace029
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
[GitHub] [lucene-solr] jimczi commented on a change in pull request #1394: LUCENE-9300: Fix field infos update on doc values update
jimczi 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_r401750154
##
File path: lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java
##
@@ -543,27 +543,36 @@ 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.
+int maxFieldNumber = -1;
+Map newFields = new HashMap<>();
for (FieldInfo fi : reader.getFieldInfos()) {
- FieldInfo clone = builder.add(fi);
- // copy the stuff FieldInfos.Builder doesn't copy
- for (Entry 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 = cloneFieldInfo(fi, fi.number);
+ newFields.put(clone.name, clone);
+ maxFieldNumber = Math.max(clone.number, maxFieldNumber);
}
// create new fields with the right DV type
+FieldInfos.Builder builder = new FieldInfos.Builder(fieldNumbers);
for (List 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);
+if (fi.number <= maxFieldNumber) {
+ // the global field number is already used in this segment for a
different field so we force a new one locally.
+ fi = cloneFieldInfo(fi, ++maxFieldNumber);
Review comment:
> Do we need to set the doc-value type before cloning to make sure the
doc-value type is set on the writer?
thanks, I pushed
https://github.com/apache/lucene-solr/pull/1394/commits/090ef78171dfe671b0fb1ab238951fdd617bd65c
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
[GitHub] [lucene-solr] jimczi commented on a change in pull request #1394: LUCENE-9300: Fix field infos update on doc values update
jimczi 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_r401749799
##
File path: lucene/core/src/java/org/apache/lucene/index/ReadersAndUpdates.java
##
@@ -543,27 +543,36 @@ 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.
+int maxFieldNumber = -1;
+Map newFields = new HashMap<>();
for (FieldInfo fi : reader.getFieldInfos()) {
- FieldInfo clone = builder.add(fi);
- // copy the stuff FieldInfos.Builder doesn't copy
- for (Entry 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 = cloneFieldInfo(fi, fi.number);
+ newFields.put(clone.name, clone);
+ maxFieldNumber = Math.max(clone.number, maxFieldNumber);
}
// create new fields with the right DV type
+FieldInfos.Builder builder = new FieldInfos.Builder(fieldNumbers);
for (List 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);
+if (fi.number <= maxFieldNumber) {
+ // the global field number is already used in this segment for a
different field so we force a new one locally.
+ fi = cloneFieldInfo(fi, ++maxFieldNumber);
Review comment:
> I also wonder if we should use FieldInfos.Builder since this logic you
implemented looks similar to FieldInfos.Builder#add.
I don't think it works since the logic there is to use the global field
number if the field already exists in another segment. In the logic above we
try to re-assign the global field number locally since it clashes with a local
one.
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
[GitHub] [lucene-solr] jimczi commented on a change in pull request #1394: LUCENE-9300: Fix field infos update on doc values update
jimczi 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_r401672524
##
File path:
lucene/core/src/test/org/apache/lucene/index/TestNumericDocValuesUpdates.java
##
@@ -1483,6 +1484,81 @@ public void testAddIndexes() throws Exception {
IOUtils.close(dir1, dir2);
}
+ public void testUpdatesAterAddIndexes() throws Exception {
Review comment:
thanks,
https://github.com/apache/lucene-solr/pull/1394/commits/abd5e08170cc8b7963c0aff3d58afa5766afcb32
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
[GitHub] [lucene-solr] jimczi commented on a change in pull request #1394: LUCENE-9300: Fix field infos update on doc values update
jimczi 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_r401672435
##
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 newFields = new HashMap<>();
for (FieldInfo fi : reader.getFieldInfos()) {
- FieldInfo clone = builder.add(fi);
- // copy the stuff FieldInfos.Builder doesn't copy
- for (Entry 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 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:
How to introduce the same bug but the other way around :(. Thanks for
catching this Adrien. I pushed
https://github.com/apache/lucene-solr/pull/1394/commits/abd5e08170cc8b7963c0aff3d58afa5766afcb32
to handle this case properly.
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
