jerryshao commented on code in PR #10189:
URL: https://github.com/apache/gravitino/pull/10189#discussion_r2893560929
##########
common/src/main/java/org/apache/gravitino/dto/policy/PolicyContentDTO.java:
##########
@@ -69,4 +78,163 @@ public Map<String, String> properties() {
return properties;
}
}
+
+ /** Represents a typed iceberg compaction policy content DTO. */
+ @EqualsAndHashCode
+ @ToString
+ @Builder(setterPrefix = "with")
+ @AllArgsConstructor(access = lombok.AccessLevel.PRIVATE)
+ class IcebergCompactionContentDTO implements PolicyContentDTO {
+
+ @JsonProperty("minDataFileMse")
+ @JsonAlias("minDatafileMse")
+ private Long minDataFileMse;
+
+ @JsonProperty("minDeleteFileNumber")
+ private Long minDeleteFileNumber;
+
+ @JsonProperty("dataFileMseWeight")
+ @JsonAlias("datafileMseWeight")
+ private Long dataFileMseWeight;
+
+ @JsonProperty("deleteFileNumberWeight")
+ private Long deleteFileNumberWeight;
+
+ @JsonProperty("maxPartitionNum")
+ private Long maxPartitionNum;
+
+ @JsonProperty("rewriteOptions")
+ private Map<String, String> rewriteOptions;
+
+ private static final Pattern OPTION_KEY_PATTERN =
Pattern.compile("[A-Za-z0-9._-]+");
+
+ // Default constructor for Jackson deserialization only.
+ private IcebergCompactionContentDTO() {}
+
+ /**
+ * Returns the minimum threshold for custom-data-file-mse metric.
+ *
+ * @return minimum data file MSE threshold
+ */
+ public Long minDataFileMse() {
+ return minDataFileMse == null
+ ? IcebergCompactionContent.DEFAULT_MIN_DATA_FILE_MSE
+ : minDataFileMse;
+ }
+
+ /**
+ * Returns the minimum threshold for custom-delete-file-number metric.
+ *
+ * @return minimum delete file number threshold
+ */
+ public Long minDeleteFileNumber() {
+ return minDeleteFileNumber == null
+ ? IcebergCompactionContent.DEFAULT_MIN_DELETE_FILE_NUMBER
+ : minDeleteFileNumber;
+ }
+
+ /**
+ * Returns the weight for custom-data-file-mse metric in score expression.
+ *
+ * @return data file MSE score weight
+ */
+ public Long dataFileMseWeight() {
+ return dataFileMseWeight == null
+ ? IcebergCompactionContent.DEFAULT_DATA_FILE_MSE_WEIGHT
+ : dataFileMseWeight;
+ }
+
+ /**
+ * Returns the weight for custom-delete-file-number metric in score
expression.
+ *
+ * @return delete file number score weight
+ */
+ public Long deleteFileNumberWeight() {
+ return deleteFileNumberWeight == null
+ ? IcebergCompactionContent.DEFAULT_DELETE_FILE_NUMBER_WEIGHT
+ : deleteFileNumberWeight;
+ }
+
+ /**
+ * Returns max partition number selected for compaction.
+ *
+ * @return max partition number
+ */
+ public Long maxPartitionNum() {
+ return maxPartitionNum == null
+ ? IcebergCompactionContent.DEFAULT_MAX_PARTITION_NUM
+ : maxPartitionNum;
+ }
+
+ /**
+ * Returns rewrite options expanded to job.options.* during rule
generation.
+ *
+ * @return rewrite options map
+ */
+ public Map<String, String> rewriteOptions() {
+ return rewriteOptions == null
+ ? IcebergCompactionContent.DEFAULT_REWRITE_OPTIONS
+ : Collections.unmodifiableMap(new LinkedHashMap<>(rewriteOptions));
+ }
+
+ @Override
+ public Set<MetadataObject.Type> supportedObjectTypes() {
+ return ImmutableSet.of(
+ MetadataObject.Type.CATALOG, MetadataObject.Type.SCHEMA,
MetadataObject.Type.TABLE);
+ }
+
+ @Override
+ public Map<String, String> properties() {
Review Comment:
`properties()` and `rules()` each construct a full
`IcebergCompactionContent` via `PolicyContents.icebergCompaction(...)` just to
call `.properties()` or `.rules()` on it. If both are called together (e.g.
during serialization + validation), two intermediate objects are allocated.
Consider caching or calling the API domain object once:
```java
private IcebergCompactionContent toDomain() {
return (IcebergCompactionContent) PolicyContents.icebergCompaction(
minDataFileMse(), minDeleteFileNumber(), dataFileMseWeight(),
deleteFileNumberWeight(), maxPartitionNum(), rewriteOptions());
}
@Override public Map<String, String> properties() { return
toDomain().properties(); }
@Override public Map<String, Object> rules() { return toDomain().rules(); }
```
##########
common/src/main/java/org/apache/gravitino/dto/policy/PolicyContentDTO.java:
##########
@@ -69,4 +78,163 @@ public Map<String, String> properties() {
return properties;
}
}
+
+ /** Represents a typed iceberg compaction policy content DTO. */
+ @EqualsAndHashCode
+ @ToString
+ @Builder(setterPrefix = "with")
+ @AllArgsConstructor(access = lombok.AccessLevel.PRIVATE)
+ class IcebergCompactionContentDTO implements PolicyContentDTO {
+
+ @JsonProperty("minDataFileMse")
+ @JsonAlias("minDatafileMse")
+ private Long minDataFileMse;
+
+ @JsonProperty("minDeleteFileNumber")
+ private Long minDeleteFileNumber;
+
+ @JsonProperty("dataFileMseWeight")
+ @JsonAlias("datafileMseWeight")
+ private Long dataFileMseWeight;
+
+ @JsonProperty("deleteFileNumberWeight")
+ private Long deleteFileNumberWeight;
+
+ @JsonProperty("maxPartitionNum")
+ private Long maxPartitionNum;
+
+ @JsonProperty("rewriteOptions")
+ private Map<String, String> rewriteOptions;
+
+ private static final Pattern OPTION_KEY_PATTERN =
Pattern.compile("[A-Za-z0-9._-]+");
Review Comment:
`OPTION_KEY_PATTERN` is duplicated between `IcebergCompactionContent` (line
82 in the API module) and `IcebergCompactionContentDTO` here. The DTO
validation also duplicates the same range-check and `rewriteOptions` iteration
logic from `IcebergCompactionContent.validate()`. The DTO should delegate to
the domain object (e.g., `toDomain().validate()`) rather than re-implementing
the same checks.
--
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]
