Jackie-Jiang commented on a change in pull request #7844:
URL: https://github.com/apache/pinot/pull/7844#discussion_r759760648
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/PartitionUpsertMetadataManager.java
##########
@@ -186,27 +182,32 @@ public GenericRow updateRecord(IndexSegment segment,
RecordInfo recordInfo, Gene
}
}
+ ThreadSafeMutableRoaringBitmap validDocIds = segment.getValidDocIds();
+ assert validDocIds != null;
_result = record;
- _primaryKeyToRecordLocationMap
- .compute(hashPrimaryKey(recordInfo._primaryKey, _hashFunction),
(primaryKey, currentRecordLocation) -> {
+
_primaryKeyToRecordLocationMap.compute(hashPrimaryKey(recordInfo._primaryKey,
_hashFunction),
+ (primaryKey, currentRecordLocation) -> {
if (currentRecordLocation != null) {
// Existing primary key
- // Update the record location when the new comparison value is
greater than or equal to the current value
- // . Update
- // the record location when there is a tie to keep the newer
record.
+ // Update the record location when the new comparison value is
greater than or equal to the current value.
+ // Update the record location when there is a tie to keep the
newer record.
if
(recordInfo._comparisonValue.compareTo(currentRecordLocation.getComparisonValue())
>= 0) {
IndexSegment currentSegment = currentRecordLocation.getSegment();
+ int currentDocId = currentRecordLocation.getDocId();
if (_partialUpsertHandler != null) {
// Partial upsert
_reuse.clear();
- GenericRow previousRecord =
currentSegment.getRecord(currentRecordLocation.getDocId(), _reuse);
+ GenericRow previousRecord =
currentSegment.getRecord(currentDocId, _reuse);
_result = _partialUpsertHandler.merge(previousRecord, record);
}
- assert currentSegment.getValidDocIds() != null;
Review comment:
@yupeng9 That shouldn't matter as it just reads a member variable from
the segment into a local variable. I simply extracted that common logic ahead
to avoid making multiple assertions.
@richardstartin Great article, thanks for sharing. In that case should we
prefer
`Objects.requireNonNull(currentSegment.getValidDocIds()).remove(currentDocId);`
as IDE suggested to suppress the warning?
--
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]
