rzo1 opened a new pull request, #178:
URL: https://github.com/apache/openjpa/pull/178

   Three separate defects, all reachable: the in-memory executor is selected 
whenever a candidate collection is supplied, when the store does not support 
datastore execution (openjpa-xmlstore), or when dirty instances are queried 
with `FlushBeforeQueries` disabled.
   
   **`ID()`** returned the internal identity wrapper rather than the raw key, 
so a comparison against the plain key threw `ClassCastException` out of 
`Filters.convert` for numeric ids, never matched for an `@EmbeddedId` 
(`OpenJPAId.equals` requires class equality), and matched only by accident for 
a String id. It now unwraps exactly as the JDBC projection does. One correction 
to the review comment: JDBC does not unwrap the comparison operand — it 
normalises the other side, and its only unwrap is `GetNativeObjectId.load()`. 
The wrapper-returning `getObjectId()` is unchanged.
   
   **`NULLS FIRST`/`NULLS LAST`** were ignored: the comparator hard-coded nulls 
last when ascending and first when descending, so two of the four combinations 
were already right and two were silently wrong. The requested precedence is now 
threaded through, falling back to the previous policy when none is given — so 
rejecting these instead would have been a regression.
   
   **Set operations** produced a `NullPointerException` from a compound 
`QueryExpressions` with no filter, or an empty result. They are now rejected 
with a message saying why the query is running in memory and how to avoid it. 
Implementing them would be a feature, not a fix: the executor is built for a 
single candidate extent, `setCandidateCollection` has no defined meaning across 
operands, and the kernel has no multiset semantics for the ALL variants.
   
   Note that the in-memory path cannot yet be exercised end to end from JPQL 
with an identification variable: `JPQLExpressionBuilder` casts the value from 
`getThis()` to `Path`, and the in-memory factory returns a `Val`, so it fails 
with a `ClassCastException`. That is an older, separate defect and wants its 
own issue; it is why the `ID()` change has no end-to-end test here.
   


-- 
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]

Reply via email to