Closed colinchilds closed 2 years ago
After looking into this further, I've noticed that many of my queries that return jsonb are breaking. Here's another simple example that returns a PGobject now instead of a String:
connection.query("select '[\"test\"]'::jsonb", res -> {
var results = res.result().getResults();
System.out.println(results.get(0).getValue(0).getClass());
});
@tsegismont would you be able to help me look into this? It's blocking my efforts to upgrade dozens of projects to the latest vertx and is a bit time sensitive.
@pmlopes can you have a look ?
Hey @pmlopes just wanted to ping you again to see if you could help me look into this. Thank you!
@zero88 I think this issue arises from the new "encode/decode" functionality. I'm not sure if the vendor specific type checks are correct. I believe they should use the type OTHER
instead of null. However this will break the support for UUID. Which also is seen as type OTHER
for postgres.
Can you help you clear the issue here?
@zero88 just bumping this again to see if you can help out. Thanks!
@colinchilds thanks for the heads up, and so sorry for the late response. let me re-check it. Do you mind sharing your environment
As your description, I understand you are using legacy JDBCClient
, right?
nvm, I'm able to reproduce it. Will dig more
thanks @zero88
because PG driver wraps an unknown type by JDBC Standards (JDBCType.OTHER
) into PGobject
, then the vendorTypeClass
will be PGObject
in most cases.
vendorTypeClass == PGObject
, the field value will be an instance of PGObject
.another class
, the field value will be cast to the given class if possible (this process is handled internally depending on JDBC driver)string
, uuid
, pgobject
, or whatever type depending on JDBC driver (and its version). Luckily, if it is a custom type from JDBC driver, toString
method will convert to a human-readable value instead of a memory address.So, to deal with the JDBC drivers world and make it compatible with Vertx JsonObject, all unhandled JDBC type will be converted to String
.
To resolve this ticket, I think we need to revert decodeUnhandledType @pmlopes to support UUID natively, I think we can add a check if the value is an instance of java lang type or UUID, which should skip toString. However, AFAIK, MySQL treats UUID as JDBCType.BINARY, so this solution does not guard every kind of database.
can we revert the commit partially ?
@zero88 Any update on this?
I've recently reviewed PR#273 to understand the changes reasons.
After digging in deep, there are 3 issues here:
For the function/procedure on PostgreSQL, the ParameterMetaData
of PgCallableStatement
does not return correct information as other JDBC driver (e.g: MSSQL), but ResultSetMetadata
is correct. I think it works as a design since PostgreSQL can use select * function()
syntax.
So, I suggest JDBC client/pool keep the callable outputs, to look up later, rather than depend on ParameterMetaData
of CallableStatement
.
the old JDBCDecoder (<v4.3.0) is correct in most cases. I will revert PR#273 partially and combine it with PR-267
support for UUID: Is it reasonable to keep/cast an unhandled type in case of the runtime value is a native java type such as java.util.UUID
, java.util.Date
, java.sql.Time
, etc... but JDBCType = OTHER?
Any inputs @vietj @pmlopes ?
I will try to provide a fix this weekend after receiving decision from Vertx team @colinchilds
Thank you for the update!
Any chance we could get this into the next vertx release? I was really hoping to get all my services onto vertx 4 by the end of the year if possible.
Sorry for bumping this yet again. I don't mean to be annoying, I just am really anxious for a fix. @zero88 did you get a decision from the vertx team?
@colinchilds I created a PR. @vietj @pmlopes could you pls have a look? Thanks
@vietj and @pmlopes just bumping this again. Thanks!
Just FYI, I pulled the PR branch and tested my use cases locally and they worked. It seems to have fixed the issue for me. Would love to get this merged and put in the next version! Can I bribe you guys with Amazon gift cards or something? Is that allowed? I'm getting kinda desperate here.
@colinchilds we'll going to review it
Thank you so much!
Hey, just wanted to say thanks again for everyone who helped out with this. Do we have any idea when it could potentially make into a release?
we will release this week
On Wed, Nov 16, 2022 at 9:31 PM colinchilds @.***> wrote:
Hey, just wanted to say thanks again for everyone who helped out with this. Do we have any idea when it could potentially make into a release?
— Reply to this email directly, view it on GitHub https://github.com/vert-x3/vertx-jdbc-client/issues/291#issuecomment-1317631733, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABXDCXB2LAP5G5KDKC6ORLWIVADTANCNFSM6AAAAAAQDS3BRA . You are receiving this because you were mentioned.Message ID: @.***>
Excellent, that's great news. Thank you
I found this issue while trying to upgrade from vert.x 3 to vert.x 4.3.3. After some testing, I noticed that it works correctly in 4.3.0 but starts failing in 4.3.1.
I have many tables with a text
id
and a jsonbdata
column. After runningupdateWithParams
and getting the keys, I used to be able to get thedata
column withresult.getKeys().getString(1)
but that now throws an error, as the key is aPGobject
instead of a string.This can be easily reproduced by simply running the documentation code found here on vert.x 4.3.0 and again on 4.3.1+ with a jsonb column.
result.getKeys()
contains different values between the two.