You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "danielcweeks (via GitHub)" <gi...@apache.org> on 2025/01/24 16:07:29 UTC
[PR] OpenAPI: add initial/write defaults to schema [iceberg]
danielcweeks opened a new pull request, #12094:
URL: https://github.com/apache/iceberg/pull/12094
Adds `inital-default` and `write-default` to the REST API spec.
Includes updates to a couple small round-trip request/response serde tests.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: add initial/write defaults to schema [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks merged PR #12094:
URL: https://github.com/apache/iceberg/pull/12094
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: add initial/write defaults to schema [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #12094:
URL: https://github.com/apache/iceberg/pull/12094#discussion_r1929384425
##########
core/src/test/java/org/apache/iceberg/rest/requests/TestCreateTableRequest.java:
##########
@@ -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},"
Review 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
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: add initial/write defaults to schema [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #12094:
URL: https://github.com/apache/iceberg/pull/12094#discussion_r1929207089
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2052,6 +2052,10 @@ components:
type: boolean
doc:
type: string
+ initial-default:
+ type: string
+ write-default:
+ type: string
Review Comment:
✅
"""
By default, OpenAPI treats all request parameters as optional.
"""
from
https://swagger.io/docs/specification/v3_0/describing-parameters/#required-and-optional-parameters
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: add initial/write defaults to schema [iceberg]
Posted by "gaborkaszab (via GitHub)" <gi...@apache.org>.
gaborkaszab commented on code in PR #12094:
URL: https://github.com/apache/iceberg/pull/12094#discussion_r1930181443
##########
core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponse.java:
##########
@@ -147,6 +147,23 @@ public void testRoundTripSerdeWithV2TableMetadata() throws Exception {
assertRoundTripSerializesEquallyFrom(json, resp);
}
+ @Test
+ public void testRoundTripSerdeWithV3TableMetadata() throws Exception {
+ String tableMetadataJson = readTableMetadataInputFile("TableMetadataV3ValidMinimal.json");
+ 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
Review Comment:
nit: this line could be merged with the one below
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: add initial/write defaults to schema [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #12094:
URL: https://github.com/apache/iceberg/pull/12094#discussion_r1933030915
##########
core/src/test/java/org/apache/iceberg/rest/responses/TestLoadTableResponse.java:
##########
@@ -147,6 +147,23 @@ public void testRoundTripSerdeWithV2TableMetadata() throws Exception {
assertRoundTripSerializesEquallyFrom(json, resp);
}
+ @Test
+ public void testRoundTripSerdeWithV3TableMetadata() throws Exception {
+ String tableMetadataJson = readTableMetadataInputFile("TableMetadataV3ValidMinimal.json");
+ 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
Review Comment:
Yeah, this was borrowed from above and looks like an auto-formatter issue. Fixed.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: add initial/write defaults to schema [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #12094:
URL: https://github.com/apache/iceberg/pull/12094#discussion_r1933029579
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2052,6 +2052,10 @@ components:
type: boolean
doc:
type: string
+ initial-default:
+ type: string
Review Comment:
Nice catch, updated
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] OpenAPI: add initial/write defaults to schema [iceberg]
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #12094:
URL: https://github.com/apache/iceberg/pull/12094#discussion_r1929383792
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -2052,6 +2052,10 @@ components:
type: boolean
doc:
type: string
+ initial-default:
+ type: string
Review 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"`.
--
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-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org