OpenAPI: add initial/write defaults to schema #12094
Conversation
There was a problem hiding this comment.
LGTM! Aligns with V3 default values from the spec
https://iceberg.apache.org/spec/#default-values
| initial-default: | ||
| type: string | ||
| write-default: | ||
| type: string |
There was a problem hiding this comment.
✅
"""
By default, OpenAPI treats all request parameters as optional.
"""
from
https://swagger.io/docs/specification/v3_0/describing-parameters/#required-and-optional-parameters
| @@ -2052,6 +2052,10 @@ components: | |||
| type: boolean | |||
| doc: | |||
| type: string | |||
| initial-default: | |||
| type: string | |||
There was a problem hiding this comment.
I don't think the type should be string. The table spec serializes these using the JSON single-value serialization, so it would be $ref: "#/components/schemas/PrimitiveTypeValue".
| @@ -59,7 +60,7 @@ public class TestCreateTableRequest extends RequestResponseTestBase<CreateTableR | |||
| public void testRoundTripSerDe() throws JsonProcessingException { | |||
| String fullJsonRaw = | |||
| "{\"name\":\"test_tbl\",\"location\":\"file://tmp/location/\",\"schema\":{\"type\":\"struct\"," | |||
| + "\"schema-id\":0,\"fields\":[{\"id\":1,\"name\":\"id\",\"required\":true,\"type\":\"int\"}," | |||
| + "\"schema-id\":0,\"fields\":[{\"id\":1,\"name\":\"id\",\"required\":true,\"type\":\"int\",\"write-default\":1}," | |||
There was a problem hiding this comment.
This demonstrates my comment below. The default here is not serialized as a string, it is a JSON value.
Here's where it is produced: https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/SchemaParser.java#L96
| TableMetadata v3Metadata = | ||
| TableMetadataParser.fromJson(TEST_METADATA_LOCATION, tableMetadataJson); | ||
| // Convert the TableMetadata JSON from the file to an object and then back to JSON so that | ||
| // missing fields |
There was a problem hiding this comment.
nit: this line could be merged with the one below
Adds
inital-defaultandwrite-defaultto the REST API spec.Includes updates to a couple small round-trip request/response serde tests.