Community
Participate
Working Groups
Build Identifier: 2.2.1 First let me say that I love the QueryHints system - it is really useful. However, in applying QueryHints.BATCH, I am running into a problem. See QueryHintsHandler.BatchHint#applyToDatabaseQuery: an expression in the path is resolved by calling: expression = expression.get(token); This get is named problematically, as it has side-effects (a sidenote - it is my strong believe that method prefixed with a get should never have side-effects): it changes the expression to be an inner-join, independent of what has been set before. See the following code in ObjectExpression: public Expression get(String attributeName, Vector arguments) { Expression operatorExpression = super.get(attributeName, arguments); if (operatorExpression != null) { return operatorExpression; } QueryKeyExpression result = derivedExpressionNamed(attributeName); result.doNotUseOuterJoin(); return result; } IMHO, adding a batch hint should not change the query in any other way, especially not changing outer joins (for which I also explicitly set a QueryHint) to inner joins. An example where this was used: query string: "select wl from WorklistEntry wl left join fetch wl.owner left join fetch wl.modifierEmployee left join wl.approval app left join app.approver apprv left join app.roleRequest rr left join app.functionGroupRequest fgr" and the hints: query.setHint(QueryHints.BATCH_TYPE, "IN"); query.setHint(QueryHints.LEFT_FETCH, "wl.owner"); query.setHint(QueryHints.LEFT_FETCH, "wl.modifierEmployee"); query.setHint(QueryHints.LEFT_FETCH, "wl.approval"); query.setHint(QueryHints.LEFT_FETCH, "wl.approval.roleRequest"); query.setHint(QueryHints.LEFT_FETCH, "wl.approval.functionGroupRequest"); query.setHint(QueryHints.LEFT_FETCH, "wl.approval.approver"); query.setHint(QueryHints.BATCH, "wl.approval.roleRequest.countries"); best regards, Martin Reproducible: Always
It might be that the issue is really the code in ObjectExpression - there are two methods there: public Expression get(String attributeName, Vector arguments) { Expression operatorExpression = super.get(attributeName, arguments); if (operatorExpression != null) { return operatorExpression; } QueryKeyExpression result = derivedExpressionNamed(attributeName); result.doNotUseOuterJoin(); return result; } public Expression getAllowingNull(String attributeName, Vector arguments) { ObjectExpression exp = existingDerivedExpressionNamed(attributeName); // The same (aliased) table cannot participate in a normal join and an outer join. // To help enforce this, if the node already exists if (exp != null) { return exp; } exp = derivedExpressionNamed(attributeName); exp.doUseOuterJoin(); return exp; } Where getAllowingNull() retrieves the expression and only if it is not existing sets the outer join use, but get() retrieves the expression in the first line (which is always null as super.get() returns null), and then always sets exp.doNotUseOuterJoin();. My suggested patch would hence be to change: Expression operatorExpression = super.get(attributeName, arguments); to: ObjectExpression exp = existingDerivedExpressionNamed(attributeName); best regards, Martin
Setting target and priority. See the following page for the meanings of these fields: http://wiki.eclipse.org/EclipseLink/Development/Bugs/Guidelines Community: Please vote for this bug if it is important to you. Votes are one of the main criteria we use to determine which bugs to fix next.
reviewing the suggestion
When does the problem occur? 'At the first run of the query, or when the batch is actually triggered? What happens if you do something like this: query.setHint(QueryHints.BATCH, "rr.countries");
'looks like the following probably will not work: query.setHint(QueryHints.BATCH, "rr.countries"); I think the solution to this issue is to provide a facility like the one suggested above. (i.e. a way to indicate that the batch should use an outer join) Changing the expression code as suggested above will change the fundamentals of how expressions work and make it so certain expression no longer function as they should.
@rr.countries: no, that would not work, Eclipselink wouldn't permit it. @your argument about expressions: this might change the way expressions work, but my hunch would be they are working wrongly in the moment. I have already asked you guys to explain to me why this code is there, I just don't understand what the explanation for this is. In my humble opinion, you are arguing in a circular way: this code can't be wrong because it was always there (and would fundamentally change the way things worked). Maybe things were fundamentally wrong? If you are worried about backwards compatibility, just think about adding a parameter which will change this behaviour. best regards, Martin
Can you please answer this question from the above and perhaps give some pseudocode and output as an example: "When does the problem occur? 'At the first run of the query, or when the batch is actually triggered?" The test of whether the expression framework needs to be fixed or the parsing framework is whether it is possbile to build a query based on the Expression framework that acheives what you want.
Hi Tom, I hope I have understood your question right: this happens in the base query - not in the query which executes the batch fetching. I see the inner joins in the SQL which is printed by the session log. best regards, Martin
Can you please help me reproduce this. I have the following: A OneToMany -> B B OneToOne -> C I am running the following query: query = em.createQuery("select a from A a"); query.setHint(QueryHints.LEFT_FETCH, "a.bs"); query.setHint(QueryHints.LEFT_FETCH, "a.bs.c"); query.setHint(QueryHints.BATCH_TYPE, "IN"); query.setHint(QueryHints.BATCH, "a.bs.c"); When I run that query, all my joins are outer joins. What should I change/add?
I am still not quite sure if I reproduced the problem but the following: query = em.createQuery("select a from A a left join a.bs b"); query.setHint(QueryHints.LEFT_FETCH, "a.bs"); query.setHint(QueryHints.LEFT_FETCH, "a.bs.c"); query.setHint(QueryHints.BATCH_TYPE, "IN"); query.setHint(QueryHints.BATCH, "a.bs.c"); Yields a query that in addition to the left joins contains an inner join from a to b. If that this the problem, the following Expression-built query does not show the same problem: ReadAllQuery raq = new ReadAllQuery(Customer.class); ExpressionBuilder custBuilder = raq.getExpressionBuilder(); Expression ordersExp = custBuilder.anyOfAllowingNone("orders"); Expression itemExp = ordersExp.get("item"); raq.addJoinedAttribute(ordersExp); raq.addJoinedAttribute(itemExp); raq.addBatchReadAttribute(itemExp); If the above is a demonstration of the bug, then having EclipseLink build similar query to the ReadAllQuery here for that query would solve the problem. There are two ways we could do that. 1. allowing the use of the alias "b" from the initial query in the batch and fetch hints 2. allowing some kind of outer join syntax in the hints
Here is the query adjusted for my A-B-C example ReadAllQuery raq = new ReadAllQuery(A.class); ExpressionBuilder aBuilder = raq.getExpressionBuilder(); Expression bsExp = aBuilder.anyOfAllowingNone("bs"); Expression cExp = bsExp.get("c"); raq.addJoinedAttribute(bsExp); raq.addJoinedAttribute(cExp); raq.addBatchReadAttribute(cExp);
Hi Tom, glad that you could reproduce some of the issue, I would have invested some time to give you a testcase soon, if not. From the setup: we only have many-to-one's and one-to-one's in the base query (we need to keep the SQL count intact, to make paging and sorting in the database available). For the one-to-many's (and those can be in a level arbitrarily deep and still need to be fetched), we want to use the batch query hint facility. I agree your options are possible solutions, where I am still not sure that option 1) won't run into the same problem (you also need to retrieve the expression than somehow, even if you use the alias, right?). But if you make this available without having to call get() on ObjectExpression, everything is fine. You will in any case always run into this problem if the get() Method on ObjectExpression is called - see the sourcecode I posted in my first comment. But - could you please explain to me why this line of code (doNotUseOuterJoin()) is valid ;) ? I still see no reason (from an SQL perspective) why this should be there. I promise I will immediately shut up bugging you with this if I have understood the reason ;) best regards, Martin
One key thing here is that there is a translation lair between the JPQL string and the expression code. One thing that translation lair does is keep an index of the various aliased expressions and reuses those expressions when the aliases are used. So.. for instance in: select e from Employee e left join e.projects p In the translation lair, there will be a map that contains: e, (Expression Builder for Employee) p, (Expression Builder for Employee).getAllowingNull("projects") Any reference to p will use that map to get the proper expression for p. Note: the code for this is in our JPQL parsing framework, not in our expression framework (i.e. it is not in the get() or getAllowingNull() method - those look-ups are used in a different context) The org.eclipse.persistence.internal.parsing.jpql package contains the JPQL parsing code. The get() method has a call doNotUseOuterJoin() because the callers of the get method specifically have chosen not to do an outer join. They would have called getAllowingNull() if they want an outer join.
Created attachment 234820 [details] patch
Git main push: bug#356296 batch fetch forces inner joins Fixes batch fetch hint to not convert outer joins to inner joins. Changes: - Added Query hint "eclipselink.query-results-cache.invalidate-on-change" to allow disable of query cache invalidation. - Expression: added get() API to allow not forcing inner join. - IdentityMapManager: Fixed query cache invalidation to avoid memory leak in queryResultsInvalidationsByClass by using Set instead of List. - ForeignReferenceMapping: Changed join fetch to overwrite batch fetch two avoid both being done.
fixed in 2.6
The Eclipselink project has moved to Github: https://github.com/eclipse-ee4j/eclipselink
