eric-maynard commented on code in PR #1555:
URL: https://github.com/apache/polaris/pull/1555#discussion_r2085101838
##########
extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java:
##########
@@ -414,6 +415,11 @@ public <T> Page<T> listEntities(
// Limit can't be pushed down, due to client side filtering
// absence of transaction.
String query = QueryGenerator.generateSelectQuery(new ModelEntity(),
params);
+
+ if (pageToken instanceof EntityIdPageToken entityIdPageToken) {
+ query += String.format(" AND id > %d ORDER BY id ASC",
entityIdPageToken.getId());
Review Comment:
I spoke with @singhpk234 who noted this is probably the same discussion as
[here on the old
PR](https://github.com/apache/polaris/pull/273#discussion_r2021548762).
I think it's alright that the list ordering you'd get across metastores
won't be the same. Besides that, I think it will work fine with the JDBC
`IdGenerator`. Although the IDs aren't generated sequentially, pagination only
uses the entity ID as an essentially arbitrary consistent ordering.
The key implication here is that if an entity gets created in the middle of
a listing operation (e.g. between list calls 2 and 3) it may or may not show up
in the next page. An alternative would be to try to filter it out so that the
behavior is more obvious & consistent, but I think the simple approach that
ultimately gives the user a chance to see these new entities is good.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
