You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "singhpk234 (via GitHub)" <gi...@apache.org> on 2025/04/23 21:32:31 UTC
[PR] JDBC: create objects without reflection [polaris]
singhpk234 opened a new pull request, #1434:
URL: https://github.com/apache/polaris/pull/1434
Create model objects without reflection :
Address feedback [here](https://github.com/apache/polaris/pull/1433/files#r2056875073)
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] JDBC: create objects without reflection [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard commented on PR #1434:
URL: https://github.com/apache/polaris/pull/1434#issuecomment-2837950158
In #1487 I solved this by just having the caller pass in an instance of the `Converter`, which seems easy enough
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] JDBC: create objects without reflection [polaris]
Posted by "eric-maynard (via GitHub)" <gi...@apache.org>.
eric-maynard merged PR #1434:
URL: https://github.com/apache/polaris/pull/1434
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] JDBC: create objects without reflection [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #1434:
URL: https://github.com/apache/polaris/pull/1434#discussion_r2056934252
##########
extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java:
##########
@@ -125,10 +128,8 @@ public <T, R> List<R> executeSelect(
ResultSet resultSet = statement.executeQuery(query)) {
List<R> resultList = new ArrayList<>();
while (resultSet.next() && resultList.size() < limit) {
- Converter<T> object =
- (Converter<T>)
- entityClass.getDeclaredConstructor().newInstance(); // Create a new instance
- R entity = transformer.apply(object.fromResultSet(resultSet));
+ Converter<T> model = (Converter<T>) createModelInstance(entityClass);
Review Comment:
How about changing `entityClass` to `Function<ResultSet, R> converter` and removing `transformer`.
Then:
```
public static ModelEntity fromResultSet0(ResultSet r) {
return ModelEntity.builder()
....
```
End result no reflection, not if/else, one less method param, simpler entity classes.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] JDBC: create objects without reflection [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #1434:
URL: https://github.com/apache/polaris/pull/1434#discussion_r2056934252
##########
extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java:
##########
@@ -125,10 +128,8 @@ public <T, R> List<R> executeSelect(
ResultSet resultSet = statement.executeQuery(query)) {
List<R> resultList = new ArrayList<>();
while (resultSet.next() && resultList.size() < limit) {
- Converter<T> object =
- (Converter<T>)
- entityClass.getDeclaredConstructor().newInstance(); // Create a new instance
- R entity = transformer.apply(object.fromResultSet(resultSet));
+ Converter<T> model = (Converter<T>) createModelInstance(entityClass);
Review Comment:
How about changing `entityClass` to `Function<ResultSet, R> converter` and removing `transformer`.
Then:
```
public static ModelEntity fromResultSet(ResultSet r) {
return ModelEntity.builder()
....
```
End result no reflection, not if/else, one less method param, simpler entity classes.
##########
extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java:
##########
@@ -125,10 +128,8 @@ public <T, R> List<R> executeSelect(
ResultSet resultSet = statement.executeQuery(query)) {
List<R> resultList = new ArrayList<>();
while (resultSet.next() && resultList.size() < limit) {
- Converter<T> object =
- (Converter<T>)
- entityClass.getDeclaredConstructor().newInstance(); // Create a new instance
- R entity = transformer.apply(object.fromResultSet(resultSet));
+ Converter<T> model = (Converter<T>) createModelInstance(entityClass);
Review Comment:
How about changing `entityClass` to `Function<ResultSet, R> converter` and removing `transformer`.
Then:
```
public static ModelEntity fromResultSet(ResultSet r) {
return ModelEntity.builder()
....
```
End result: no reflection, not if/else, one less method param, simpler entity classes.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] JDBC: create objects without reflection [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #1434:
URL: https://github.com/apache/polaris/pull/1434#discussion_r2057546612
##########
extension/persistence/relational-jdbc/src/main/java/org/apache/polaris/extension/persistence/relational/jdbc/DatasourceOperations.java:
##########
@@ -125,10 +128,8 @@ public <T, R> List<R> executeSelect(
ResultSet resultSet = statement.executeQuery(query)) {
List<R> resultList = new ArrayList<>();
while (resultSet.next() && resultList.size() < limit) {
- Converter<T> object =
- (Converter<T>)
- entityClass.getDeclaredConstructor().newInstance(); // Create a new instance
- R entity = transformer.apply(object.fromResultSet(resultSet));
+ Converter<T> model = (Converter<T>) createModelInstance(entityClass);
Review Comment:
+1. It's a good idea to combine `toEntity()` and `fromResultSet()` here.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] JDBC: create objects without reflection [polaris]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on PR #1434:
URL: https://github.com/apache/polaris/pull/1434#issuecomment-2844222632
Ack, there was one other place reflection was called, got rid of that as well, we can get both in and we would be finally getting rid of all reflection from JDBC !
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org