cpoerschke commented on code in PR #2176:
URL: https://github.com/apache/solr/pull/2176#discussion_r1471203055
##########
solr/core/src/java/org/apache/solr/index/SlowCompositeReaderWrapper.java:
##########
@@ -109,9 +109,10 @@ public static LeafReader wrap(IndexReader reader) throws
IOException {
minVersion = leafVersion;
}
}
- int createdVersionMajor =
-
reader.leaves().get(0).reader().getMetaData().getCreatedVersionMajor();
- metaData = new LeafMetaData(createdVersionMajor, minVersion, null);
+ LeafMetaData leafMetaData =
reader.leaves().get(0).reader().getMetaData();
+ metaData =
+ new LeafMetaData(
+ leafMetaData.getCreatedVersionMajor(), minVersion, null,
leafMetaData.hasBlocks());
Review Comment:
> Out of curiosity, why don't we also specify the `sort` parameter here (3rd
parameter)? It could be retrieved from existing leaf metadata (this is not new
with this change).
I wonder if this might be because we don't yet have 'full' index sorting
directly configurable in Solr --
https://issues.apache.org/jira/browse/SOLR-13681 exists for that -- i.e.
commonly the `sort` would indeed be `null` except when a `SortingMergePolicy`
is configured in which case we'd have 'partial' index sorting i.e. some
segments could be sorted and some could be unsorted and thus it would be tricky
to confidently set the `sort` value to anything other than the `null` value.
> ... (this is not new with this change).
My preference would be to not change the parameter setting as part of the
Lucene upgrade but to perhaps leave a note on SOLR-13681 or have a separate
issue for that change.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
