Abhishek-Gawande commented on code in PR #17672:
URL: https://github.com/apache/iceberg/pull/17672#discussion_r3813317969


##########
core/src/main/java/org/apache/iceberg/SingleValueParser.java:
##########
@@ -410,6 +414,9 @@ public static void toJson(Type type, Object defaultValue, 
JsonGenerator generato
         }
         generator.writeEndObject();
         break;
+      case UNKNOWN:
+        generator.writeString(String.valueOf(defaultValue));

Review Comment:
   Thanks for the review @anoopj
   
   So the null case is actually already handled.if defaultValue is null, we 
write null at the top of toJson() (before the switch) and return early. The 
UNKNOWN case only runs for non-null values.
   
   The scenario where this matters is partition values for dropped columns. 
When a source column gets dropped, PartitionSpec.resultType() falls back to 
UnknownType, but the existing data files still have the original partition 
values (like 4888 from a date column). If we unconditionally wrote null here, 
those values would get lost during REST serialization and break the /plan 
response.
   
   Since unknown isn't listed in the JSON single-value serialization table 
(Appendix D), there's no spec-defined format for it . I went with string as a 
lossless fallback. The fromJson side parses it back as a string too, so 
round-tripping works.



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