You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "zeroshade (via GitHub)" <gi...@apache.org> on 2025/05/21 16:10:22 UTC
[I] REST Catalog fixture is particular about query params [iceberg]
zeroshade opened a new issue, #13119:
URL: https://github.com/apache/iceberg/issues/13119
### Apache Iceberg version
1.9.0 (latest release)
### Query engine
None
### Please describe the bug 🐞
I was testing the Iceberg CLI built using https://github.com/apache/iceberg-go while working on the new release along with @kevinjqliu and we discovered an issue in the iceberg-rest-fixture. It turns out that if you set a `pageSize` then you *have* to also pass `pageToken` even for the initial request, otherwise you get an exception that it failed to parse an integer.
The exception stack looks like the following:
With a bit of digging, the issue appears to be here: https://github.com/apache/iceberg/blob/apache-iceberg-1.9.0/core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java#L114
On that line, we do `INITIAL_PAGE_TOKEN.equals(pageToken)` where `INITIAL_PAGE_TOKEN` is an empty string. But if you don't pass a `pageToken` query parameter (which you wouldn't do on the first request, you'd only use it on subsequent requests) then the check fails because `pageToken` is `null`. Leading to `Integer.parseInt(pageToken)` throwing an exception because of the null string. The fix is likely just to add a null check there.
### Willingness to contribute
- [ ] I can contribute a fix for this bug independently
- [ ] I would be willing to contribute a fix for this bug with guidance from the Iceberg community
- [ ] I cannot contribute a fix for this bug at this time
--
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.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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "elphastori (via GitHub)" <gi...@apache.org>.
elphastori commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2906957307
> Specifically when pageToken is omitted and pageSize is set (http://localhost:8181/v1/namespaces/default/tables?pageSize=1), the server with pagination support should return at most 1 record
Agreed! If we can merge the current open PR I'll consider opening another one to clarify the spec.
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2898703130
heres the relevant code on the go side
[Catalog.ListTables](https://github.com/apache/iceberg-go/blob/8270498f1932bececb5cec22c174df5f1865f7eb/catalog/rest/rest.go#L666-L670)
[getPageSize](https://github.com/apache/iceberg-go/blob/8270498f1932bececb5cec22c174df5f1865f7eb/catalog/rest/rest.go#L1115-L1120) defaults to 20
[pageSize param is set, while pageToken is not](https://github.com/apache/iceberg-go/blob/8270498f1932bececb5cec22c174df5f1865f7eb/catalog/rest/rest.go#L696-L702)
Works: http://localhost:8181/v1/namespaces/default/tables?pageSize=1&pageToken=1
Doesn't work: http://localhost:8181/v1/namespaces/default/tables?pageSize=1
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2936808986
With #13129 merged, I've rebuild the [apache/iceberg-rest-fixture](https://hub.docker.com/r/apache/iceberg-rest-fixture) by triggering the github action, https://github.com/apache/iceberg/actions/runs/15425627699
Verified locally that http://localhost:8181/v1/namespaces/default/tables?pageSize=1 now works.
Thanks everyone!
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2899022165
@elphastori Go for it! I'm not a java guy myself, so it would be better for someone else (like yourself!) to give it a try :smile:
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "elphastori (via GitHub)" <gi...@apache.org>.
elphastori commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2906092762
@kevinjqliu Thanks for reviewing the PR! We clearly agree that omitting `pageToken` when `pageSize` is specified should work, and the PR will fix that.
Although the spec states that `pageToken` is optional, just like `pageSize`, it also indicates that it should be included for paginated requests, including the first one.
The wording might be a bit confusing, as it seems to rely on a Java-specific concept of an empty string. Either way, this fix should help reduce that confusion.
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2898703967
sure @elphastori feel free to submit a PR!
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "elphastori (via GitHub)" <gi...@apache.org>.
elphastori commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2931317681
@nastra Yes, I would like to do this (with some guidance of course!)
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2929697615
> Specifically when `pageToken` is omitted and `pageSize` is set (`http://localhost:8181/v1/namespaces/default/tables?pageSize=1`), the server with pagination support should return at most 1 record
I feel like the Impl currently behaves as defined by the Spec (although the behavior isn't dev friendly). The spec says
> Clients may initiate the first paginated request by sending an empty query parameter `pageToken` to the server
Looking deeper into this, I believe we wanted to disambiguate what **empty** means via `allowEmptyValue`, but I agree that this rather adds more confusion around the meaning of **empty** and there's https://github.com/OAI/OpenAPI-Specification/issues/1573 that essentially arrives at
> In PR https://github.com/OAI/OpenAPI-Specification/issues/1573, targeted for the 3.0.2 patch revision, we have made use of allowEmptyValue NOT RECOMMENDED. Consensus on the TSC is that this feature has ambiguous meanings, and interacts in awkward ways with Schema Object and Parameter style. See previous comments in this issue thread for more details.
> We can revisit in OpenAPI 4.x if there's a good use case.
That being said, we should add a DISCUSS thread on the mailing list to remove usage of `allowEmptyValue` (due to https://github.com/OAI/OpenAPI-Specification/pull/1632) and to disambiguate the behavior. @elphastori could you please do that?
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "elphastori (via GitHub)" <gi...@apache.org>.
elphastori commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2901144875
@kevinjqliu Thanks for the context, I confirmed that the issue also occurs while paginating `namespaces` and `views`:
- http://localhost:8181/v1/namespaces?pageSize=1
- http://localhost:8181/v1/namespaces/default/views?pageSize=1
The null check should fix all three cases. The PR is on the way!
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "elphastori (via GitHub)" <gi...@apache.org>.
elphastori commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2898603478
Hi @zeroshade , can I pick this up? I'm pretty new to Iceberg.
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "elphastori (via GitHub)" <gi...@apache.org>.
elphastori commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2904808219
@nastra I've opened the PR ([#13129](https://github.com/apache/iceberg/pull/13129)) with the fix/improvement. Could you please take a look?
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2906306952
I feel like I'm not understanding the improvement here, but feel free to open a PR to clarify the spec!
These are the options to consider:
```
http://localhost:8181/v1/namespaces/default/tables
http://localhost:8181/v1/namespaces/default/tables?pageToken=
http://localhost:8181/v1/namespaces/default/tables?pageToken=1
http://localhost:8181/v1/namespaces/default/tables?pageSize=
http://localhost:8181/v1/namespaces/default/tables?pageSize=1
http://localhost:8181/v1/namespaces/default/tables?pageSize=1&pageToken=
http://localhost:8181/v1/namespaces/default/tables?pageToken=1&pageSize=
http://localhost:8181/v1/namespaces/default/tables?pageSize=1&pageToken=1
```
Specifically when `pageToken` is omitted and `pageSize` is set (`http://localhost:8181/v1/namespaces/default/tables?pageSize=1`), the server with pagination support should return at most 1 record
```
page-size:
name: pageSize
in: query
description:
For servers that support pagination, this signals an upper bound of the number of results that a client will receive.
For servers that do not support pagination, clients may receive results larger than the indicated `pageSize`.
required: false
schema:
type: integer
minimum: 1
```
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2901811346
Nice i think those should cover all of them. I double checked with the rest spec
* [list namespace](https://github.com/apache/iceberg/blob/91dff9886e6e6c494f6a970129f4c08487c98a0d/open-api/rest-catalog-open-api.yaml#L257)
* [list table](https://github.com/apache/iceberg/blob/91dff9886e6e6c494f6a970129f4c08487c98a0d/open-api/rest-catalog-open-api.yaml#L518)
* [list view](https://github.com/apache/iceberg/blob/91dff9886e6e6c494f6a970129f4c08487c98a0d/open-api/rest-catalog-open-api.yaml#L1455)
PageToken allows null value, which is the issue here
https://github.com/apache/iceberg/blob/91dff9886e6e6c494f6a970129f4c08487c98a0d/open-api/rest-catalog-open-api.yaml#L1879-L1885
https://github.com/apache/iceberg/blob/91dff9886e6e6c494f6a970129f4c08487c98a0d/open-api/rest-catalog-open-api.yaml#L2018C1-L2041C21
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "elphastori (via GitHub)" <gi...@apache.org>.
elphastori commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2904798383
Thanks for linking the [rest spec](https://github.com/apache/iceberg/blob/91dff9886e6e6c494f6a970129f4c08487c98a0d/open-api/rest-catalog-open-api.yaml#L2018C1-L2041C21).
> An opaque token that allows clients to make use of pagination for list APIs
> (e.g. ListTables). Clients may initiate the first paginated request by sending an empty
> query parameter `pageToken` to the server.
Based on this description, the REST catalog is technically behaving as expected.
- Works: http://localhost:8181/v1/namespaces/default/tables?pageSize=1&pageToken=
- Doesn't work: http://localhost:8181/v1/namespaces/default/tables?pageSize=1
The API expects the first request to have an *empty* `pageToken`, rather than omitting the parameter entirely. While this aligns with the current spec, it’s not the most intuitive developer experience.
Ideally, I would suggest updating the spec description to something like:
> Clients can initiate the first paginated request *either* without the `pageToken` query parameter or with an empty one.
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2905336288
> The API expects the first request to have an empty pageToken, rather than omitting the parameter entirely. While this aligns with the current spec, it’s not the most intuitive developer experience.
Since `pageToken` is optional, it can be omitted. Omitted means i dont want pagination.
I still expect http://localhost:8181/v1/namespaces/default/tables?pageSize=1 to work though
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "elphastori (via GitHub)" <gi...@apache.org>.
elphastori commented on issue #13119:
URL: https://github.com/apache/iceberg/issues/13119#issuecomment-2960019346
Thanks @nastra , I've started a DISCUSS thread: https://lists.apache.org/thread/pg2obnmlhkdfg2gtbd0xo4j9zqwlcxf4
--
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: [I] REST Catalog fixture is particular about query params [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu closed issue #13119: REST Catalog fixture is particular about query params
URL: https://github.com/apache/iceberg/issues/13119
--
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