Copilot commented on code in PR #17606:
URL: https://github.com/apache/pinot/pull/17606#discussion_r2768439512


##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java:
##########


Review Comment:
   The exception `e` is not included in the `RuntimeException` constructor, 
causing the original exception details to be lost. Include the cause by passing 
`e` as the second argument.
   ```suggestion
                 "Caught exception while reading records from segment: " + 
indexSegment.getSegmentName(), e);
   ```



##########
pinot-core/src/main/java/org/apache/pinot/core/data/manager/offline/DimensionTableDataManager.java:
##########
@@ -397,4 +498,73 @@ public FieldSpec getColumnFieldSpec(String columnName) {
   public List<String> getPrimaryKeyColumns() {
     return _dimensionTable.get().getPrimaryKeyColumns();
   }
+
+  private void applyQueryableDocIdsForRecordLocations(List<SegmentDataManager> 
segmentDataManagers,
+      Object2ObjectOpenCustomHashMap<Object[], RecordLocation> 
recordLocationMap) {
+    if (!_enableUpsert) {
+      return;
+    }
+    if (recordLocationMap.isEmpty() || segmentDataManagers.isEmpty()) {
+      return;
+    }
+    List<MutableRoaringBitmap> queryableDocIdsBySegment = new 
ArrayList<>(segmentDataManagers.size());
+    for (int i = 0; i < segmentDataManagers.size(); i++) {
+      queryableDocIdsBySegment.add(new MutableRoaringBitmap());
+    }
+    for (RecordLocation recordLocation : recordLocationMap.values()) {
+      
queryableDocIdsBySegment.get(recordLocation._segmentIndex).add(recordLocation._docId);
+    }
+    applyQueryableDocIdsToSegments(segmentDataManagers, 
queryableDocIdsBySegment);
+  }
+
+  private void applyQueryableDocIdsForLookupTable(List<SegmentDataManager> 
segmentDataManagers,
+      Object2LongOpenCustomHashMap<Object[]> lookupTable) {
+    if (!_enableUpsert) {
+      return;
+    }
+    if (lookupTable.isEmpty() || segmentDataManagers.isEmpty()) {
+      return;
+    }
+    List<MutableRoaringBitmap> queryableDocIdsBySegment = new 
ArrayList<>(segmentDataManagers.size());
+    for (int i = 0; i < segmentDataManagers.size(); i++) {
+      queryableDocIdsBySegment.add(new MutableRoaringBitmap());
+    }
+    for (Object2LongOpenCustomHashMap.Entry<Object[]> entry : 
lookupTable.object2LongEntrySet()) {
+      long readerIdxAndDocId = entry.getLongValue();
+      int readerIdx = (int) (readerIdxAndDocId >>> 32);
+      int docId = (int) readerIdxAndDocId;
+      queryableDocIdsBySegment.get(readerIdx).add(docId);
+    }
+    applyQueryableDocIdsToSegments(segmentDataManagers, 
queryableDocIdsBySegment);
+  }
+
+  private void applyQueryableDocIdsToSegments(List<SegmentDataManager> 
segmentDataManagers,
+      List<MutableRoaringBitmap> queryableDocIdsBySegment) {
+    for (int i = 0; i < segmentDataManagers.size(); i++) {
+      IndexSegment segment = segmentDataManagers.get(i).getSegment();
+      if (!(segment instanceof ImmutableSegmentImpl)) {
+        continue;
+      }
+      MutableRoaringBitmap queryableDocIds = queryableDocIdsBySegment.get(i);
+      ThreadSafeMutableRoaringBitmap existingQueryableDocIds = 
segment.getQueryableDocIds();
+      if (existingQueryableDocIds != null) {
+        MutableRoaringBitmap existingSnapshot = 
existingQueryableDocIds.getMutableRoaringBitmap();
+        queryableDocIds.and(existingSnapshot);
+      }
+      ThreadSafeMutableRoaringBitmap validDocIds =
+          new ThreadSafeMutableRoaringBitmap(queryableDocIds.clone());
+      ThreadSafeMutableRoaringBitmap queryableDocIdsSnapshot =
+          new ThreadSafeMutableRoaringBitmap(queryableDocIds.clone());

Review Comment:
   The `queryableDocIds` bitmap is cloned twice in successive lines for 
`validDocIds` and `queryableDocIdsSnapshot`. Since these appear to be identical 
snapshots of the same state, consider clarifying the intent with a comment or 
consolidating if both references can share the same cloned instance.



-- 
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]

Reply via email to