Skip to content

JDBC: Remove logging for DatabaseOperations #1433

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 23, 2025
Merged

JDBC: Remove logging for DatabaseOperations #1433

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 1 addition & 15 deletions ...n/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,8 @@ public void executeScript(String scriptFilePath) throws SQLException {
if (line.endsWith(";")) { // Execute statement when semicolon is found
String sql = sqlBuffer.toString().trim();
try {
int rowsUpdated = statement.executeUpdate(sql);
LOGGER.debug("Query {} executed {} rows affected", sql, rowsUpdated);
statement.executeUpdate(sql);
} catch (SQLException e) {
LOGGER.error("Error executing query {}", sql, e);
// re:throw this as unhandled exception
throw new RuntimeException(e);
}
sqlBuffer.setLength(0); // Clear the buffer for the next statement
Expand All @@ -92,11 +89,7 @@ public void executeScript(String scriptFilePath) throws SQLException {
connection.setAutoCommit(autoCommit);
}
} catch (IOException e) {
LOGGER.debug("Error reading the script file", e);
throw new RuntimeException(e);
} catch (SQLException e) {
LOGGER.debug("Error executing the script file", e);
throw e;
}
}

Expand Down Expand Up @@ -135,7 +128,6 @@ public <T, R> List<R> executeSelect(
}
return resultList;
} catch (SQLException e) {
LOGGER.debug("Error executing query {}", query, e);
throw e;
} catch (Exception e) {
throw new RuntimeException(e);
Comment on lines 130 to 133
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do something like this to avoid reflection?

 Converter<T> object = createObject(entityClass);
# createObject() can create object by mapping name to a class instead of reflection.

Not a blocker though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I missed this as I merged. The PR looked like a pretty simple red diff to me... where is the reflection that we should remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds fair let me do that, we just have like 3 classes so we should be fine, adding a PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries, not a blocker.

Expand All @@ -156,9 +148,6 @@ public int executeUpdate(String query) throws SQLException {
connection.setAutoCommit(true);
try {
return statement.executeUpdate(query);
} catch (SQLException e) {
LOGGER.debug("Error executing query {}", query, e);
throw e;
} finally {
connection.setAutoCommit(autoCommit);
}
Expand Down Expand Up @@ -188,9 +177,6 @@ public void runWithinTransaction(TransactionCallback callback) throws SQLExcepti
}
connection.setAutoCommit(autoCommit);
}
} catch (SQLException e) {
LOGGER.debug("Caught Error while executing transaction", e);
throw e;
}
}

Expand Down