rzo1 commented on code in PR #144:
URL: https://github.com/apache/openjpa/pull/144#discussion_r3801405861
##########
openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/common/apps/CompUser.java:
##########
@@ -66,6 +67,9 @@ public class CompUser {
@Enumerated
@Basic
private CreditRating creditRating;
+
+ @Version
Review Comment:
Fixed in 08d79ebab.
The entity change was fine — adding `CompVerUser` just removed a coincidence
that was hiding a pre-existing bug in the `VERSION()` implementation.
`VersionVal.appendTo()`/`length()` delegated to `_path`, and for a bare
identification variable `PCPath` falls back to `getPrimaryKeyColumns()`, so
`version(u) = :version` compiled to `t0.userid = ?` bound with the version
value. The old `CompUser "Seetha"` happened to have `userid == version == 1`,
so it matched by accident; `CompVerUser` draws a later sequence block (`userid
= 101`), which exposed it.
Both methods now use `getColumns(state)` (the version columns), which is
what `selectColumns()` already did — that's why the projection form always
worked. Predicate SQL is now `WHERE (t0.name = ? AND t0.version = ?)`,
otherwise byte-identical. `TestEJBQLFunction` is 65/65 and a full reactor run
is clean (4061 tests, 0 failures).
Two related things I left alone, happy to file separately:
`VersionVal.groupBy`/`orderBy` have the same defect, so `GROUP BY`/`ORDER BY
VERSION(u)` still emit the PK column; and `getColumns()` dereferences
`getVersionFieldMapping()` unguarded, so `VERSION(u)` on an unversioned entity
would now NPE instead of silently comparing the PK — `initialize()` would be
the place for a `UserException`.
--
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]
