Hi +1 on the default-method approach.
GenericTableCatalog is a real multi-implementer SPI (HiveFederatedCatalogFactory, HadoopFederatedCatalogFactory, BigQueryMetastoreFederatedCatalogFactory all return one from createGenericCatalog(), and GenericTableCatalogHandler holds the result in a field typed as the interface (this.genericTableConfig), set to either a federated impl or a new PolarisGenericTableCatalog(...) depending on whether the catalog is federated. So a pagination method that only exists on the concrete PolarisGenericTableCatalog class wouldn't be reachable from the handler without an instanceof check, the default-method version is the only one of the two options that actually works cleanly through that field. Also worth noting for scoping the change: the REST contract already carries everything needed, generic-tabls-api.yaml declares page-token/page-size request params and a next-page-token field on ListGenericTablesResponse. So no spec change is needed here, this is purely internal plumbing. The two gaps I see: 1. GenericTableCatalogAdopter.listGenericTables(...) already receives pageToken/pageSize as method args but never forwards them to handler.listGenericTables(namespace): they are dropped at that call site. 2. GenericTableCatalogHandler.listGenericTables(Namespace) doesn't set next-page-token on the response either, so even with the SPI change we need to thread the token through the handler and populate the response there. Once the interface has the default method, PolarisGenericTableCatalog can override it the same way LocalIcebergCatalog.listTables(namespace, pageToken) does, and the Hive/Hadoop/BigQuery factories stay untouched until someone wants to add real paging there. Happy to help in review or PRs. Regards JB On Mon, Sep 14, 2026 at 5:15 AM Yong Zheng <[email protected]> wrote: > > Hello Prithvi, > > Thanks for the quick review. I think that is a good plan. Lets wait for a bit > more time in case there are more feedbacks from community before I do the > implementation. > > Thanks, > Yong > > On 2026/09/13 12:05:12 Prithvi S wrote: > > Hi Yong, > > > > Thanks again for splitting this from the policy thread :) Same bug, > > slightly different question because GenericTableCatalog is a polaris-core > > SPI with federation impls (Hive / Hadoop / BigQuery), whereas > > listSemanticModels was not. > > > > I would not replace listGenericTables(Namespace) the way semantic models > > did. That is a breaking SPI change for every federated catalog, and it is > > more than we need to honor the REST API. I’d add a PageToken overload on > > the SPI with a default that keeps today’s behavior: > > > > default Page<TableIdentifier> listGenericTables(Namespace ns, PageToken > > > token) { > > > return Page.fromItems(listGenericTables(ns)); > > > } > > > > > > PolarisGenericTableCatalog can then thread the token into listEntities the > > same way LocalIcebergCatalog does. Federation impls keep compiling and > > still return everything, which matches “server does not support > > pagination". We can give them real paging later if we want. That is option > > 1, just on the interface so the handler does not have to downcast. > > > > WDYT? > > > > Thanks, > > Prithvi S > > > > On Sun, Sep 13, 2026 at 6:25 AM Yong Zheng <[email protected]> wrote: > > > > > Hello, > > > > > > I would like to start this ML for discussing if we should implement the > > > pagination support for generic table API (in this case, > > > /polaris/v1/{prefix}/namespaces/{namespace}/generic-tables). Here is a > > > reference report issue for this matter: > > > https://github.com/apache/polaris/issues/5311. > > > > > > Looking at the what we currently have as of today, the parameters reach > > > the adapter then got dropped. We are calling PageToken.readEverything() > > > regardless ( > > > https://github.com/apache/polaris/blob/main/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalog.java#L196 > > > ). > > > > > > Now to honor this, I think we have two approaches that we can consider: > > > > > > option 1: No change to SPI and use method overload instead > > > > > > This would be similar to what we are doing in LocalIcebergCatalog where we > > > will be adding Page listTables(namespace, pageToken) via method overload. > > > Here is the reference: > > > https://github.com/apache/polaris/blame/main/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java#L632 > > > > > > option 2: Change to SPI with listGenericTables(namespace, pageToken) > > > > > > This would be similar to listSemanticModels which got implemented > > > recently. Here is the reference: > > > https://github.com/apache/polaris/blob/48126190377cfb2d0a54b4b9905a7375fde457ec/extensions/semantic-models/src/main/java/org/apache/polaris/service/catalog/semanticmodel/SemanticModelCatalogAdapter.java#L92 > > > > > > Thanks, > > > Yong > > > > >
