xiangfu0 commented on code in PR #19897:
URL: https://github.com/apache/pinot/pull/19897#discussion_r4364553975
##########
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)
Review Comment:
The shared helper is now in TextIndexUtils, ready for a bounded follow-up.
Multi-column writers/readers remain stored-field based in this PR, as do their
queries; the description makes that boundary explicit. Adding doc values there
would expand the writer/format/performance scope and needs separate
measurements and coverage.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/text/LuceneTextIndexCreator.java:
##########
@@ -264,21 +269,26 @@ private void buildMappingFile(File segmentIndexDir,
String column, Directory dir
try (PinotDataBuffer buffer = PinotDataBuffer.mapFile(docIdMappingFile, /*
readOnly */ false, 0, length,
ByteOrder.LITTLE_ENDIAN, desc)) {
try {
- // If immutableToMutableIdMap is null, then docIds should not change
between the mutable and immutable segments.
- // Therefore, the mapping file can be built without doing an
additional docId conversion
- if (mutableToImmutableDocIdMap == null) {
- for (int i = 0; i < numDocs; i++) {
- Document document = indexSearcher.storedFields().document(i);
- int pinotDocId =
Integer.parseInt(document.get(LuceneTextIndexCreator.LUCENE_INDEX_DOC_ID_COLUMN_NAME));
- buffer.putInt(i * Integer.BYTES, pinotDocId);
- }
- } else {
- for (int i = 0; i < numDocs; i++) {
- Document document = indexSearcher.storedFields().document(i);
- int mutableDocId =
Integer.parseInt(document.get(LuceneTextIndexCreator.LUCENE_INDEX_DOC_ID_COLUMN_NAME));
- int immutableDocId = mutableToImmutableDocIdMap[mutableDocId];
- buffer.putInt(i * Integer.BYTES, immutableDocId);
+ NumericDocValues docIds = MultiDocValues.getNumericValues(indexReader,
PINOT_DOC_ID_DOC_VALUES_FIELD_NAME);
Review Comment:
Implemented in
https://github.com/apache/pinot/commit/5c9153975628af902279503127bd7d141f031460:
creator and reader now call TextIndexUtils.fillDocIdMapping(IndexReader,
buffer, documentCount, optionalMutableToImmutableRemap). Numeric lookup,
per-document stored fallback, validation and buffer writes are shared;
remapping happens before identity detection. The existing mapping test now
verifies a remap that produces identity, and constructor/overload/Javadoc
parameters are renamed mutableToImmutableDocIdMap.
--
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]
