mxm commented on code in PR #18582:
URL: https://github.com/apache/iceberg/pull/18582#discussion_r4334981709
##########
flink/v2.3/flink/src/main/java/org/apache/iceberg/flink/sink/dynamic/TableUpdater.java:
##########
@@ -78,14 +79,36 @@ private void findOrCreateTable(
TableIdentifier identifier, Schema schema, PartitionSpec spec,
TableCreator tableCreator) {
Tuple2<Boolean, Exception> exists = cache.exists(identifier);
if (Boolean.FALSE.equals(exists.f0)) {
- if (exists.f1 instanceof NoSuchNamespaceException) {
+ if (exists.f1 != null
+ && !(exists.f1 instanceof NoSuchTableException)
+ && !(exists.f1 instanceof NoSuchNamespaceException)) {
+ throw new IllegalStateException(
+ String.format("Unexpected failure looking up table %s",
identifier), exists.f1);
+ }
+
+ // Create the namespace if it is missing. Tasks that get here at the
same time can all find it
+ // missing and try to create it, which is not an error. The lookup of
the table is not used to
+ // tell: which exception it throws for a missing namespace is up to the
catalog (Glue throws
+ // NoSuchTableException), and an answer from the cache carries none.
+ if (catalog instanceof SupportsNamespaces) {
SupportsNamespaces catalogWithNameSpace = (SupportsNamespaces) catalog;
- LOG.info("Namespace {} not found during table search. Creating
namespace", identifier);
- try {
- catalogWithNameSpace.createNamespace(identifier.namespace());
- } catch (AlreadyExistsException e) {
- LOG.debug("Namespace {} created concurrently",
identifier.namespace(), e);
+ // A table without a namespace has none to create, and some catalogs,
like the Hadoop
+ // catalog, do not accept the empty namespace in the call.
+ if (!identifier.namespace().isEmpty()
+ && !catalogWithNameSpace.namespaceExists(identifier.namespace())) {
+ LOG.info("Namespace {} not found during table search. Creating
namespace", identifier);
+ try {
+ catalogWithNameSpace.createNamespace(identifier.namespace());
+ } catch (AlreadyExistsException e) {
+ LOG.debug("Namespace {} created concurrently",
identifier.namespace(), e);
+ }
}
+ } else {
+ throw new UnsupportedOperationException(
+ String.format(
+ "Cannot create table %s: catalog %s does not support
namespaces, so the namespace of the table cannot be created if it is missing",
+ identifier, catalog.name()),
+ exists.f1);
Review Comment:
Let's move this into a `Precondition.checkState(catalog instanceof
SupportsNamespace, ...);` check before line 93. We can then remove the if
condition.
--
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]
