rohityadav1993 commented on code in PR #13107:
URL: https://github.com/apache/pinot/pull/13107#discussion_r1602098764


##########
pinot-common/src/main/java/org/apache/pinot/common/metadata/segment/SegmentPartitionMetadata.java:
##########
@@ -48,6 +53,21 @@ public SegmentPartitionMetadata(
       @Nonnull @JsonProperty("columnPartitionMap") Map<String, 
ColumnPartitionMetadata> columnPartitionMap) {
     Preconditions.checkNotNull(columnPartitionMap);
     _columnPartitionMap = columnPartitionMap;
+    _uploadedSegmentPartitionId = -1;
+  }
+
+  /**
+   * Constructor for the class.
+   *
+   * @param columnPartitionMap Column name to ColumnPartitionMetadata map.
+   */
+  @JsonCreator
+  public SegmentPartitionMetadata(
+      @Nullable @JsonProperty("columnPartitionMap") Map<String, 
ColumnPartitionMetadata> columnPartitionMap,
+      @Nullable @JsonProperty(value = "uploadedSegmentPartitionId", 
defaultValue = "-1")

Review Comment:
   I see one usage in `SegmentAssignmentUtils.getPartitionId()` used by 
`ReplicaGroupSegmentAssignmentStrategy` for assigning offline segments.
   
   I think we can fail fast here if column partition and 
uploadedSegmentPartitionId both are provided as `uploadedSegmentPartitionId` is 
more for keeping the assignment same as stream partitions for upserts but it 
does not make sense for an offline table.
   
   Open to better name suggestion for `uploadedSegmentPartitionId` in case it 
sounds misleading.



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

Reply via email to