singhpk234 commented on code in PR #14739:
URL: https://github.com/apache/iceberg/pull/14739#discussion_r2583360021


##########
core/src/test/java/org/apache/iceberg/rest/responses/TestFetchScanTasksResponseParser.java:
##########
@@ -95,7 +95,7 @@ public void 
roundTripSerdeWithDeleteFilesNoFileScanTasksPresent() {
 
     String invalidJson =
         "{\"plan-tasks\":[\"task1\",\"task2\"],"
-            + 
"\"delete-files\":[{\"spec-id\":0,\"content\":\"POSITION_DELETES\","
+            + 
"\"delete-files\":[{\"spec-id\":0,\"content\":\"position-deletes\","
             + 
"\"file-path\":\"/path/to/data-a-deletes.parquet\",\"file-format\":\"PARQUET\","

Review Comment:
   it seems like file-format is also lower-case : 
   
   
https://github.com/apache/iceberg/blob/35d66a3fe8ad76e1c76b39a898d9477f1da023ae/open-api/rest-catalog-open-api.yaml#L4329-L4335



##########
core/src/main/java/org/apache/iceberg/ContentFileParser.java:
##########
@@ -345,4 +347,24 @@ private static PartitionData partitionFromJson(
 
     return partitionData;
   }
+
+  private static FileContent fileContentFromJson(String content) {
+    Preconditions.checkArgument(content != null, "Invalid file content: null");
+    switch (content) {
+      case "data":
+        return FileContent.DATA;
+      case "position-deletes":
+        return FileContent.POSITION_DELETES;
+      case "equality-deletes":
+        return FileContent.EQUALITY_DELETES;

Review Comment:
   I am not sure why did we go with these constants when we have public enum 
for this class 
   
https://github.com/apache/iceberg/blob/main/api/src/main/java/org/apache/iceberg/FileContent.java#L23
   
   I still deciding if we should align spec to parser impl vs parser impl to 
align with spec.
   
   @amogh-jahagirdar @nastra wdyt ? 



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

Reply via email to