rzo1 commented on code in PR #168:
URL: https://github.com/apache/openjpa/pull/168#discussion_r3897687876
##########
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:
Done, every block this PR touches.
##########
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:
Correct, it cannot. `ClassMapping._version` is `private final`, assigned
unconditionally in both constructors (`ClassMapping.java:102` and `:112`, via
`MappingRepository.newVersion()` which always returns `new Version(cls)`),
there is no setter, and there's no subclass of
`ClassMapping`/`MappingRepository` or override of `newVersion()`/`getVersion()`
in the tree. An unresolved mapping still has one — `_version.resolve(...)` runs
later and never clears it — and the embeddable case is rejected one line
earlier by the `getEmbeddingMapping() != null` check. What an unversioned type
gives you is a `Version` with no columns, and `getColumns()` is null-safe in
itself (`_cols` starts as `EMPTY_COLUMNS`, `setColumns()` coerces null back),
so the remaining `getColumns().length == 0` is the only check that means
anything.
--
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]
