solomax commented on code in PR #168:
URL: https://github.com/apache/openjpa/pull/168#discussion_r3891558097
##########
openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/VersionVal.java:
##########
@@ -78,11 +79,19 @@ public void setMetaData(ClassMetaData meta) {
@Override
public Class getType() {
- FieldMetaData versionField = _path.getMetaData().getVersionField();
- if (versionField != null) {
- return versionField.getType();
- }
- return null;
+ // note: getColumns()/initialize() resolve the target type from the
+ // ExpState; getType() has none, so it uses the path's class.
+ ClassMetaData meta = _path.getMetaData();
+ FieldMetaData versionField = (meta == null) ? null :
meta.getVersionField();
+ if (versionField != null)
Review Comment:
Could you please add `{}`? :)
##########
openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/VersionVal.java:
##########
@@ -98,23 +107,43 @@ public ExpState initialize(Select sel, ExpContext ctx, int
flags) {
// non-pc fields at all
ClassMapping cls = _path.getClassMapping(state);
if (cls == null || cls.getEmbeddingMapping() != null)
- throw new UserException(_loc.get("bad-getobjectid",
_path.getFieldMapping(state)));
+ throw new UserException(_loc.get("bad-version-path",
pathDescription()));
// types that are not versioned have no version columns to select,
// group, order or compare by; fail with a meaningful message rather
// than a NullPointerException further down the line
- if (cls.getVersion() == null || cls.getVersion().getColumns().length
== 0)
+ if (cls.getVersion().getColumns().length == 0)
Review Comment:
`cls.getVersion()` can't be `null` here?
##########
openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/exps/VersionVal.java:
##########
@@ -78,11 +79,19 @@ public void setMetaData(ClassMetaData meta) {
@Override
public Class getType() {
- FieldMetaData versionField = _path.getMetaData().getVersionField();
- if (versionField != null) {
- return versionField.getType();
- }
- return null;
+ // note: getColumns()/initialize() resolve the target type from the
+ // ExpState; getType() has none, so it uses the path's class.
+ ClassMetaData meta = _path.getMetaData();
+ FieldMetaData versionField = (meta == null) ? null :
meta.getVersionField();
+ if (versionField != null)
Review Comment:
In all changes :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
