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



##########
File path: core/src/main/java/org/apache/iceberg/jdbc/JdbcClientPool.java
##########
@@ -48,7 +48,12 @@
   protected Connection newClient() {
     try {
       Properties dbProps = new Properties();
-      properties.forEach((key, value) -> 
dbProps.put(key.replace(JdbcCatalog.PROPERTY_PREFIX, ""), value));
+      properties.forEach((key, value) -> {
+        if (key.startsWith(JdbcCatalog.PROPERTY_PREFIX)) {
+          String dbKey = key.replaceFirst("^" + JdbcCatalog.PROPERTY_PREFIX, 
"");

Review comment:
       Also, I feel like we might need a check that the `properties` passed to 
the `JdbcClientPool` all belong to the correct `jdbc` catalog (should a user 
have two or more configured).
   
   A function definition like this might suffice inside of `JdbcUtil`.
   ```java
   // We might need to check that its got the name in it too (e.g. that it 
doesn't belong to another catalog).
   public static String jdbcPropertyPrefixForCatalog(String name) {
     return String.format("%s"."%s", JdbcCatalog.PROPERTY_PREFIX, name);
   }
   ```
   
   We have similar functions elsewhere. But possibly that's already been 
handled by this point in the code? Added tests would definitely sort this out 👍 




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