[I] REST Catalog Spec: Snapshot Summary Class [iceberg]

[via GitHub]

haizhou-zhao opened a new issue, #9837:
URL: https://github.com/apache/iceberg/issues/9837

   ### Apache Iceberg version
   
   1.4.3 (latest release)
   
   ### Query engine
   
   None
   
   ### Please describe the bug 🐞
   
   ## Summary
   This is a report on error made in REST Spec's open api definition. 
Specifically, on Snapshot Summary model.
   
   Snapshot Summary is a composition of `operation` and many other key-value 
pairs, which we are using the OpenAPI `additionalProperties` keyword to achieve.
   
   However, based on OpenAPI documentation, `additionalProperties` will only be 
treated as a keyword if it is on the same level as other model attributes like 
`type` and `properties`; otherwise, it will be treated as a common string. Ref: 
https://swagger.io/docs/specification/data-models/dictionaries/
   
   ## Relevant code reference
   
   
https://github.com/apache/iceberg/blob/acbf96f4fe5586fc7c7b6840eb19ff492e82c756/open-api/rest-catalog-open-api.yaml#L1975
   
   ## Tests
   
   Using OpenAPI's JavaGenerator to demonstrate the difference (tested on 
OpenAPI JavaGenerator 6.6.0 and 7.0.0):
   
   With the current open-api definition on the main branch, we generate this 
`SnapshotSummary` Java Class:
   ```
   ...
   public class SnapshotSummary {
     ...
     public static final String SERIALIZED_NAME_OPERATION = "operation";
     @SerializedName(SERIALIZED_NAME_OPERATION)
     private OperationEnum operation;
   
     public static final String SERIALIZED_NAME_ADDITIONAL_PROPERTIES = 
"additionalProperties";
     @SerializedName(SERIALIZED_NAME_ADDITIONAL_PROPERTIES)
     private String additionalProperties;
     ...
   }
   ```
   
   But if we bring the `additionalProperties` field up one level (on the same 
level as `properties` and other model attributes), then we generate this 
`SnapshotSummary` Java Class:
   ```
   ...
   public class SnapshotSummary extends HashMap<String, String> {
     ...
     public static final String JSON_PROPERTY_OPERATION = "operation";
     private OperationEnum operation;
     ...
     private Map<String, String> additionalProperties;
   }
   ```
   
   Subsequent test shows that the first version of Java Class cannot be used 
for parsing actual SnapshotSummary properly with all its key value pairs, while 
the second can.
   
   ---
   
   Additional test has been done using OpenAPI's python generator. When 
generating on the main branch open-api definition as is, here's the result
   ```
   class SnapshotSummary(BaseModel):
       """
       SnapshotSummary
       """
       operation: StrictStr = Field(...)
       additional_properties: Optional[StrictStr] = Field(None, 
alias="additionalProperties")
       __properties = ["operation", "additionalProperties"]
   ...
   ```
   
   With the same change proposed above, we get:
   ```
   ...
   class SnapshotSummary(BaseModel):
       """
       SnapshotSummary
       """
       operation: StrictStr = Field(...)
       additional_properties: Dict[str, Any] = {}
       __properties = ["operation"]
   ...
   ```


-- 
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: issues-unsubscr...@iceberg.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org