aokolnychyi commented on a change in pull request #1879:
URL: https://github.com/apache/iceberg/pull/1879#discussion_r537401615
##########
File path: core/src/main/java/org/apache/iceberg/CachingCatalog.java
##########
@@ -160,4 +159,90 @@ private void invalidate(TableIdentifier ident) {
return builder.build();
}
+
+ @Override
+ public TableBuilder buildTable(TableIdentifier identifier, Schema schema) {
+ return new CachingTableBuilder(identifier, schema);
+ }
+
+ private class CachingTableBuilder implements TableBuilder {
+ private final TableIdentifier ident;
+ private final TableBuilder innerBuilder;
+
+ private CachingTableBuilder(TableIdentifier identifier, Schema schema) {
+ this.innerBuilder = catalog.buildTable(identifier, schema);
+ this.ident = identifier;
+ }
+
+ @Override
+ public TableBuilder withPartitionSpec(PartitionSpec spec) {
+ innerBuilder.withPartitionSpec(spec);
+ return this;
+ }
+
+ @Override
+ public TableBuilder withSortOrder(SortOrder sortOrder) {
+ innerBuilder.withSortOrder(sortOrder);
+ return this;
+ }
+
+ @Override
+ public TableBuilder withLocation(String location) {
+ innerBuilder.withLocation(location);
+ return this;
+ }
+
+ @Override
+ public TableBuilder withProperties(Map<String, String> properties) {
+ innerBuilder.withProperties(properties);
+ return this;
+ }
+
+ @Override
+ public TableBuilder withProperty(String key, String value) {
+ innerBuilder.withProperty(key, value);
+ return this;
+ }
+
+ @Override
+ public Table create() {
+ AtomicBoolean created = new AtomicBoolean(false);
+ Table table = tableCache.get(canonicalizeIdentifier(ident), identifier
-> {
+ created.set(true);
+ return innerBuilder.create();
+ });
+
+ if (!created.get()) {
+ throw new AlreadyExistsException("Table already exists: %s", ident);
+ }
+
+ return table;
+ }
+
+ @Override
+ public Transaction createTransaction() {
+ // create a new transaction without altering the cache. the table
doesn't exist until the transaction is
+ // committed. if the table is created before the transaction commits,
any cached version is correct and the
+ // transaction create will fail. if the transaction commits before
another create, then the cache will be empty.
+ return innerBuilder.createTransaction();
+ }
+
+ @Override
+ public Transaction replaceTransaction() {
+ // create a new transaction without altering the cache. the table
doesn't change until the transaction is
+ // committed. when the transaction commits, invalidate the table in the
cache if it is present.
+ return CommitCallbackTransaction.addCallback(
+ innerBuilder.replaceTransaction(),
+ () -> tableCache.invalidate(canonicalizeIdentifier(ident)));
Review comment:
Hm, is this correct? We are calling `invalidate` on `tableCache`
directly instead of the dedicated method in `CachingCatalog` that also
invalidates metadata tables.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]