kfaraz commented on code in PR #18223:
URL: https://github.com/apache/druid/pull/18223#discussion_r2193927860
##########
processing/src/main/java/org/apache/druid/timeline/DataSegment.java:
##########
@@ -471,17 +468,27 @@ public String toString()
", loadSpec=" + loadSpec +
", dimensions=" + dimensions +
", metrics=" + metrics +
+ ", projections=" + projections +
", shardSpec=" + shardSpec +
", lastCompactionState=" + lastCompactionState +
", size=" + size +
'}';
}
+ /**
+ * @deprecated use {@link #builder(SegmentId)} or {@link
#builder(DataSegment)} instead.
+ */
+ @Deprecated
Review Comment:
I don't think we should deprecate this. It is useful (mostly in tests) when
building a segment from scratch.
##########
processing/src/main/java/org/apache/druid/timeline/DataSegment.java:
##########
@@ -495,20 +502,36 @@ public static class Builder
private Map<String, Object> loadSpec;
private List<String> dimensions;
private List<String> metrics;
+ private List<String> projections;
private ShardSpec shardSpec;
private CompactionState lastCompactionState;
private Integer binaryVersion;
private long size;
+ /**
+ * @deprecated use {@link #Builder(SegmentId)} or {@link
#Builder(DataSegment)} instead.
+ */
+ @Deprecated
public Builder()
Review Comment:
If this is not used anywhere, we can probably just make this constructor
private.
##########
server/src/test/java/org/apache/druid/client/ImmutableDruidDataSourceTest.java:
##########
@@ -38,15 +39,21 @@
public class ImmutableDruidDataSourceTest
{
+ DataSegment TEST_SEGMENT = DataSegment.builder(SegmentId.of("test",
Intervals.of("2017/2018"), "version", null))
Review Comment:
static final
##########
processing/src/main/java/org/apache/druid/timeline/DataSegment.java:
##########
@@ -156,13 +167,19 @@ public DataSegment(
loadSpec,
dimensions,
metrics,
+ null,
shardSpec,
null,
binaryVersion,
- size
+ size,
+ PruneSpecsHolder.DEFAULT
);
}
+ /**
+ * @deprecated use {@link #builder(SegmentId)} or {@link
#builder(DataSegment)} instead.
+ */
+ @Deprecated
Review Comment:
It seems that this PR is removing some usages of these constructors.
If there are no more usages left, we can just remove this constructor or
make it private.
##########
server/src/main/java/org/apache/druid/metadata/IndexerSQLMetadataStorageCoordinator.java:
##########
@@ -1203,18 +1203,12 @@ private SegmentPublishResult
commitAppendSegmentsAndMetadataInTransaction(
final SegmentId newVersionSegmentId =
pendingSegment.getId().asSegmentId();
newVersionSegmentToParent.put(newVersionSegmentId,
oldSegment.getId());
upgradedFromSegmentIdMap.put(newVersionSegmentId.toString(),
oldSegment.getId().toString());
- allSegmentsToInsert.add(
- new DataSegment(
- pendingSegment.getId().asSegmentId(),
- oldSegment.getLoadSpec(),
- oldSegment.getDimensions(),
- oldSegment.getMetrics(),
- pendingSegment.getId().getShardSpec(),
- oldSegment.getLastCompactionState(),
- oldSegment.getBinaryVersion(),
- oldSegment.getSize()
- )
- );
+ allSegmentsToInsert.add(DataSegment.builder(oldSegment)
+
.dataSource(newVersionSegmentId.getDataSource())
Review Comment:
This can be removed as the `dataSource` wouldn't change from the
`oldSegment`.
##########
processing/src/main/java/org/apache/druid/timeline/DataSegment.java:
##########
@@ -98,6 +99,7 @@ public static class PruneSpecsHolder
private final Map<String, Object> loadSpec;
private final List<String> dimensions;
private final List<String> metrics;
+ private final List<String> projections;
Review Comment:
Did you intend to name this field `projectionNames` as mentioned in the PR
description?
--
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]
