singhpk234 commented on code in PR #1517:
URL: https://github.com/apache/polaris/pull/1517#discussion_r2075791418


##########
extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java:
##########
@@ -128,22 +143,21 @@ public <T> void executeSelectOverStream(
       @Nonnull Converter<T> converterInstance,
       @Nonnull Consumer<Stream<T>> consumer)
       throws SQLException {
-    try (Connection connection = borrowConnection();
-        Statement statement = connection.createStatement();
-        ResultSet resultSet = statement.executeQuery(query)) {
-      ResultSetIterator<T> iterator = new ResultSetIterator<>(resultSet, 
converterInstance);
-      consumer.accept(iterator.toStream());
-    } catch (SQLException e) {
-      throw e;
-    } catch (RuntimeException e) {
-      if (e.getCause() instanceof SQLException) {
-        throw (SQLException) e.getCause();
-      } else {
-        throw e;
-      }
-    } catch (Exception e) {
-      throw new RuntimeException(e);
-    }
+    withRetries(
+        () -> {
+          try (Connection connection = borrowConnection();
+              Statement statement = connection.createStatement();
+              ResultSet resultSet = statement.executeQuery(query)) {
+            ResultSetIterator<T> iterator = new ResultSetIterator<>(resultSet, 
converterInstance);
+            consumer.accept(iterator.toStream());
+            return null;
+          } catch (RuntimeException e) {
+            if (e.getCause() instanceof SQLException ex) {
+              throw ex;

Review Comment:
   reads also does required, I am assuming like the row i we are selecting or 
is in my sample set does get modified in parallel, nevertheless we also retry 
on like the weird connection issues like connection lost and reset ? we do need 
SQL exception. So IMHO we should do unwrap for the retry to work or i can make 
it generic and handle the if else in the Operation and change signature. 
   
   > we're losing the original exception information
   
   Apologies, didn't fully follow, can you please elaborate, we get 
SQLException in the iterator wrap that to RTE of SQLException and then extract 
back the SQLException here, not sure where exactly its lost ? 



-- 
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-unsubscr...@polaris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to