kbendick commented on a change in pull request #3275:
URL: https://github.com/apache/iceberg/pull/3275#discussion_r805066972



##########
File path: core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java
##########
@@ -246,10 +261,95 @@ public void setConf(Configuration conf) {
     this.conf = conf;
   }
 
+  private boolean insertProperties(Namespace namespace, Map<String, String> 
properties) {
+    String namespaceName = JdbcUtil.namespaceToString(namespace);
+
+    try {
+      int insertedRecords = connections.run(conn -> {
+        String sqlStatement = 
JdbcUtil.insertPropertiesStatement(properties.size());
+
+        try (PreparedStatement sql = conn.prepareStatement(sqlStatement)) {
+          int rowIndex = 0;
+          for (Map.Entry<String, String> keyValue : properties.entrySet()) {
+            sql.setString(rowIndex + 1, catalogName);
+            sql.setString(rowIndex + 2, namespaceName);
+            sql.setString(rowIndex + 3, keyValue.getKey());
+            sql.setString(rowIndex + 4, keyValue.getValue());
+            rowIndex += 4;
+          }
+          return sql.executeUpdate();
+        }
+      });
+
+      if (insertedRecords == properties.size()) {
+        LOG.debug("Successfully inserted {} properties for namespace {}", 
properties, namespaceName);
+        return true;
+      } else {
+        throw new IllegalStateException();
+      }
+    } catch (InterruptedException e) {
+      Thread.currentThread().interrupt();
+      throw new UncheckedInterruptedException(e, "Interrupted in call to 
insertProperties(namespace, properties) " +
+          "Namespace: %s", namespace);

Review comment:
       Can you update this error message to not refer to code, but 
human-readable ones?
   
   If somebody said it was fine for here, then leave it. But generally 
speaking, we prefer to use plain English (vs English with code in it) as end 
users are unlikely to know the code.
   
   I'd go with something like `Interrupted while inserting properties to 
namespace %s in catalog %s. Insertion likely needs to be retried`.
   
   You can drop the `insertion likely needs to be retried part` if there's not 
a chance of a partial success (I don't think there is as you're using prepared 
statements).




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