You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "sqd (via GitHub)" <gi...@apache.org> on 2026/02/24 16:23:35 UTC
[PR] #13924: in Iceberg catalog, JdbcTableOperations should catch and handle PostgresSQL's exception for duplicated keys [iceberg]
sqd opened a new pull request, #15434:
URL: https://github.com/apache/iceberg/pull/15434
Fixes https://github.com/apache/iceberg/issues/13924
Previously reviewed at https://github.com/apache/iceberg/pull/13925 (fixed all the suggestions in there)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] #13924: in Iceberg catalog, JdbcTableOperations should catch and handle PostgresSQL's exception for duplicated keys [iceberg]
Posted by "sqd (via GitHub)" <gi...@apache.org>.
sqd commented on code in PR #15434:
URL: https://github.com/apache/iceberg/pull/15434#discussion_r2848810143
##########
core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java:
##########
@@ -138,7 +139,10 @@ public void doCommit(TableMetadata base, TableMetadata metadata) {
throw new UncheckedSQLException(e, "Database warning");
} catch (SQLException e) {
// SQLite doesn't set SQLState or throw SQLIntegrityConstraintViolationException
- if (e.getMessage() != null && e.getMessage().contains("constraint failed")) {
+ // Postgres doesn't throw SQLIntegrityConstraintViolationException but sets SQLState to
+ // "23505" (unique violation)
Review Comment:
Good point. I extrapolated that out and added a few tests.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] #13924: in Iceberg catalog, JdbcTableOperations should catch and handle PostgresSQL's exception for duplicated keys [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #15434:
URL: https://github.com/apache/iceberg/pull/15434#discussion_r2848291872
##########
core/src/main/java/org/apache/iceberg/jdbc/JdbcTableOperations.java:
##########
@@ -138,7 +139,10 @@ public void doCommit(TableMetadata base, TableMetadata metadata) {
throw new UncheckedSQLException(e, "Database warning");
} catch (SQLException e) {
// SQLite doesn't set SQLState or throw SQLIntegrityConstraintViolationException
- if (e.getMessage() != null && e.getMessage().contains("constraint failed")) {
+ // Postgres doesn't throw SQLIntegrityConstraintViolationException but sets SQLState to
+ // "23505" (unique violation)
Review Comment:
Don't we need a similar fix in ViewOperations?
JDBCCatalog has a similar check in renameTable and renameView
It may be best to pull this bit of logic out into a utility method and ditch the magic string.
```java
static boolean isConstraintViolation(SQLException e) {
return e instanceof SQLIntegrityConstraintViolationException
|| POSTGRES_UNIQUE_VIOLATION_SQLSTATE.equals(e.getSQLState())
|| (e.getMessage() != null && e.getMessage().contains("constraint failed"));
}
static void handleCommitSqlException(
SQLException e, String currentMetadataLocation, String entityType, Object identifier) {
if (isConstraintViolation(e)) {
if (currentMetadataLocation == null) {
throw new AlreadyExistsException(e, "%s already exists: %s", entityType, identifier);
} else {
throw new UncheckedSQLException(e, "%s already exists: %s", entityType, identifier);
}
}
throw new UncheckedSQLException(e, "Unknown failure");
}
```
We also need a test of the new pathway, should be a pretty simple mock I think?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org