xiangfu0 commented on code in PR #19897:
URL: https://github.com/apache/pinot/pull/19897#discussion_r4364555769
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java:
##########
@@ -491,33 +460,7 @@ private DocIdTranslator
createDocIdTranslator(PinotDataBuffer docIdMappingBuffer
try {
if (config.getDocIdTranslatorMode() == DocIdTranslatorMode.TryOptimize) {
LOGGER.debug("Creating lucene to pinot doc id mapping.");
- boolean allIdsAreEqual = true;
-
- class DocIdVisitor extends DocumentStoredFieldVisitor {
- int _pinotDocId;
-
- @Override
- public Status needsField(FieldInfo fieldInfo) {
- // assume doc id is the only stored document
- assert
LuceneTextIndexCreator.LUCENE_INDEX_DOC_ID_COLUMN_NAME.equals(fieldInfo.name);
- return Status.YES;
- }
-
- @Override
- public void intField(FieldInfo fieldInfo, int value) {
- _pinotDocId = value;
- }
- }
-
- DocIdVisitor visitor = new DocIdVisitor();
- StoredFields storedFields = _indexSearcher.storedFields();
-
- for (int i = 0; i < numDocs; i++) {
- storedFields.document(i, visitor);
- int pinotDocId = visitor._pinotDocId;
- allIdsAreEqual &= (i == pinotDocId);
- buffer.putInt(i * Integer.BYTES, pinotDocId);
- }
+ boolean allIdsAreEqual = fillDocIdMapping(_indexSearcher, buffer,
numDocs);
Review Comment:
Simplified both paths in
https://github.com/apache/pinot/commit/5c9153975628af902279503127bd7d141f031460:
fill the mapping once, handle TryOptimize identity, then use one common
DefaultDocIdTranslator return. Skip/existing-mapping early exits and the
directory path configuration updates are preserved. All 19 focused tests passed.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java:
##########
@@ -542,4 +480,53 @@ public void intField(FieldInfo fieldInfo, int value) {
e);
}
}
+
+ /// Reads the actual Pinot ids after Lucene merges and sorting. Doc values
avoid stored-field decompression;
+ /// indexes created by older writers, including individual older leaves,
retain the stored-field fallback.
+ @VisibleForTesting
+ static boolean fillDocIdMapping(IndexSearcher indexSearcher, PinotDataBuffer
buffer, int numDocs)
+ throws IOException {
+ NumericDocValues docIds =
MultiDocValues.getNumericValues(indexSearcher.getIndexReader(),
+ LuceneTextIndexCreator.PINOT_DOC_ID_DOC_VALUES_FIELD_NAME);
+ class DocIdVisitor extends DocumentStoredFieldVisitor {
Review Comment:
Moved the visitor alongside the shared helper in
https://github.com/apache/pinot/commit/5c9153975628af902279503127bd7d141f031460
as private static PinotDocIdVisitor extending StoredFieldVisitor directly. Each
mapping operation owns one visitor, resets its sentinel before every
stored-field read, and lazily obtains one StoredFields instance; no mutable
visitor is shared statically.
--
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]
