gaborkaszab commented on code in PR #14398:
URL: https://github.com/apache/iceberg/pull/14398#discussion_r2609861237
##########
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:
##########
@@ -474,20 +601,44 @@ public Table loadTable(SessionContext context,
TableIdentifier identifier) {
tableMetadata,
endpoints);
- trackFileIO(ops);
-
BaseTable table =
new BaseTable(
ops,
fullTableName(finalIdentifier),
metricsReporter(paths.metrics(finalIdentifier), tableClient));
+
+ String eTag = responseHeaders.getOrDefault(HttpHeaders.ETAG, null);
+ if (eTag != null) {
+ BaseTable clonedTable = cloneTable(table);
+
+ trackFileIO((RESTTableOperations) clonedTable.operations());
+
+ tableCache.put(
Review Comment:
Hey @singhpk234 ,
Let me invite you to review this PR, because I saw you had an overlapping
change on this area with introducing the concept of RESTTable and server-side
planning. I'm resolving merge conflicts here in the code and it made me
wondering what should be the strategy for interworking the two functionalities:
server-side planning and freshness-aware loading .
If I'm not mistaken, RESTTable is also a regular table like BaseTable, the
difference is that some scan APIs are overridden to go to a different route
when planning a query, right? Following the logic, it would still make sense to
have ETags for such tables and cache them with freshness-aware loading
functionality.
Would be great to hear your take on this! Thanks!
(I'll come back with the resolved code to work with both features soon)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
