Jackie-Jiang commented on code in PR #11791:
URL: https://github.com/apache/pinot/pull/11791#discussion_r1366257090
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -121,6 +121,32 @@ public List<String> getPrimaryKeyColumns() {
return _primaryKeyColumns;
}
+ private MutableRoaringBitmap getQueryableDocIds(IndexSegment segment, String
deleteRecordColumn,
Review Comment:
Make it `protected` so that it can be reused if necessary. No need to pass
in `deleteRecordColumn` since it is a member variable. Annotate the return as
`@Nullable`
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -121,6 +121,32 @@ public List<String> getPrimaryKeyColumns() {
return _primaryKeyColumns;
}
+ private MutableRoaringBitmap getQueryableDocIds(IndexSegment segment, String
deleteRecordColumn,
+ MutableRoaringBitmap validDocIds) {
+ MutableRoaringBitmap queryableDocIds = null;
Review Comment:
(minor) Slightly more readable
```suggestion
if (_deleteRecordColumn == null) {
return null;
}
...
```
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/BasePartitionUpsertMetadataManager.java:
##########
@@ -121,6 +121,32 @@ public List<String> getPrimaryKeyColumns() {
return _primaryKeyColumns;
}
+ private MutableRoaringBitmap getQueryableDocIds(IndexSegment segment, String
deleteRecordColumn,
+ MutableRoaringBitmap validDocIds) {
+ MutableRoaringBitmap queryableDocIds = null;
+ if (deleteRecordColumn != null) {
+ queryableDocIds = new MutableRoaringBitmap();
+ // we allow passing null values for primaryKey columns and
comparisonColumns to read deleteColumn to avoid paying
+ // the overhead of reading primary key, comparison column etc. They will
never be null in other scenarios.
+ UpsertUtils.RecordInfoReader recordInfoReader =
Review Comment:
Suggest not modifying the `RecordInfoReader` because it is not designed for
this purpose. We can simply create a `PinotSegmentColumnReader` for
`_deleteRecordColumn`, then iterate over the `validDocIds`
--
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]
