Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ResultClassROF can fail to set fields/properties when JDBC driver returns unassignable type #457

Closed
aley2003 opened this issue Jan 11, 2023 · 9 comments
Labels
Milestone

Comments

@aley2003
Copy link

Actually ResultClassROF.getResultObject() uses a getter specific to the resultClass (if resultClass is a simple class).
In all other cases ResultSet.getObject() will be called.
The Object got from getObject() needs to be converted afterwards to match the result class field.
Unfortunately the conversion works for the first row fetched only.

I have enhanced ResultClassROF to support ResultSet getters specific to the field classes.
I will try to create a pull request for it.

@aley2003
Copy link
Author

Created pull request #458

@andyjefferson
Copy link
Member

Please provide a simple testcase that demonstrates the problem. Test suites need to be able to recreate such things.

@aley2003
Copy link
Author

How can I write a test case that shows how Datanucleus fetch data from the database (in this case which ResultSet getter has been called)?

I have problems with the mapping of SQL queries to ResultObjects (with Oracle 11g).
I have seen in debugger, that there was a call of ResultSet.getObject() for all my number columns.
In Oracle ResultSet.getObject() deliveres a BigDecimal for all number types.
This leads to error messages like

Query needs to return objects of type "..." but it was impossible to set the field "..." type "java.math.BigDecimal". The field should have either a public set/put method, or be public.

I have to add an setter with a BigDecimal parameter to my result class for all number fields (of type short, int, Integer, Long, ...).

@aley2003
Copy link
Author

Using ResultSet.getInt()/getLong()/... hides a problem with setting the values in the result object fields.
For example the BigDecimal got from Oracle has to be converted to an int.
This works on the first row of the ResultSet because of the conversion in the catch in ResultClassROF.getObject() line 456 .
For the following rows ResultClassFieldSetter.set will be called directely with the BigDecimal. At this point, there will be no conversion of the BigDecimal to Integer, because Integer is not assignable from BigDecimal (ResultClassROF:889).
This results in the 021204 NucleusUserException in ResultClassROF:343 mentioned above.

@aley2003
Copy link
Author

My test case can reproduce the problem but I was able to reproduce the problem with Oracle only.
Strangely it does get no error with 'mvn clean compile test' but if I call SimpleTestMain directly.

test-jdo-sql-ResultSetROF-getObject.zip

@andyjefferson
Copy link
Member

Thx. The testcase reproduces a problem using Junit on Oracle (11g) (no need for any "main"), due to how Oracle JDBC maps types

andyjefferson added a commit that referenced this issue Jan 18, 2023
TypeConversionHelper in more situations. Likely needs further tweaking.
For #457
@andyjefferson
Copy link
Member

Suggest that you try with the change made above, since that is way simpler than your PR and works.for.me

@aley2003
Copy link
Author

It looks like your commit solves the conversions problem for Oracle. Thx.

My commit would be an additional enhancement for the way the data is fetched from the ResultSet.
ResultClassROF.getResultObject(ResultSet, int) has an optimization to use the suitable getter (instead of getObject()), if the query has a simple return type. My commit expands this optimization for the fields of the class in case of a class return type.

@andyjefferson andyjefferson changed the title ResultClassROF.getResultObject() does not use getter on ResultSet specific to fields ResultClassROF can fail to set fields/properties when JDBC driver returns unassignable type Jan 20, 2023
@andyjefferson andyjefferson added this to the 6.0.4 milestone Jan 20, 2023
@andyjefferson
Copy link
Member

Marking as fixed, but retaining PR since would be nice to rework the whole class and could use as part of that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants