-
Notifications
You must be signed in to change notification settings - Fork 377
DATAJDBC-341 - Map NULL values in EntityRowMapper for columns not being fetched in the query #170
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
DATAJDBC-341 - Map NULL values in EntityRowMapper for columns not being fetched in the query #170
Conversation
schauder
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when running all the tests I get tons of NPE. I didn't even check why. Those obviously have to go.
The general direction looks good.
| * @author Mark Paluch | ||
| * @author Jens Schauder | ||
| * @author Christoph Strobl | ||
| * @since 1.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure to use a formatter so that formatting and ordering of annotation/imports does not get changed.
| propertyAccessor.setProperty(property, readOrLoadProperty(idValue, property)); | ||
| } | ||
| } catch (SQLException e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never ever print directly to console. Always use proper logging, with a meaningful message.
In general Exceptions shouldn't get ignored. In the rare cases where this is actually what we want pleas add a comment, why this is the right thing to do.
| value = getObjectFromResultSet(path.extendBy(property).getColumnAlias()); | ||
| } | ||
| } catch (SQLException e) { | ||
| e.printStackTrace(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above.
| try { | ||
| return resultSet.getObject(backreferenceName); | ||
| } catch (SQLException o_O) { | ||
| //throw new MappingException(String.format("Could not read value %s from result set!", backreferenceName), o_O); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, I don't think we should ignore exceptions.
If a column is not in the result set we should not ask for it and if it is and exception should actually get propagated.
965e440 to
3764798
Compare
…ng fetched in the query (fixed tests)
e4cac74 to
b287cb2
Compare
…ng fetched in the query. Original pull request: #170.
…ng fetched in the query. Original pull request: #170.
|
Thank you for your contribution. That 's merged and polished via #201. |
Fix of issue #341 - see here: https://jira.spring.io/browse/DATAJDBC-341