Think you are right Michael, Also think we don't need a map but we can just iterate over parameters there - if we want we can use intern() to optimize the search - since we'll rarely get hundreds of parameters so iteration can be as fast as hashmap usage here. Anyway, not caching per parameter but per name can be enough too.
@Mark Struberg <[email protected]> , any feedback on that? Romain Manni-Bucau @rmannibucau <https://twitter.com/rmannibucau> | Blog <https://rmannibucau.metawerx.net/> | Old Blog <http://rmannibucau.wordpress.com> | Github <https://github.com/rmannibucau> | LinkedIn <https://www.linkedin.com/in/rmannibucau> | Book <https://www.packtpub.com/application-development/java-ee-8-high-performance> Le mar. 9 avr. 2019 à 16:29, Michael Wiles <[email protected]> a écrit : > I use Spring DAta extensively and am a big fan of openjpa so I want them to > place nice... > > I can't upgrade the 3.1 until this issue is fixed as basically, as far as I > can tell, any parameterised call via spring data does not work. > > Not sure it's the right place to discuss this but the way I see it the > ParameterExpressionImpl ( > > https://github.com/apache/openjpa/blob/master/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/criteria/ParameterExpressionImpl.java > ) > has acquired a hashCode and equals with this release - > > https://github.com/apache/openjpa/commit/0e4ec5b392b978c4515b26c60e485f2b610de94f#diff-e357856846fb8b88f15c08e60891cc35 > and > this is the code of the problem with spring data. > > Now what's happening is that the compile is called - and this is called > before the parameter expression has a value. All hashcode calcs are done > and stuff is added to a set. > > Then later on the value for the parameter is set. This causes changes to > the hashCode and equals, resulting in the problem that I'm seeing. > > Now I apologise if I'm completely out of line but I'm wondering why the > value is included in the hashCode and equals of a Parameter as surely a > value is a "runtime" concept and it not necessarily available at compile > time. > > Now the hashCode and equals were added for good reason I assume, and > furthermore, the value is included in the hashCode/equals also for good > reason. But we arguably need a mechanism to view the parameter purely from > a metadata point of view (which is I think what we need here) as well as > from a metadata+value point of view. > > But I do wonder why the ParamterExpressionImpl does include the value in > the hashCode and equals. My gut feel is that it's not necessary. > > Relevant stack traces: first time hashCode is called - at this point the > value is not specified in the ParameterExpressionImpl. Notice that the > CriteriaQueryImpl.compile kicks this off. > > > org.apache.openjpa.persistence.criteria.ParameterExpressionImpl<T>.hashCode() > line: 154 > java.util.HashMap<K,V>.hash(java.lang.Object) line: 338 > > java.util.LinkedHashMap<K,V>(java.util.HashMap<K,V>).containsKey(java.lang.Object) > line: 595 > org.apache.openjpa.lib.util.OrderedMap<K,V>.containsKey(java.lang.Object) > line: 70 > > org.apache.openjpa.persistence.criteria.CriteriaQueryImpl<T>.registerParameter(org.apache.openjpa.persistence.criteria.ParameterExpressionImpl<?>) > line: 227 > > org.apache.openjpa.persistence.criteria.CriteriaExpressionVisitor$ParameterVisitor.enter(org.apache.openjpa.persistence.criteria.CriteriaExpression) > line: 106 > > org.apache.openjpa.persistence.criteria.Expressions.acceptVisit(org.apache.openjpa.persistence.criteria.CriteriaExpressionVisitor, > org.apache.openjpa.persistence.criteria.CriteriaExpression, > javax.persistence.criteria.Expression<?>...) line: 106 > > org.apache.openjpa.persistence.criteria.ParameterExpressionImpl<T>(org.apache.openjpa.persistence.criteria.SelectionImpl<X>).acceptVisit(org.apache.openjpa.persistence.criteria.CriteriaExpressionVisitor) > line: 156 > > org.apache.openjpa.persistence.criteria.Expressions.visitChildren(org.apache.openjpa.persistence.criteria.CriteriaExpressionVisitor, > javax.persistence.criteria.Expression<?>...) line: 121 > > org.apache.openjpa.persistence.criteria.Expressions.acceptVisit(org.apache.openjpa.persistence.criteria.CriteriaExpressionVisitor, > org.apache.openjpa.persistence.criteria.CriteriaExpression, > javax.persistence.criteria.Expression<?>...) line: 108 > > org.apache.openjpa.persistence.criteria.Expressions$Equal(org.apache.openjpa.persistence.criteria.Expressions$BinaryLogicalExpression).acceptVisit(org.apache.openjpa.persistence.criteria.CriteriaExpressionVisitor) > line: 278 > > org.apache.openjpa.persistence.criteria.CriteriaQueryImpl<T>.collectParameters(org.apache.openjpa.persistence.criteria.CriteriaExpressionVisitor) > line: 681 > *org.apache.openjpa.persistence.criteria.CriteriaQueryImpl<T>.compile() > line: 672 * > > org.apache.openjpa.persistence.EntityManagerImpl.createQuery(javax.persistence.criteria.CriteriaQuery<T>) > line: 1898 > > Then the error I get, occurs here - in org.apache.openjpa.kernel.QueryImpl > > protected void assertParameters(StoreQuery q, StoreQuery.Executor ex, Map > params) { > if (!q.requiresParameterDeclarations()) > return; > > OrderedMap<Object,Class<?>> paramTypes = > ex.getOrderedParameterTypes(q); > * for (Object actual : params.keySet()) {* > * if (!paramTypes.containsKey(actual))* > throw new UserException(_loc.get("unbound-params", > actual, paramTypes.keySet())); > } > for (Object expected : paramTypes.keySet()) { > if (!params.containsKey(expected)) > throw new UserException(_loc.get("unbound-params", > expected, paramTypes.keySet())); > } > > for (Entry<Object, Class<?>> entry : paramTypes.entrySet()) { > if (entry.getValue().isPrimitive() > && params.get(entry.getKey()) == null) > throw new UserException(_loc.get("null-primitive-param", > entry.getKey())); > } > } > > The error occurs in the bold stuff. > > And the fundamental reason as far as I can tell is that the paramtypes map > was populated when the value was set and then the *actual* reference in > this code has the value set... > > Iow, getOrderedParameterTypes returns the map created before the value was > set and the params.keySet has parameterExpressionImpls that have their > values set. > > And you know what happens when you use a hashMap and you change the > hashCode after you've populated the hashmap. > > Error stack trace: > at > org.apache.openjpa.kernel.QueryImpl.assertParameters(QueryImpl.java:1849) > at org.apache.openjpa.kernel.QueryImpl.execute(QueryImpl.java:905) > at org.apache.openjpa.kernel.QueryImpl.execute(QueryImpl.java:843) > at > org.apache.openjpa.kernel.DelegatingQuery.execute(DelegatingQuery.java:601) > at org.apache.openjpa.persistence.QueryImpl.execute(QueryImpl.java:297) > at > org.apache.openjpa.persistence.QueryImpl.getResultList(QueryImpl.java:314) > at > > org.apache.openjpa.persistence.QueryImpl.getSingleResult(QueryImpl.java:343) > at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) > at > > sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) > at > > sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) > at java.lang.reflect.Method.invoke(Method.java:498) > at org.springframework.orm.jpa.SharedEntityManagerCreat > > On Mon, 8 Apr 2019 at 23:24, Romain Manni-Bucau <[email protected]> > wrote: > > > Yep, same thought > > > > No worries, we can always delay a bit the end of the vote to be sure of > the > > quality we deliver > > > > Le lun. 8 avr. 2019 à 23:13, Jonathan Gallimore < > > [email protected]> a écrit : > > > > > At the moment, I believe its a spring-data thing rather than an OpenEJB > > > thing. I'll try and come up with an actual test, but it'll likely be > > > tomorrow morning. Hope that's ok. > > > > > > Jon > > > > > > On Mon, Apr 8, 2019 at 10:08 PM Romain Manni-Bucau < > > [email protected]> > > > wrote: > > > > > > > IMHO if we break spring data it is a blocker, if we break openejb > layer > > > it > > > > is not since fixable with the upgrade. > > > > > > > > Le lun. 8 avr. 2019 à 23:05, Jonathan Gallimore < > > > > [email protected]> a écrit : > > > > > > > > > Removing the equals() and hashcode() methods added on > > > > > ParameterExpressionImpl here: > > > > > > > > > > > > > > > > > > > > https://github.com/apache/openjpa/commit/0e4ec5b392b978c4515b26c60e485f2b610de94f#diff-e357856846fb8b88f15c08e60891cc35 > > > > > enables > > > > > the test I mentioned to pass. It seems that _boundsParam.get() > here: > > > > > > > > > > > > > > > > > > > > https://github.com/apache/openjpa/blob/master/openjpa-persistence/src/main/java/org/apache/openjpa/persistence/AbstractQuery.java#L81 > > > > > fails > > > > > with these equals()/hashcode() methods. > > > > > > > > > > Will see if I can fix and send a patch. I'm not sure if this issue > > > would > > > > be > > > > > considered to be a release blocker or not. > > > > > > > > > > Jon > > > > > > > > > > On Mon, Apr 8, 2019 at 11:25 AM Jonathan Gallimore < > > > > > [email protected]> wrote: > > > > > > > > > > > I get a single test failure on the TomEE side with this release - > > > > > > specifically org.superbiz.dynamic.DynamicUserDaoTest in > > > > > > examples/spring-proxy-data. I'll take a look during the day. > > > Otherwise, > > > > > > I'm +1. Thanks for the new release! > > > > > > > > > > > > Jon > > > > > > > > > > > > On Sun, Apr 7, 2019 at 3:14 PM Maxim Solodovnik < > > > [email protected]> > > > > > > wrote: > > > > > > > > > > > >> +1 > > > > > >> > > > > > >> Thanks for this release! > > > > > >> > > > > > >> On Sun, 7 Apr 2019 at 17:29, Francesco Chicchiriccò < > > > > > [email protected]> > > > > > >> wrote: > > > > > >> > > > > > > >> > On 2019-04-06 14:52 Mark Struberg wrote: > > > > > >> > > hi folks! > > > > > >> > > > > > > > >> > > We have fixed tons of enhancements and bugs in OpenJPA for > our > > > > 3.1.0 > > > > > >> > > release. > > > > > >> > > One of the main improvements is to move to a native JavaEE8 > > > level > > > > - > > > > > >> > > JPA-2.2. > > > > > >> > > Please note that we implemented many JPA-2.2 features but > not > > > all > > > > > yet. > > > > > >> > > We will work towards implementing the rest of the missing > 2.2 > > > > stuff > > > > > in > > > > > >> > > the next bugfix releases. > > > > > >> > > > > > > > >> > > > > > > > >> > > Here are the fixed tickets: > > > > > >> > > > > > > > >> > > Sub-task > > > > > >> > > > > > > > >> > > • [OPENJPA-2710] - Create and update to > > > > geronimo-jpa_2.2_spec > > > > > >> > > Bug > > > > > >> > > > > > > > >> > > • [OPENJPA-1993] - Deadlock Potential with ORM XML > > > > Processing > > > > > >> > > • [OPENJPA-2555] - Timestamp precision from manual > > schema > > > > not > > > > > >> > > respected > > > > > >> > > • [OPENJPA-2567] - TINY/MEDIUM/LONG TEXT fields for > > MySQL > > > > and > > > > > >> MariaDB > > > > > >> > > are not supported > > > > > >> > > • [OPENJPA-2673] - Table is not created in openjpa > > > > > >> 3.0.0-SNAPSHOT and > > > > > >> > > OSGi > > > > > >> > > • [OPENJPA-2704] - The openjpa.jdbc.Schema no longer > > > > overrides > > > > > >> orm.xml > > > > > >> > > default > > > > > >> > > • [OPENJPA-2733] - Subquery parameters are incorrectly > > > > > assigned > > > > > >> > > • [OPENJPA-2742] - SchemaTool fails with MySQL > > > > > >> > > • [OPENJPA-2746] - OpenJPA Karaf feature is not > complete > > > > > >> > > • [OPENJPA-2756] - PostgreSQL requires escaping of > > search > > > > > >> strings in > > > > > >> > > all versions > > > > > >> > > • [OPENJPA-2757] - upgrade to xbean-asm7 to support > > Java11 > > > > > >> > > • [OPENJPA-2761] - problem inserting more than 4000 > > > > charcters > > > > > in > > > > > >> > > oracle XMLTYPE column > > > > > >> > > • [OPENJPA-2764] - Map path expression tests behave > > random > > > > > >> > > • [OPENJPA-2768] - XMLStore SAXParser doesn't > > distinguish > > > > > >> between > > > > > >> > > element and extent > > > > > >> > > • [OPENJPA-2770] - false boolean literal doesn't work > > > > > >> > > • [OPENJPA-2771] - It seems like h2 'unlimited' is not > > > > "LIMIT > > > > > >> 0" but > > > > > >> > > rather "LIMIT -1" > > > > > >> > > • [OPENJPA-2772] - list of h2 reserved words is > > incomplete > > > > > >> > > • [OPENJPA-2777] - Indices specified using > > > > > >> javax.persistence.Index > > > > > >> > > annotation are not being created > > > > > >> > > • [OPENJPA-2780] - ReverseMappingTool does not > generate > > > > > >> @Enumerated > > > > > >> > > annotation > > > > > >> > > • [OPENJPA-2781] - OpenJPA need internet connection to > > > read > > > > > the > > > > > >> > > persistence.xml > > > > > >> > > Improvement > > > > > >> > > > > > > > >> > > • [OPENJPA-2745] - Clean up try-catch implementation > for > > > > > >> DB2Dictionary > > > > > >> > > • [OPENJPA-2747] - Upgrade to JPA 2.2 and use > > > > > >> javax.persistence-api > > > > > >> > > spec > > > > > >> > > • [OPENJPA-2748] - commons-collections should be > updated > > > to > > > > > most > > > > > >> > > recent version > > > > > >> > > • [OPENJPA-2750] - commons-dbcp need to be updated to > > most > > > > > >> recent > > > > > >> > > version > > > > > >> > > • [OPENJPA-2751] - Code clean-up should be performed > > > > > >> > > • [OPENJPA-2752] - More libraries can be updated > > > > > >> > > • [OPENJPA-2753] - Create profiles to start various > > > > databases > > > > > >> via > > > > > >> > > Docker > > > > > >> > > • [OPENJPA-2755] - support MySQL DATETIME and > TIMESTAMP > > > > > >> fractions > > > > > >> > > (milliseconds, nanos) > > > > > >> > > • [OPENJPA-2773] - set minIdle to > 0 in > > > > DBCPDriverDataSource > > > > > >> > > • [OPENJPA-2775] - hsqldb doesn't support NullTable to > > > > > retrieve > > > > > >> meta > > > > > >> > > information > > > > > >> > > Task > > > > > >> > > > > > > > >> > > • [OPENJPA-2744] - commons-pool should be updated to > > most > > > > > recent > > > > > >> > > version > > > > > >> > > • [OPENJPA-2754] - update to latest dbcp and verify > > moving > > > > > from > > > > > >> > > maxActive to maxTotal > > > > > >> > > • [OPENJPA-2758] - JPA 2.2 compliance > > > > > >> > > Dependency upgrade > > > > > >> > > > > > > > >> > > • [OPENJPA-2784] - update docs before our release > > > > > >> > > > > > > > >> > > > > > > > >> > > The staging repository is at: > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > > https://repository.apache.org/content/repositories/orgapacheopenjpa-1005/ > > > > > >> > > > > > > > >> > > The source release is at > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > > https://repository.apache.org/content/repositories/orgapacheopenjpa-1005/org/apache/openjpa/openjpa-parent/3.1.0/ > > > > > >> > > sha1 is 1aea7cfff20c3a5fed57fb41fb1fcd4784b999ae > > > > > >> > > > > > > > >> > > I've pushed the release branch to my github repo > > > > > >> > > https://github.com/struberg/openjpa/tree/release-3.1.0 > > > > > >> > > > > > > > >> > > > > > > > >> > > Please VOTE: > > > > > >> > > > > > > > >> > > [+1] yeah, ship it! > > > > > >> > > [+0] meh, don't care > > > > > >> > > [-1] nah, because ${showstopper} > > > > > >> > > > > > > >> > +1 > > > > > >> > > > > > > >> > ..and special thanks to Mark, for his enduring effort! > > > > > >> > Regards. > > > > > >> > -- > > > > > >> > Francesco Chicchiriccò > > > > > >> > > > > > > >> > Tirasa - Open Source Excellence > > > > > >> > http://www.tirasa.net/ > > > > > >> > > > > > > >> > Member at The Apache Software Foundation > > > > > >> > Syncope, Cocoon, Olingo, CXF, OpenJPA, PonyMail > > > > > >> > http://home.apache.org/~ilgrosso/ > > > > > >> > > > > > >> > > > > > >> > > > > > >> -- > > > > > >> WBR > > > > > >> Maxim aka solomax > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > -- > see my blog: > http://analysis102.blogspot.com > http://audiblethoughts.blogspot.com > http://outsideofficehours.blogspot.com >
