Baunsgaard commented on code in PR #16072:
URL: https://github.com/apache/iceberg/pull/16072#discussion_r3234990680
##########
kafka-connect/kafka-connect/src/main/java/org/apache/iceberg/connect/data/IcebergWriterFactory.java:
##########
@@ -134,14 +135,17 @@ static void createNamespaceIfNotExist(Catalog catalog,
Namespace identifierNames
return;
}
+ SupportsNamespaces nsCatalog = (SupportsNamespaces) catalog;
String[] levels = identifierNamespace.levels();
for (int index = 0; index < levels.length; index++) {
Namespace namespace = Namespace.of(Arrays.copyOfRange(levels, 0, index +
1));
- try {
- ((SupportsNamespaces) catalog).createNamespace(namespace);
- } catch (AlreadyExistsException | ForbiddenException ex) {
- // Ignoring the error as forcefully creating the namespace even if it
exists
- // to avoid double namespaceExists() check.
+ if (!nsCatalog.namespaceExists(namespace)) {
+ try {
+ nsCatalog.createNamespace(namespace);
+ } catch (AlreadyExistsException | ForbiddenException |
NotAuthorizedException ex) {
+ // Namespace may have been created concurrently, or the user lacks
create permission
+ // but the namespace already exists. Either way, proceed gracefully.
Review Comment:
This seems wrong to me. We should at minimum do a `LOG.warning()` to
highlight that we have an conflict. I do see the old code did not do any
logging, but if we add it we can get some statistics on how often this error is
encountered.
--
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]
