feat: support getObject(int, byte[].class) for bytea #3274
Conversation
| @@ -3875,6 +3875,13 @@ public void updateArray(String columnName, @Nullable Array x) throws SQLExceptio | |||
| throw new PSQLException(GT.tr("conversion to {0} from {1} not supported", type, getPGType(columnIndex)), | |||
| PSQLState.INVALID_PARAMETER_VALUE); | |||
| } | |||
| } else if (type == byte[].class) { | |||
| if (sqlType == Types.BINARY) { | |||
There was a problem hiding this comment.
How about Types.VARBINARY, Types.LONGVARBINARY?
There was a problem hiding this comment.
I thought about it, but decided against because Blob only checks for BINARY. Besides, sqlType here is what PG thinks is appropriate for BYTEA, its not a user input, so actually it should never (?) be multiple values.
Do you think other *BINARY values are needed here?
There was a problem hiding this comment.
If someone stored VARBINARY or LONGVARBINARY the driver still needs to be able to handle it
| PreparedStatement insertPS = conn.prepareStatement(TestUtil.insertSQL("table1", "bytea_column", "?")); | ||
| try { |
There was a problem hiding this comment.
Ok, will do, I just copy-pasted Blob test )
| @Test | ||
| void getBytea() throws SQLException { | ||
| Statement stmt = conn.createStatement(); | ||
| conn.setAutoCommit(false); |
There was a problem hiding this comment.
Why autoCommit modification is needed here?
There was a problem hiding this comment.
Well, its still copy-paste from getBlob(), but AFAIK without it the select statement could take too much memory: https://postgrespro.ru/list/thread-id/2502309
There was a problem hiding this comment.
Right, but you should never run out of memory in this test.
| */ | ||
| @Test | ||
| void getBytea() throws SQLException { | ||
| Statement stmt = conn.createStatement(); |
There was a problem hiding this comment.
WDYT of moving the declaration of stmt closer to its usage?
|
The change looks good to me, however I wonder:
|
| insertPS.setBytes(1, data); | ||
| insertPS.executeUpdate(); | ||
| } finally { | ||
| insertPS.close(); |
There was a problem hiding this comment.
Yes please use try with resources. We are now trying to move all new code to that style
|
Ok, I think I fixed all comments. |
New Feature Submissions:
yes
./gradlew styleCheckpass ?yes
I did, though they weren't
Changes to Existing Features:
no
see Feature: please support getObject(int, byte[].class) for bytea #3269
yes
yes
All Submissions:
yes
yes