tibrewalpratik17 commented on code in PR #12544:
URL: https://github.com/apache/pinot/pull/12544#discussion_r1510236537
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -640,7 +640,10 @@ protected static Iterator<RecordInfo>
resolveComparisonTies(
return recordInfo;
}
int comparisonResult =
newComparisonValue.compareTo(maxComparisonValueRecordInfo.getComparisonValue());
- if (comparisonResult >= 0) {
+ if (comparisonResult > 0) {
+ return recordInfo;
+ }
+ if (comparisonResult == 0 && recordInfo.getDocId() >
maxComparisonValueRecordInfo.getDocId()) {
Review Comment:
> I think this is incorrect.
>
> recordInfoIterator and this function iterates over the new segment. We
can't pick the record with the latest doc-id in the new segment.
Yeah i misunderstood the sortedIndex implementation i thought we preserve
the old docId values for a record. Seems we do not.
> To fix this, for each docId in the new segment, you need to get the docId
in the old segment, and keep the record which had the highest docId in the old
segment.
>
> That is a bit of an involved problem.
Hmm looks like we have the mapping available here:
https://github.com/apache/pinot/blob/43dadbfd96a70c19a9ac83bb6c0c35f3fa58bffb/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/converter/RealtimeSegmentConverter.java#L122
Somehow we need to make it available here while resolving comparison ties.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
