rdblue commented on a change in pull request #3275:
URL: https://github.com/apache/iceberg/pull/3275#discussion_r736012410
##########
File path: core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java
##########
@@ -252,8 +266,42 @@ public void setConf(Configuration conf) {
@Override
public void createNamespace(Namespace namespace, Map<String, String>
metadata) {
- throw new UnsupportedOperationException("Cannot create namespace " +
namespace +
- ": createNamespace is not supported");
+ if (namespaceExists(namespace)) {
+ throw new AlreadyExistsException("Namespace already exists: %s",
namespace);
+ }
+
+ if (metadata == null || metadata.isEmpty()) {
+ throw new IllegalArgumentException("Cannot create a namespace with null
or empty metadata");
+ }
+
+ String namespaceName = JdbcUtil.namespaceToString(namespace);
+ try {
+ int[] insertedRecords = connections.run(conn -> {
+ try (PreparedStatement sql =
conn.prepareStatement(JdbcUtil.DO_COMMIT_CREATE_NAMESPACE_SQL)) {
+ for (Map.Entry<String, String> keyValue : metadata.entrySet()) {
+ sql.setString(1, catalogName);
+ sql.setString(2, namespaceName);
+ sql.setString(3, keyValue.getKey());
+ sql.setString(4, keyValue.getValue());
+ sql.addBatch();
+ }
+ return sql.executeBatch();
+ }
+ });
+
+ if (insertedRecords.length == metadata.size()) {
Review comment:
I think that this statement is always going to be true. According to the
[executeBatch
docs](https://docs.oracle.com/javase/8/docs/api/java/sql/Statement.html#executeBatch--),
the number of ints should correspond to the number of statements in the batch.
Each one is the number of rows affected by the corresponding set of values in
the batch, or a negative signal value (`SUCCESS_NO_INFO=-2` or
`EXECUTE_FAILED=-3`).
This check is always going to pass, even if there was an error and none of
the inserts actually worked.
The approach to use a batch also raises an issue: what do you do if only
some of the inserts work?
I think you can avoid that problem by using an insert query with multiple
`VALUES` tuples, like this:
```sql
INSERT INTO iceberg_namespace_properties
VALUES
(?, ?, ?, ?),
(?, ?, ?, ?),
(?, ?, ?, ?)
```
You'd just need to construct the argument tuples from a base string, `(?, ?,
?, ?)` and join them with `, `. Then you'd also have to do some position math
to when setting the strings (rowNum + 1, rowNum + 2, etc.).
If you get that working, it should return the number of inserted rows, which
would match `metadata.size()` as you expect here. And I think since it executes
as a single statement, it should either completely succeed or completely fail.
--
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]
