FrankChen021 commented on code in PR #20795:
URL: https://github.com/apache/druid/pull/20795#discussion_r4419981051
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/actions/SegmentAllocateActionTest.java:
##########
@@ -1277,10 +1236,7 @@ private void assertSameIdentifier(final
SegmentIdWithShardSpec expected, final S
private DataSegment getSegmentForIdentifier(SegmentIdWithShardSpec
identifier)
{
- return DataSegment.builder()
- .dataSource(identifier.getDataSource())
- .interval(identifier.getInterval())
- .version(identifier.getVersion())
+ return DataSegment.builder(SegmentId.of(identifier.getDataSource(),
identifier.getInterval(), identifier.getVersion(), 0))
Review Comment:
nit: `identifier.getShardSpec()` is often a non-zero partition here, so
hardcoding `0` in `SegmentId.of(...)` is misleading (the builder ignores it and
takes the partition from `shardSpec`). `SegmentIdWithShardSpec` already has
`asSegmentId()`:
```java
return DataSegment.builder(identifier.asSegmentId())
```
--
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]
