You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "Fokko (via GitHub)" <gi...@apache.org> on 2023/04/27 07:12:23 UTC
[GitHub] [iceberg] Fokko opened a new pull request, #7445: Spec: Add missing `last-column-id`
Fokko opened a new pull request, #7445:
URL: https://github.com/apache/iceberg/pull/7445
I noticed that this was required:
```
com.fasterxml.jackson.databind.JsonMappingException: Cannot parse missing int: last-column-id (through reference chain: org.apache.iceberg.rest.requests.UpdateTableRequest["updates"]->java.util.ArrayList[0])
at com.fasterxml.jackson.databind.JsonMappingException.wrapWithPath(JsonMappingException.java:402)
at com.fasterxml.jackson.databind.JsonMappingException.wrapWithPath(JsonMappingException.java:373)
at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer._deserializeFromArray(CollectionDeserializer.java:375)
at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:244)
at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer.deserialize(CollectionDeserializer.java:28)
at com.fasterxml.jackson.databind.deser.impl.FieldProperty.deserializeAndSet(FieldProperty.java:138)
at com.fasterxml.jackson.databind.deser.BeanDeserializer.vanillaDeserialize(BeanDeserializer.java:314)
at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:177)
at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:323)
at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:4730)
at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:3690)
at org.apache.iceberg.rest.RESTCatalogServlet$ServletRequestContext.from(RESTCatalogServlet.java:179)
at org.apache.iceberg.rest.RESTCatalogServlet.doPost(RESTCatalogServlet.java:78)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:707)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:799)
at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:550)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
at org.eclipse.jetty.server.handler.gzip.GzipHandler.handle(GzipHandler.java:713)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:235)
at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1434)
at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188)
at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:501)
at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186)
at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1349)
at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)
at org.eclipse.jetty.server.Server.handle(Server.java:516)
at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:388)
at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:633)
at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:380)
at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:277)
at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)
at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105)
at org.eclipse.jetty.io.ChannelEndPoint$1.run(ChannelEndPoint.java:104)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:338)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:315)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:173)
at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:131)
at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:386)
at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:883)
at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1034)
at java.base/java.lang.Thread.run(Thread.java:833)
Caused by: java.lang.IllegalArgumentException: Cannot parse missing int: last-column-id
at org.apache.iceberg.relocated.com.google.common.base.Preconditions.checkArgument(Preconditions.java:220)
at org.apache.iceberg.util.JsonUtil.getInt(JsonUtil.java:108)
at org.apache.iceberg.MetadataUpdateParser.readAddSchema(MetadataUpdateParser.java:400)
at org.apache.iceberg.MetadataUpdateParser.fromJson(MetadataUpdateParser.java:245)
at org.apache.iceberg.rest.RESTSerializers$MetadataUpdateDeserializer.deserialize(RESTSerializers.java:130)
at org.apache.iceberg.rest.RESTSerializers$MetadataUpdateDeserializer.deserialize(RESTSerializers.java:125)
at com.fasterxml.jackson.databind.deser.std.CollectionDeserializer._deserializeFromArray(CollectionDeserializer.java:359)
... 41 more
```
Java code:
https://github.com/apache/iceberg/blob/882459d488a3fae73eda5b1f09f18d1af9fe6f51/core/src/main/java/org/apache/iceberg/MetadataUpdateParser.java#L397-L402
--
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
[GitHub] [iceberg] rdblue commented on a diff in pull request #7445: Spec: Add missing `last-column-id`
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #7445:
URL: https://github.com/apache/iceberg/pull/7445#discussion_r1218480422
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1500,6 +1500,9 @@ components:
properties:
schema:
$ref: '#/components/schemas/Schema'
+ last-column-id:
+ type: integer
+ description: The highest assigned column ID for the table. This is used to ensure columns are always assigned an unused ID when evolving schemas.
Review Comment:
I'd say let's make it optional and document what Java does, but not force people to do the same.
--
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
[GitHub] [iceberg] rdblue commented on a diff in pull request #7445: Spec: Add missing `last-column-id`
Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #7445:
URL: https://github.com/apache/iceberg/pull/7445#discussion_r1179703697
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1500,6 +1500,9 @@ components:
properties:
schema:
$ref: '#/components/schemas/Schema'
+ last-column-id:
+ type: integer
+ description: The highest assigned column ID for the table. This is used to ensure columns are always assigned an unused ID when evolving schemas.
Review Comment:
This isn't technically required in the OpenAPI spec. We send it in the Java client because we already have it. But there isn't really a requirement for it because the server side can find `max(id for id in schema)`
--
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
[GitHub] [iceberg] Fokko commented on a diff in pull request #7445: Spec: Add missing `last-column-id`
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #7445:
URL: https://github.com/apache/iceberg/pull/7445#discussion_r1218547465
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1500,6 +1500,9 @@ components:
properties:
schema:
$ref: '#/components/schemas/Schema'
+ last-column-id:
+ type: integer
+ description: The highest assigned column ID for the table. This is used to ensure columns are always assigned an unused ID when evolving schemas.
Review Comment:
```suggestion
description: The highest assigned column ID for the table. This is used to ensure columns are always assigned an unused ID when evolving schemas. When omitted, it will be computed on the server side.
```
--
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
[GitHub] [iceberg] Fokko merged pull request #7445: Spec: Add missing `last-column-id`
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #7445:
URL: https://github.com/apache/iceberg/pull/7445
--
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
[GitHub] [iceberg] Fokko commented on a diff in pull request #7445: Spec: Add missing `last-column-id`
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #7445:
URL: https://github.com/apache/iceberg/pull/7445#discussion_r1181289705
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1500,6 +1500,9 @@ components:
properties:
schema:
$ref: '#/components/schemas/Schema'
+ last-column-id:
+ type: integer
+ description: The highest assigned column ID for the table. This is used to ensure columns are always assigned an unused ID when evolving schemas.
Review Comment:
Got it. It is also not required in the OpenAPI spec. I updated the Java side as well to make it optional. We can also omit it from the spec entirely, I don't have a strong opinion here.
--
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
[GitHub] [iceberg] Fokko commented on pull request #7445: Spec: Add missing `last-column-id`
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #7445:
URL: https://github.com/apache/iceberg/pull/7445#issuecomment-1530345012
@nastra Excellent idea, added a test! 👍🏻
--
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
[GitHub] [iceberg] nastra commented on pull request #7445: Spec: Add missing `last-column-id`
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #7445:
URL: https://github.com/apache/iceberg/pull/7445#issuecomment-1525626500
just an FYI that there's also https://github.com/apache/iceberg/pull/6701 to address the same
--
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
[GitHub] [iceberg] Fokko commented on pull request #7445: Spec: Add missing `last-column-id`
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #7445:
URL: https://github.com/apache/iceberg/pull/7445#issuecomment-1577523848
Thanks @nastra, @dramaticlly, @singhpk234 and @rdblue for the review! 🙌🏻
--
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