REST Catalog fixture is particular about query params

[zeroshade]

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

[elphastori]

Hi @zeroshade , can I pick this up? I'm pretty new to Iceberg.

[kevinjqliu]

heres the relevant code on the go side

Catalog.ListTables
getPageSize defaults to 20
pageSize param is set, while pageToken is not

Works: http://localhost:8181/v1/namespaces/default/tables?pageSize=1&pageToken=1
Doesn't work: http://localhost:8181/v1/namespaces/default/tables?pageSize=1

[kevinjqliu]

sure @elphastori feel free to submit a PR!

[zeroshade]

@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 😄

[elphastori]

@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!

[kevinjqliu]

Nice i think those should cover all of them. I double checked with the rest spec

• list namespace
• list table
• list view

PageToken allows null value, which is the issue here

iceberg/open-api/rest-catalog-open-api.yaml

Lines 1879 to 1885 in 91dff98

	page-token:
	name: pageToken
	in: query
	required: false
	allowEmptyValue: true
	schema:
	$ref: '#/components/schemas/PageToken'

https://github.com/apache/iceberg/blob/91dff9886e6e6c494f6a970129f4c08487c98a0d/open-api/rest-catalog-open-api.yaml#L2018C1-L2041C21

[elphastori]

Thanks for linking the rest spec.

   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.

[elphastori]

@nastra I've opened the PR (#13129) with the fix/improvement. Could you please take a look?

[kevinjqliu]

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

[elphastori]

@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.

[kevinjqliu]

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