SNOW-1848734 Prevent string creation on getObject call when not necessary #1989
Conversation
…eation-in-structured-types-get-object
…uctured-types-get-object' of github.com:snowflakedb/snowflake-jdbc into SNOW-1848734-prevent-unnecessary-string-creation-in-structured-types-get-object
| boolean isStructuredType = resultSetMetaData.isStructuredTypeColumn(columnIndex); | ||
| if (obj == null) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
We have a lot of duplicated code lines between getObject(index) and getObject(index, type)
There was a problem hiding this comment.
moved some part of it to external method
| return new StructObjectWrapper(null, null, obj); | ||
| } | ||
|
|
||
| @SnowflakeJdbcInternalApi |
There was a problem hiding this comment.
private method does not need the internal annotation
There was a problem hiding this comment.
good catch, I'll remove
| @@ -113,6 +113,8 @@ public abstract class SFBaseResultSet { | |||
|
|
|||
| public abstract Object getObject(int columnIndex) throws SFException; | |||
|
|
|||
| public abstract <T> Object getObject(int columnIndex, Class<T> object) throws SFException; | |||
There was a problem hiding this comment.
this API looks strange - the result type should be T, not Object
There was a problem hiding this comment.
I agree, in reality this getObject mostly returns StructObjectWrapper which possibly could be generic. I may rename this method since it's used in higher level getObject anyway to better indicate it's purpose
| private <T> Map<String, Object> prepareMapWithValues(Object object, Class<T> type) | ||
| throws SFException { | ||
| if (object instanceof JsonSqlInput) { | ||
| private <T> Map<String, Object> prepareMapWithValues( |
There was a problem hiding this comment.
where are we using type?
There was a problem hiding this comment.
It was already there, in line 1830 to check if it's assignable to SQLData
sfc-gh-dprzybysz
deleted the
SNOW-1848734-prevent-unnecessary-string-creation-in-structured-types-get-object
branch
Overview
Commit 1 move StructObjectWrapper from converters to SFArrowResultSet
SFArrowResultSet.getObject(int columnIndex). Next step is implementinggetObject(int columnIndex, Class<T> type)which only calls the converter.toString when type is stringFurther commits:
SNOW-1848734
Pre-review self checklist
masterbranchmvn -P check-style validate)mvn verifyand inspecttarget/japicmp/japicmp.html)SNOW-XXXX:External contributors - please answer these questions before submitting a pull request. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Issue: #NNNN
Fill out the following pre-review checklist:
@SnowflakeJdbcInternalApi(note that public/protected methods/fields in classes marked with this annotation are already internal)Please describe how your code solves the related issue.
Please write a short description of how your code change solves the related issue.