You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "johnnysohn (via GitHub)" <gi...@apache.org> on 2026/02/20 09:24:21 UTC
[PR] change rs.getObject to specific types like getLong, getString, getInt [polaris]
johnnysohn opened a new pull request, #3849:
URL: https://github.com/apache/polaris/pull/3849
Certain jdbc drivers don't implement ResultSet.getObject(String columnLabel, Class<T> type) and throws `SQLFeatureNotSupportedException`. Consolidating the method to be specific to each type in line with other uses in Polaris like:
https://github.com/apache/polaris/blob/main/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/models/ModelEvent.java#L109
https://github.com/apache/polaris/blob/main/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/models/ModelIdempotencyRecord.java#L143
https://github.com/apache/polaris/blob/main/persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/models/EntityNameLookupRecordConverter.java
<!--
๐ Describe what changes you're proposing, especially breaking or user-facing changes.
๐ See https://github.com/apache/polaris/blob/main/CONTRIBUTING.md for more.
-->
## Checklist
- [ ] ๐ก๏ธ Don't disclose security issues! (contact security@apache.org)
- [ ] ๐ Clearly explained why the changes are needed, or linked related issues: Fixes #
- [ ] ๐งช Added/updated tests with good coverage, or manually tested (and explained how)
- [ ] ๐ก Added comments for complex logic
- [ ] ๐งพ Updated `CHANGELOG.md` (if needed)
- [ ] ๐ Updated documentation in `site/content/in-dev/unreleased` (if needed)
--
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] change rs.getObject to specific types like getLong, getString, getInt [polaris]
Posted by "johnnysohn (via GitHub)" <gi...@apache.org>.
johnnysohn commented on PR #3849:
URL: https://github.com/apache/polaris/pull/3849#issuecomment-3958139779
> which pg driver is being used here, can you please elaborate
actually we're using mysql (using this patch https://github.com/apache/polaris/pull/2704) with Vitess jdbc [driver](https://github.com/vitessio/vitess/blob/main/java/jdbc/src/main/java/io/vitess/jdbc/VitessResultSet.java#L1570). Strange choice I know ๐ but it mostly works except without this issue.
--
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] change rs.getObject to specific types like getLong, getString, getInt [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on PR #3849:
URL: https://github.com/apache/polaris/pull/3849#issuecomment-3957293886
> Overall LGTM. Re: the initial problem, I wonder if possible to add an integ test making sure `SQLFeatureNotSupportedException` is no longer thrown?
That would be a good thing to have!
--
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] change rs.getObject to specific types like getLong, getString, getInt [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy merged PR #3849:
URL: https://github.com/apache/polaris/pull/3849
--
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] change rs.getObject to specific types like getLong, getString, getInt [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3849:
URL: https://github.com/apache/polaris/pull/3849#discussion_r2848079828
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/models/ModelEntity.java:
##########
@@ -219,23 +219,21 @@ public PolarisBaseEntity fromResultSet(ResultSet r) throws SQLException {
var modelEntity =
ModelEntity.builder()
- .catalogId(r.getObject("catalog_id", Long.class))
- .id(r.getObject("id", Long.class))
- .parentId(r.getObject("parent_id", Long.class))
- .typeCode(r.getObject("type_code", Integer.class))
- .name(r.getObject("name", String.class))
- .entityVersion(r.getObject("entity_version", Integer.class))
- .subTypeCode(r.getObject("sub_type_code", Integer.class))
- .createTimestamp(r.getObject("create_timestamp", Long.class))
- .dropTimestamp(r.getObject("drop_timestamp", Long.class))
- .purgeTimestamp(r.getObject("purge_timestamp", Long.class))
- .toPurgeTimestamp(r.getObject("to_purge_timestamp", Long.class))
- .lastUpdateTimestamp(r.getObject("last_update_timestamp", Long.class))
- // JSONB: use getString(), not getObject().
+ .catalogId(r.getLong("catalog_id"))
+ .id(r.getLong("id"))
+ .parentId(r.getLong("parent_id"))
+ .typeCode(r.getInt("type_code"))
Review Comment:
`null` handling may be different with the new code... Are we sure the affected integer fields are non-null?
--
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] change rs.getObject to specific types like getLong, getString, getInt [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3849:
URL: https://github.com/apache/polaris/pull/3849#discussion_r2848119608
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/models/ModelEntity.java:
##########
@@ -219,23 +219,21 @@ public PolarisBaseEntity fromResultSet(ResultSet r) throws SQLException {
var modelEntity =
ModelEntity.builder()
- .catalogId(r.getObject("catalog_id", Long.class))
- .id(r.getObject("id", Long.class))
- .parentId(r.getObject("parent_id", Long.class))
- .typeCode(r.getObject("type_code", Integer.class))
- .name(r.getObject("name", String.class))
- .entityVersion(r.getObject("entity_version", Integer.class))
- .subTypeCode(r.getObject("sub_type_code", Integer.class))
- .createTimestamp(r.getObject("create_timestamp", Long.class))
- .dropTimestamp(r.getObject("drop_timestamp", Long.class))
- .purgeTimestamp(r.getObject("purge_timestamp", Long.class))
- .toPurgeTimestamp(r.getObject("to_purge_timestamp", Long.class))
- .lastUpdateTimestamp(r.getObject("last_update_timestamp", Long.class))
- // JSONB: use getString(), not getObject().
+ .catalogId(r.getLong("catalog_id"))
+ .id(r.getLong("id"))
+ .parentId(r.getLong("parent_id"))
+ .typeCode(r.getInt("type_code"))
Review Comment:
I checked this myself, but it would be nice if someone else could double check.
--
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] change rs.getObject to specific types like getLong, getString, getInt [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3849:
URL: https://github.com/apache/polaris/pull/3849#discussion_r2848146135
##########
persistence/relational-jdbc/src/main/java/org/apache/polaris/persistence/relational/jdbc/models/ModelEntity.java:
##########
@@ -219,23 +219,21 @@ public PolarisBaseEntity fromResultSet(ResultSet r) throws SQLException {
var modelEntity =
ModelEntity.builder()
- .catalogId(r.getObject("catalog_id", Long.class))
- .id(r.getObject("id", Long.class))
- .parentId(r.getObject("parent_id", Long.class))
- .typeCode(r.getObject("type_code", Integer.class))
- .name(r.getObject("name", String.class))
- .entityVersion(r.getObject("entity_version", Integer.class))
- .subTypeCode(r.getObject("sub_type_code", Integer.class))
- .createTimestamp(r.getObject("create_timestamp", Long.class))
- .dropTimestamp(r.getObject("drop_timestamp", Long.class))
- .purgeTimestamp(r.getObject("purge_timestamp", Long.class))
- .toPurgeTimestamp(r.getObject("to_purge_timestamp", Long.class))
- .lastUpdateTimestamp(r.getObject("last_update_timestamp", Long.class))
- // JSONB: use getString(), not getObject().
+ .catalogId(r.getLong("catalog_id"))
+ .id(r.getLong("id"))
+ .parentId(r.getLong("parent_id"))
+ .typeCode(r.getInt("type_code"))
Review Comment:
I only saw "null safe" primitives.
--
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