Copilot commented on code in PR #17762:
URL: https://github.com/apache/druid/pull/17762#discussion_r2325676671
##########
services/src/main/java/org/apache/druid/cli/DumpSegment.java:
##########
@@ -459,8 +459,8 @@ public static void runDumpNestedColumn(
final ColumnHolder columnHolder =
index.getColumnHolder(columnName);
final BaseColumn baseColumn = columnHolder.getColumn();
Preconditions.checkArgument(baseColumn instanceof
CompressedNestedDataComplexColumn);
- final CompressedNestedDataComplexColumn<?> nestedDataColumn =
- (CompressedNestedDataComplexColumn<?>) baseColumn;
+ final CompressedNestedDataComplexColumn<?, ?> nestedDataColumn
=
+ (CompressedNestedDataComplexColumn<?, ?>) baseColumn;
Review Comment:
[nitpick] The cast from wildcard generic type `<?>` to `<?, ?>` is redundant
and adds no type safety. Consider using a single wildcard
`CompressedNestedDataComplexColumn<?>` or removing the explicit type parameters
altogether.
```suggestion
final CompressedNestedDataComplexColumn<?> nestedDataColumn =
(CompressedNestedDataComplexColumn<?>) baseColumn;
```
##########
server/src/main/java/org/apache/druid/server/compaction/CompactionStatus.java:
##########
@@ -209,7 +209,8 @@ static CompactionStatus compute(
)
{
final Evaluator evaluator = new Evaluator(candidateSegments, config,
objectMapper);
- return CHECKS.stream().map(f -> f.apply(evaluator))
+ return CHECKS.stream()
+ .map(f -> f.apply(evaluator))
.filter(status -> !status.isComplete())
.findFirst().orElse(COMPLETE);
Review Comment:
[nitpick] The formatting change here appears to be a style-only modification
without functional impact. Consider maintaining consistency with the existing
codebase formatting patterns unless there's a specific reason for this change.
```suggestion
return CHECKS.stream().map(f -> f.apply(evaluator))
.filter(status -> !status.isComplete())
.findFirst().orElse(COMPLETE);
```
##########
server/src/main/java/org/apache/druid/server/compaction/CompactionStatus.java:
##########
@@ -232,19 +233,27 @@ static PartitionsSpec
findPartitionsSpecFromConfig(ClientCompactionTaskQueryTuni
}
}
+ @Nullable
private static List<DimensionSchema> getNonPartitioningDimensions(
@Nullable final List<DimensionSchema> dimensionSchemas,
- @Nullable final PartitionsSpec partitionsSpec
+ @Nullable final PartitionsSpec partitionsSpec,
+ @Nullable final IndexSpec indexSpec
)
{
if (dimensionSchemas == null || !(partitionsSpec instanceof
DimensionRangePartitionsSpec)) {
- return dimensionSchemas;
+ if (dimensionSchemas != null) {
+ return dimensionSchemas.stream()
+ .map(dim -> dim.getEffectiveSchema(indexSpec))
+ .collect(Collectors.toList());
+ }
+ return null;
}
final List<String> partitionsDimensions = ((DimensionRangePartitionsSpec)
partitionsSpec).getPartitionDimensions();
return dimensionSchemas.stream()
- .filter(dim ->
!partitionsDimensions.contains(dim.getName()))
- .collect(Collectors.toList());
+ .filter(dim ->
!partitionsDimensions.contains(dim.getName()))
+ .map(dim -> dim.getEffectiveSchema(indexSpec))
+ .collect(Collectors.toList());
Review Comment:
[nitpick] The logic for transforming dimension schemas is duplicated below.
Consider extracting this transformation into a private helper method to reduce
code duplication and improve maintainability.
```suggestion
return transformDimensionSchemas(dimensionSchemas, indexSpec);
}
return null;
}
final List<String> partitionsDimensions =
((DimensionRangePartitionsSpec) partitionsSpec).getPartitionDimensions();
return transformDimensionSchemas(
dimensionSchemas.stream()
.filter(dim -> !partitionsDimensions.contains(dim.getName()))
.collect(Collectors.toList()),
indexSpec
);
}
private static List<DimensionSchema> transformDimensionSchemas(
final List<DimensionSchema> dimensionSchemas,
final IndexSpec indexSpec
)
{
return dimensionSchemas.stream()
.map(dim -> dim.getEffectiveSchema(indexSpec))
.collect(Collectors.toList());
```
--
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]
