ebyhr commented on code in PR #18760:
URL: https://github.com/apache/iceberg/pull/18760#discussion_r4464006811


##########
core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java:
##########
@@ -474,7 +474,17 @@ public void createNamespace(Namespace namespace, 
Map<String, String> metadata) {
               .buildOrThrow();
     }
 
-    insertProperties(namespace, createMetadata);
+    try {
+      insertProperties(namespace, createMetadata);
+    } catch (UncheckedSQLException e) {
+      // A concurrent create can pass the check above and insert first, so 
this insert then fails
+      // on the primary key. Report that as the namespace already existing.

Review Comment:
   The two-line comment partly restates the code. We could trim it to the 
non-obvious part: `// A concurrent create inserted first`.



##########
core/src/test/java/org/apache/iceberg/jdbc/TestJdbcCatalog.java:
##########
@@ -985,6 +985,22 @@ public void testCreateNamespaceWithMetadata() {
     assertThat(catalog.namespaceExists(testNamespace)).isTrue();
   }
 
+  @Test
+  public void testCreateNamespaceLosingConcurrentCreate() {

Review Comment:
   https://iceberg.apache.org/contribute/#conventions-and-recommendations
   
   * Omit the public modifier for test classes, test methods, and lifecycle 
methods for newly added tests.
   * Omit the test prefix for newly added test methods.
   



-- 
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]

Reply via email to