From 86acd38c4cc0dd7249176815eaca62f5ae66a04d Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 28 Apr 2025 19:18:59 -0700 Subject: [PATCH 01/10] refactor --- .../relational/jdbc/DatasourceOperations.java | 55 ++++++++----- .../jdbc/JdbcBasePersistenceImpl.java | 61 +++++++------- .../relational/jdbc/ResultSetIterator.java | 80 +++++++++++++++++++ .../jdbc/DatasourceOperationsTest.java | 3 +- 4 files changed, 144 insertions(+), 55 deletions(-) create mode 100644 extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java index 6fec8e67fe..5abccbd8a6 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java @@ -31,9 +31,13 @@ import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; +import java.util.stream.Stream; import javax.sql.DataSource; + +import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.extension.persistence.relational.jdbc.models.Converter; public class DatasourceOperations { @@ -90,39 +94,48 @@ public void executeScript(String scriptFilePath) throws SQLException { } /** - * Executes SELECT Query + * Executes SELECT Query and returns the results after applying a transformer * * @param query : Query to executed - * @param entityClass : Class of the entity being selected - * @param transformer : Transformation of entity class to Result class - * @param entityFilter : Filter to applied on the Result class - * @param limit : Limit to to enforced. - * @return List of Result class objects + * @param converterInstance : An entity of the type being selected, used to convert to PolarisBaseEntity + * @return The list of results yielded by the query * @param : Entity class * @param : Result class * @throws SQLException : Exception during the query execution. */ public List executeSelect( @Nonnull String query, - @Nonnull Class entityClass, - @Nonnull Function transformer, - Predicate entityFilter, - int limit) + @Nonnull Converter converterInstance, + @Nonnull Function transformer) throws SQLException{ + ArrayList results = new ArrayList<>(); + executeSelectOverStream(query, converterInstance, stream -> { + stream + .map(transformer) + .forEach(results::add); + }); + return results; + } + + /** + * Executes SELECT Query and takes a consumer over the results. For callers that + * want more sophisticated control over how query results are handled. + * + * @param query : Query to executed + * @param converterInstance : An entity of the type being selected + * @param consumer : An function to consume the returned results + * @param : Entity class + * @throws SQLException : Exception during the query execution. + */ + public void executeSelectOverStream( + @Nonnull String query, + @Nonnull Converter converterInstance, + @Nonnull Consumer> consumer) throws SQLException { try (Connection connection = borrowConnection(); Statement statement = connection.createStatement(); ResultSet resultSet = statement.executeQuery(query)) { - List resultList = new ArrayList<>(); - while (resultSet.next() && resultList.size() < limit) { - Converter object = - (Converter) - entityClass.getDeclaredConstructor().newInstance(); // Create a new instance - R entity = transformer.apply(object.fromResultSet(resultSet)); - if (entityFilter == null || entityFilter.test(entity)) { - resultList.add(entity); - } - } - return resultList; + ResultSetIterator iterator = new ResultSetIterator<>(resultSet, converterInstance); + consumer.accept(iterator.toStream()); } catch (SQLException e) { throw e; } catch (Exception e) { diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index 5ffce813f9..f688b5c6b7 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -30,6 +30,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -302,8 +304,7 @@ public PolarisBaseEntity lookupEntityByName( private PolarisBaseEntity getPolarisBaseEntity(String query) { try { List results = - datasourceOperations.executeSelect( - query, ModelEntity.class, ModelEntity::toEntity, null, Integer.MAX_VALUE); + datasourceOperations.executeSelect(query, new ModelEntity(), ModelEntity::toEntity); if (results.isEmpty()) { return null; } else if (results.size() > 1) { @@ -327,8 +328,7 @@ public List lookupEntities( if (entityIds == null || entityIds.isEmpty()) return new ArrayList<>(); String query = generateSelectQueryWithEntityIds(realmId, entityIds); try { - return datasourceOperations.executeSelect( - query, ModelEntity.class, ModelEntity::toEntity, null, Integer.MAX_VALUE); + return datasourceOperations.executeSelect(query, new ModelEntity(), ModelEntity::toEntity); } catch (SQLException e) { throw new RuntimeException( String.format("Failed to retrieve polaris entities due to %s", e.getMessage()), e); @@ -417,9 +417,14 @@ public List listEntities( // absence of transaction. String query = QueryGenerator.generateSelectQuery(ModelEntity.class, params); try { - List results = - datasourceOperations.executeSelect( - query, ModelEntity.class, ModelEntity::toEntity, entityFilter, limit); + List results = new ArrayList<>(); + datasourceOperations.executeSelectOverStream(query, new ModelEntity(), stream -> { + stream + .map(ModelEntity::toEntity) + .filter(entityFilter) + .limit(limit) + .forEach(results::add); + }); return results == null ? Collections.emptyList() : results.stream().filter(entityFilter).map(transformer).collect(Collectors.toList()); @@ -464,13 +469,8 @@ public PolarisGrantRecord lookupGrantRecord( realmId); String query = generateSelectQuery(ModelGrantRecord.class, params); try { - List results = - datasourceOperations.executeSelect( - query, - ModelGrantRecord.class, - ModelGrantRecord::toGrantRecord, - null, - Integer.MAX_VALUE); + List results = datasourceOperations + .executeSelect(query, new ModelGrantRecord(), ModelGrantRecord::toGrantRecord); if (results.size() > 1) { throw new IllegalStateException( String.format( @@ -502,10 +502,9 @@ public List loadAllGrantRecordsOnSecurable( List results = datasourceOperations.executeSelect( query, - ModelGrantRecord.class, - ModelGrantRecord::toGrantRecord, - null, - Integer.MAX_VALUE); + new ModelGrantRecord(), + ModelGrantRecord::toGrantRecord + ); return results == null ? Collections.emptyList() : results; } catch (SQLException e) { throw new RuntimeException( @@ -528,10 +527,9 @@ public List loadAllGrantRecordsOnGrantee( List results = datasourceOperations.executeSelect( query, - ModelGrantRecord.class, - ModelGrantRecord::toGrantRecord, - null, - Integer.MAX_VALUE); + new ModelGrantRecord(), + ModelGrantRecord::toGrantRecord + ); return results == null ? Collections.emptyList() : results; } catch (SQLException e) { throw new RuntimeException( @@ -557,9 +555,8 @@ public boolean hasChildren( } String query = generateSelectQuery(ModelEntity.class, params); try { - List results = - datasourceOperations.executeSelect( - query, ModelEntity.class, Function.identity(), null, Integer.MAX_VALUE); + List results = datasourceOperations + .executeSelect(query, new ModelEntity(), Function.identity()); return results != null && !results.isEmpty(); } catch (SQLException e) { throw new RuntimeException( @@ -579,10 +576,9 @@ public PolarisPrincipalSecrets loadPrincipalSecrets( List results = datasourceOperations.executeSelect( query, - ModelPrincipalAuthenticationData.class, - ModelPrincipalAuthenticationData::toPrincipalAuthenticationData, - null, - Integer.MAX_VALUE); + new ModelPrincipalAuthenticationData(), + ModelPrincipalAuthenticationData::toPrincipalAuthenticationData + ); return results == null || results.isEmpty() ? null : results.getFirst(); } catch (SQLException e) { LOGGER.error( @@ -885,10 +881,9 @@ private List fetchPolicyMappingRecords(String query) List results = datasourceOperations.executeSelect( query, - ModelPolicyMappingRecord.class, - ModelPolicyMappingRecord::toPolicyMappingRecord, - null, - Integer.MAX_VALUE); + new ModelPolicyMappingRecord(), + ModelPolicyMappingRecord::toPolicyMappingRecord + ); return results == null ? Collections.emptyList() : results; } catch (SQLException e) { throw new RuntimeException( diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java new file mode 100644 index 0000000000..61a8821452 --- /dev/null +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java @@ -0,0 +1,80 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.extension.persistence.relational.jdbc; + +import java.sql.Connection; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.Iterator; +import java.util.NoSuchElementException; +import java.util.Spliterator; +import java.util.Spliterators; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; +import org.apache.polaris.extension.persistence.relational.jdbc.models.Converter; + +/** + * Used to wrap a ResultSet and the connection which generated it for lazy iteration over a + * ResultSet + */ +public class ResultSetIterator implements Iterator { + private final ResultSet resultSet; + private final Converter converterInstance; + private boolean hasNext; + + public ResultSetIterator(ResultSet resultSet, Converter converterInstance) { + this.resultSet = resultSet; + this.converterInstance = converterInstance; + advance(); + } + + private void advance() { + try { + hasNext = resultSet.next(); + } catch (SQLException e) { + hasNext = false; + throw new RuntimeException(e); + } + } + + @Override + public boolean hasNext() { + return hasNext; + } + + @Override + public T next() { + if (!hasNext) { + throw new NoSuchElementException(); + } + try { + T object = converterInstance.fromResultSet(resultSet); + advance(); + return object; + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + public Stream toStream() { + Spliterator spliterator = Spliterators.spliteratorUnknownSize(this, 0); + return StreamSupport.stream(spliterator, false); + } +} \ No newline at end of file diff --git a/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/DatasourceOperationsTest.java b/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/DatasourceOperationsTest.java index 48d2aa39bf..8fd65cecd2 100644 --- a/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/DatasourceOperationsTest.java +++ b/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/DatasourceOperationsTest.java @@ -27,6 +27,7 @@ import java.util.function.Function; import javax.sql.DataSource; import org.apache.polaris.extension.persistence.relational.jdbc.DatasourceOperations; +import org.apache.polaris.extension.persistence.relational.jdbc.models.ModelEntity; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -78,7 +79,7 @@ void testExecuteSelect_exception() throws Exception { SQLException.class, () -> datasourceOperations.executeSelect( - query, Object.class, Function.identity(), null, Integer.MAX_VALUE)); + query, new ModelEntity(), Function.identity())); } @Test From d8d4ce28c47503ee734a80d2bf72c01a799a3563 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 28 Apr 2025 19:19:28 -0700 Subject: [PATCH 02/10] autolint --- .../relational/jdbc/DatasourceOperations.java | 24 +++--- .../jdbc/JdbcBasePersistenceImpl.java | 44 +++++------ .../relational/jdbc/ResultSetIterator.java | 74 +++++++++---------- .../jdbc/DatasourceOperationsTest.java | 4 +- 4 files changed, 68 insertions(+), 78 deletions(-) diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java index 5abccbd8a6..cb49387d75 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java @@ -33,11 +33,8 @@ import java.util.Objects; import java.util.function.Consumer; import java.util.function.Function; -import java.util.function.Predicate; import java.util.stream.Stream; import javax.sql.DataSource; - -import org.apache.polaris.core.entity.PolarisBaseEntity; import org.apache.polaris.extension.persistence.relational.jdbc.models.Converter; public class DatasourceOperations { @@ -97,7 +94,8 @@ public void executeScript(String scriptFilePath) throws SQLException { * Executes SELECT Query and returns the results after applying a transformer * * @param query : Query to executed - * @param converterInstance : An entity of the type being selected, used to convert to PolarisBaseEntity + * @param converterInstance : An entity of the type being selected, used to convert to + * PolarisBaseEntity * @return The list of results yielded by the query * @param : Entity class * @param : Result class @@ -106,19 +104,21 @@ public void executeScript(String scriptFilePath) throws SQLException { public List executeSelect( @Nonnull String query, @Nonnull Converter converterInstance, - @Nonnull Function transformer) throws SQLException{ + @Nonnull Function transformer) + throws SQLException { ArrayList results = new ArrayList<>(); - executeSelectOverStream(query, converterInstance, stream -> { - stream - .map(transformer) - .forEach(results::add); - }); + executeSelectOverStream( + query, + converterInstance, + stream -> { + stream.map(transformer).forEach(results::add); + }); return results; } /** - * Executes SELECT Query and takes a consumer over the results. For callers that - * want more sophisticated control over how query results are handled. + * Executes SELECT Query and takes a consumer over the results. For callers that want more + * sophisticated control over how query results are handled. * * @param query : Query to executed * @param converterInstance : An entity of the type being selected diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java index f688b5c6b7..4271447b5a 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/JdbcBasePersistenceImpl.java @@ -30,8 +30,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; -import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -418,13 +416,16 @@ public List listEntities( String query = QueryGenerator.generateSelectQuery(ModelEntity.class, params); try { List results = new ArrayList<>(); - datasourceOperations.executeSelectOverStream(query, new ModelEntity(), stream -> { - stream - .map(ModelEntity::toEntity) - .filter(entityFilter) - .limit(limit) - .forEach(results::add); - }); + datasourceOperations.executeSelectOverStream( + query, + new ModelEntity(), + stream -> { + stream + .map(ModelEntity::toEntity) + .filter(entityFilter) + .limit(limit) + .forEach(results::add); + }); return results == null ? Collections.emptyList() : results.stream().filter(entityFilter).map(transformer).collect(Collectors.toList()); @@ -469,8 +470,9 @@ public PolarisGrantRecord lookupGrantRecord( realmId); String query = generateSelectQuery(ModelGrantRecord.class, params); try { - List results = datasourceOperations - .executeSelect(query, new ModelGrantRecord(), ModelGrantRecord::toGrantRecord); + List results = + datasourceOperations.executeSelect( + query, new ModelGrantRecord(), ModelGrantRecord::toGrantRecord); if (results.size() > 1) { throw new IllegalStateException( String.format( @@ -501,10 +503,7 @@ public List loadAllGrantRecordsOnSecurable( try { List results = datasourceOperations.executeSelect( - query, - new ModelGrantRecord(), - ModelGrantRecord::toGrantRecord - ); + query, new ModelGrantRecord(), ModelGrantRecord::toGrantRecord); return results == null ? Collections.emptyList() : results; } catch (SQLException e) { throw new RuntimeException( @@ -526,10 +525,7 @@ public List loadAllGrantRecordsOnGrantee( try { List results = datasourceOperations.executeSelect( - query, - new ModelGrantRecord(), - ModelGrantRecord::toGrantRecord - ); + query, new ModelGrantRecord(), ModelGrantRecord::toGrantRecord); return results == null ? Collections.emptyList() : results; } catch (SQLException e) { throw new RuntimeException( @@ -555,8 +551,8 @@ public boolean hasChildren( } String query = generateSelectQuery(ModelEntity.class, params); try { - List results = datasourceOperations - .executeSelect(query, new ModelEntity(), Function.identity()); + List results = + datasourceOperations.executeSelect(query, new ModelEntity(), Function.identity()); return results != null && !results.isEmpty(); } catch (SQLException e) { throw new RuntimeException( @@ -577,8 +573,7 @@ public PolarisPrincipalSecrets loadPrincipalSecrets( datasourceOperations.executeSelect( query, new ModelPrincipalAuthenticationData(), - ModelPrincipalAuthenticationData::toPrincipalAuthenticationData - ); + ModelPrincipalAuthenticationData::toPrincipalAuthenticationData); return results == null || results.isEmpty() ? null : results.getFirst(); } catch (SQLException e) { LOGGER.error( @@ -882,8 +877,7 @@ private List fetchPolicyMappingRecords(String query) datasourceOperations.executeSelect( query, new ModelPolicyMappingRecord(), - ModelPolicyMappingRecord::toPolicyMappingRecord - ); + ModelPolicyMappingRecord::toPolicyMappingRecord); return results == null ? Collections.emptyList() : results; } catch (SQLException e) { throw new RuntimeException( diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java index 61a8821452..891b1bd708 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java @@ -18,10 +18,8 @@ */ package org.apache.polaris.extension.persistence.relational.jdbc; -import java.sql.Connection; import java.sql.ResultSet; import java.sql.SQLException; -import java.sql.Statement; import java.util.Iterator; import java.util.NoSuchElementException; import java.util.Spliterator; @@ -35,46 +33,46 @@ * ResultSet */ public class ResultSetIterator implements Iterator { - private final ResultSet resultSet; - private final Converter converterInstance; - private boolean hasNext; + private final ResultSet resultSet; + private final Converter converterInstance; + private boolean hasNext; - public ResultSetIterator(ResultSet resultSet, Converter converterInstance) { - this.resultSet = resultSet; - this.converterInstance = converterInstance; - advance(); - } + public ResultSetIterator(ResultSet resultSet, Converter converterInstance) { + this.resultSet = resultSet; + this.converterInstance = converterInstance; + advance(); + } - private void advance() { - try { - hasNext = resultSet.next(); - } catch (SQLException e) { - hasNext = false; - throw new RuntimeException(e); - } + private void advance() { + try { + hasNext = resultSet.next(); + } catch (SQLException e) { + hasNext = false; + throw new RuntimeException(e); } + } - @Override - public boolean hasNext() { - return hasNext; - } + @Override + public boolean hasNext() { + return hasNext; + } - @Override - public T next() { - if (!hasNext) { - throw new NoSuchElementException(); - } - try { - T object = converterInstance.fromResultSet(resultSet); - advance(); - return object; - } catch (Exception e) { - throw new RuntimeException(e); - } + @Override + public T next() { + if (!hasNext) { + throw new NoSuchElementException(); } - - public Stream toStream() { - Spliterator spliterator = Spliterators.spliteratorUnknownSize(this, 0); - return StreamSupport.stream(spliterator, false); + try { + T object = converterInstance.fromResultSet(resultSet); + advance(); + return object; + } catch (Exception e) { + throw new RuntimeException(e); } -} \ No newline at end of file + } + + public Stream toStream() { + Spliterator spliterator = Spliterators.spliteratorUnknownSize(this, 0); + return StreamSupport.stream(spliterator, false); + } +} diff --git a/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/DatasourceOperationsTest.java b/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/DatasourceOperationsTest.java index 8fd65cecd2..f5694982c7 100644 --- a/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/DatasourceOperationsTest.java +++ b/extension/persistence/relational-jdbc/src/test/java/org/apache/polaris/extension/persistence/impl/relational/jdbc/DatasourceOperationsTest.java @@ -77,9 +77,7 @@ void testExecuteSelect_exception() throws Exception { assertThrows( SQLException.class, - () -> - datasourceOperations.executeSelect( - query, new ModelEntity(), Function.identity())); + () -> datasourceOperations.executeSelect(query, new ModelEntity(), Function.identity())); } @Test From 6ac54f1b0c718dbc3aad839d58955a0d795430c3 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 28 Apr 2025 19:20:51 -0700 Subject: [PATCH 03/10] polish --- .../persistence/relational/jdbc/ResultSetIterator.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java index 891b1bd708..c80c959999 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java @@ -29,8 +29,7 @@ import org.apache.polaris.extension.persistence.relational.jdbc.models.Converter; /** - * Used to wrap a ResultSet and the connection which generated it for lazy iteration over a - * ResultSet + * Used to wrap a ResultSet and to build a stream from the data it contains */ public class ResultSetIterator implements Iterator { private final ResultSet resultSet; From f421b8c32ed34ce95008386f2e695ee16abb8748 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Mon, 28 Apr 2025 19:20:54 -0700 Subject: [PATCH 04/10] autolint --- .../persistence/relational/jdbc/ResultSetIterator.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java index c80c959999..a0a564ab0c 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java @@ -28,9 +28,7 @@ import java.util.stream.StreamSupport; import org.apache.polaris.extension.persistence.relational.jdbc.models.Converter; -/** - * Used to wrap a ResultSet and to build a stream from the data it contains - */ +/** Used to wrap a ResultSet and to build a stream from the data it contains */ public class ResultSetIterator implements Iterator { private final ResultSet resultSet; private final Converter converterInstance; From 32a3b637ba755d1d194e14c5a93283f7b445db80 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 29 Apr 2025 11:51:49 -0700 Subject: [PATCH 05/10] changes per review --- .../relational/jdbc/ResultSetIterator.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java index a0a564ab0c..0815d5a6ed 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java @@ -28,24 +28,28 @@ import java.util.stream.StreamSupport; import org.apache.polaris.extension.persistence.relational.jdbc.models.Converter; -/** Used to wrap a ResultSet and to build a stream from the data it contains */ +/** + * Used to wrap a ResultSet and to build a stream from the data it contains. This data + * structure will not close the ResultSet passed in, so the caller is still responsible + * for managing its lifecycle + */ public class ResultSetIterator implements Iterator { private final ResultSet resultSet; private final Converter converterInstance; private boolean hasNext; - public ResultSetIterator(ResultSet resultSet, Converter converterInstance) { + public ResultSetIterator(ResultSet resultSet, Converter converterInstance) throws SQLException { this.resultSet = resultSet; this.converterInstance = converterInstance; advance(); } - private void advance() { + private void advance() throws SQLException { try { hasNext = resultSet.next(); } catch (SQLException e) { hasNext = false; - throw new RuntimeException(e); + throw e; } } From 4e9205345b9290117b8ea62546cdd9c88edd2e86 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 29 Apr 2025 11:51:52 -0700 Subject: [PATCH 06/10] autolint --- .../persistence/relational/jdbc/ResultSetIterator.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java index 0815d5a6ed..f5acfb076c 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/ResultSetIterator.java @@ -29,16 +29,17 @@ import org.apache.polaris.extension.persistence.relational.jdbc.models.Converter; /** - * Used to wrap a ResultSet and to build a stream from the data it contains. This data - * structure will not close the ResultSet passed in, so the caller is still responsible - * for managing its lifecycle + * Used to wrap a ResultSet and to build a stream from the data it contains. This data structure + * will not close the ResultSet passed in, so the caller is still responsible for managing its + * lifecycle */ public class ResultSetIterator implements Iterator { private final ResultSet resultSet; private final Converter converterInstance; private boolean hasNext; - public ResultSetIterator(ResultSet resultSet, Converter converterInstance) throws SQLException { + public ResultSetIterator(ResultSet resultSet, Converter converterInstance) + throws SQLException { this.resultSet = resultSet; this.converterInstance = converterInstance; advance(); From 734428d5a60597cb3e383e9dfc24e6188c1db4d4 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Tue, 29 Apr 2025 11:53:55 -0700 Subject: [PATCH 07/10] unwrapping caller --- .../persistence/relational/jdbc/DatasourceOperations.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java index cb49387d75..6047bf26f2 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java @@ -138,6 +138,12 @@ public void executeSelectOverStream( 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); } From 5a81a9c00e89d487166c68e5cf7c672d34784dfa Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 2 May 2025 01:08:03 -0700 Subject: [PATCH 08/10] changes per review --- .../relational/jdbc/DatasourceOperations.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java index 6047bf26f2..5c9b53d513 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java @@ -94,11 +94,12 @@ public void executeScript(String scriptFilePath) throws SQLException { * Executes SELECT Query and returns the results after applying a transformer * * @param query : Query to executed - * @param converterInstance : An entity of the type being selected, used to convert to - * PolarisBaseEntity + * @param converterInstance : An instance of the type being selected, used to convert to + * a business entity like PolarisBaseEntity + * @param transformer Transformation of entity class to Result class * @return The list of results yielded by the query - * @param : Entity class - * @param : Result class + * @param : Persistence entity class + * @param : Business entity class * @throws SQLException : Exception during the query execution. */ public List executeSelect( @@ -110,9 +111,7 @@ public List executeSelect( executeSelectOverStream( query, converterInstance, - stream -> { - stream.map(transformer).forEach(results::add); - }); + stream -> stream.map(transformer).forEach(results::add);); return results; } From 3525af3d58fcae998e743b298b6483cf8115947d Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 2 May 2025 01:14:04 -0700 Subject: [PATCH 09/10] typofix --- .../persistence/relational/jdbc/DatasourceOperations.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java index 5c9b53d513..e739d46dd2 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java @@ -111,7 +111,7 @@ public List executeSelect( executeSelectOverStream( query, converterInstance, - stream -> stream.map(transformer).forEach(results::add);); + stream -> stream.map(transformer).forEach(results::add)); return results; } From 1987821de1d1eeccb7d1f67c42afb94a090e3102 Mon Sep 17 00:00:00 2001 From: Eric Maynard Date: Fri, 2 May 2025 01:14:07 -0700 Subject: [PATCH 10/10] autolint --- .../persistence/relational/jdbc/DatasourceOperations.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java index e739d46dd2..66ee8f9710 100644 --- a/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java +++ b/extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java @@ -94,8 +94,8 @@ public void executeScript(String scriptFilePath) throws SQLException { * Executes SELECT Query and returns the results after applying a transformer * * @param query : Query to executed - * @param converterInstance : An instance of the type being selected, used to convert to - * a business entity like PolarisBaseEntity + * @param converterInstance : An instance of the type being selected, used to convert to a + * business entity like PolarisBaseEntity * @param transformer Transformation of entity class to Result class * @return The list of results yielded by the query * @param : Persistence entity class @@ -109,9 +109,7 @@ public List executeSelect( throws SQLException { ArrayList results = new ArrayList<>(); executeSelectOverStream( - query, - converterInstance, - stream -> stream.map(transformer).forEach(results::add)); + query, converterInstance, stream -> stream.map(transformer).forEach(results::add)); return results; }