Download
Getting Started
Members
Projects
Community
Marketplace
Events
Planet Eclipse
Newsletter
Videos
Participate
Report a Bug
Forums
Mailing Lists
Wiki
IRC
How to Contribute
Working Groups
Automotive
Internet of Things
LocationTech
Long-Term Support
PolarSys
Science
OpenMDM
More
Community
Marketplace
Events
Planet Eclipse
Newsletter
Videos
Participate
Report a Bug
Forums
Mailing Lists
Wiki
IRC
How to Contribute
Working Groups
Automotive
Internet of Things
LocationTech
Long-Term Support
PolarSys
Science
OpenMDM
Toggle navigation
Bugzilla – Attachment 160820 Details for
Bug 246211
using inner joins for dotNode in JPQL outside the where clause filters out null results
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
Log In
[x]
|
Terms of Use
|
Copyright Agent
Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read
this important communication.
[patch]
proposed patch
246211.patch (text/plain), 13.65 KB, created by
Chris Delahunt
on 2010-03-03 13:33:19 EST
(
hide
)
Description:
proposed patch
Filename:
MIME Type:
Creator:
Chris Delahunt
Created:
2010-03-03 13:33:19 EST
Size:
13.65 KB
patch
obsolete
>Index: foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/AttributeNode.java >=================================================================== >--- foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/AttributeNode.java (revision 6703) >+++ foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/AttributeNode.java (working copy) >@@ -80,8 +80,12 @@ > if (context.hasMemberOfNode()) { > return parentExpression.noneOf(name, new ExpressionBuilder().equal(context.getMemberOfNode().getLeftExpression())); > } >- return outerJoin ? parentExpression.anyOfAllowingNone(name) : >- parentExpression.anyOf(name); >+ //Bug246211: need to use an outer join if specified by the context >+ if ( isOuterJoin() || context.shouldUseOuterJoins() ) { >+ return parentExpression.anyOfAllowingNone(name); >+ } else { >+ return parentExpression.anyOf(name); >+ } > } else { > // check whether collection attribute is required > if (requiresCollectionAttribute()) { >@@ -89,8 +93,9 @@ > context.getParseTreeContext().getQueryInfo(), > getLine(), getColumn(), name); > } >- >- if (context.shouldUseOuterJoins() || isOuterJoin()) { >+ //Bug246211: context changed to have shouldUseOuterJoins set more often. Need to check that it is a reference >+ // mapping before using the outer join >+ if ( isOuterJoin() || (context.shouldUseOuterJoins()&& this.mapping!=null && this.mapping.isForeignReferenceMapping())) { > return parentExpression.getAllowingNull(name); > } else { > return parentExpression.get(name); >Index: foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/ConstructorNode.java >=================================================================== >--- foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/ConstructorNode.java (revision 6703) >+++ foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/ConstructorNode.java (working copy) >@@ -51,14 +51,13 @@ > ReportQuery reportQuery = (ReportQuery)theQuery; > reportQuery.beginAddingConstructorArguments( > getConstructorClass(context.getParseTreeContext())); >+ //bug246211: use outer joins outside the where clause, as inner joins will filter out null results >+ selectContext.useOuterJoins(); > for (Iterator i = constructorItems.iterator(); i.hasNext();) { > Node node = (Node)i.next(); >- if (selectingRelationshipField(node, context)) { >- selectContext.useOuterJoins(); >- } > node.applyToQuery(reportQuery, context); >- selectContext.dontUseOuterJoins(); > } >+ selectContext.dontUseOuterJoins(); > reportQuery.endAddingToConstructorItem(); > } > } >@@ -136,16 +135,6 @@ > > /** > * INTERNAL >- */ >- private boolean selectingRelationshipField(Node node, GenerationContext context) { >- if ((node == null) || !node.isDotNode()) { >- return false; >- } >- return !((DotNode)node).endsWithDirectToField(context); >- } >- >- /** >- * INTERNAL > * Get the string representation of this node. > */ > public String getAsString() { >Index: foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/GroupByNode.java >=================================================================== >--- foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/GroupByNode.java (revision 6703) >+++ foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/GroupByNode.java (working copy) >@@ -76,10 +76,14 @@ > public void addGroupingToQuery(ObjectLevelReadQuery theQuery, GenerationContext context) { > if (theQuery.isReportQuery()) { > Iterator iter = getGroupByItems().iterator(); >+ SelectGenerationContext selectContext = (SelectGenerationContext)context; >+ //bug246211: outer joins are used outside the where clause, as inner joins will filter null results >+ selectContext.useOuterJoins(); > while (iter.hasNext()) { > Node nextNode = (Node)iter.next(); > ((ReportQuery)theQuery).addGrouping(nextNode.generateExpression(context)); > } >+ selectContext.dontUseOuterJoins(); > } > } > >Index: foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/HavingNode.java >=================================================================== >--- foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/HavingNode.java (revision 6703) >+++ foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/HavingNode.java (working copy) >@@ -52,8 +52,12 @@ > */ > public void addHavingToQuery(ObjectLevelReadQuery theQuery, GenerationContext context) { > if (theQuery.isReportQuery()) { >+ //bug246211: outer joins are used outside the where clause, as inner joins will filter null results >+ SelectGenerationContext selectContext = (SelectGenerationContext)context; >+ selectContext.useOuterJoins(); > Expression havingExpression = getHaving().generateExpression(context); > ((ReportQuery)theQuery).setHavingExpression(havingExpression); >+ selectContext.dontUseOuterJoins(); > } > } > >Index: foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/SelectNode.java >=================================================================== >--- foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/SelectNode.java (revision 6703) >+++ foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/SelectNode.java (working copy) >@@ -160,11 +160,11 @@ > } > } > SelectGenerationContext selectContext = (SelectGenerationContext)context; >+ //bug246211: outer joins are used on all nodes in the select, allowing nested DotNodes to each use an outer join >+ selectContext.useOuterJoins(); > for (int i=0;i<selectExpressions.size();i++){ > Node node = (Node)selectExpressions.get(i); >- if (selectingRelationshipField(node, context)) { >- selectContext.useOuterJoins(); >- } >+ > if (node.isAliasableNode() && identifiers != null){ > String alias = (String)identifiers.get(i); > if (alias != null){ >@@ -172,9 +172,8 @@ > } > } > node.applyToQuery(readQuery, context); >- selectContext.dontUseOuterJoins(); > } >- >+ selectContext.dontUseOuterJoins(); > //indicate on the query if "return null if primary key null" > //This means we want nulls returned if we expect an outer join > readQuery.setShouldBuildNullForNullPk(this.hasOneToOneSelected(context)); >@@ -371,17 +370,6 @@ > return false; > } > >- private boolean selectingRelationshipField(Node node, GenerationContext context) { >- if ((node == null) || !node.isDotNode()) { >- return false; >- } >- TypeHelper typeHelper = context.getParseTreeContext().getTypeHelper(); >- Node path = node.getLeft(); >- AttributeNode attribute = (AttributeNode)node.getRight(); >- return typeHelper.isRelationship(path.getType(), >- attribute.getAttributeName()); >- } >- > /** > * Answer true if the SELECT ends in a direct-to-field. > * true: SELECT phone.areaCode >Index: foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/TypeHelperImpl.java >=================================================================== >--- foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/TypeHelperImpl.java (revision 6703) >+++ foundation/org.eclipse.persistence.core/src/org/eclipse/persistence/internal/jpa/parsing/TypeHelperImpl.java (working copy) >@@ -143,10 +143,7 @@ > public boolean isRelationship(Object ownerClass, String attribute) { > DatabaseMapping mapping = > resolveAttributeMapping(ownerClass, attribute); >- return (mapping != null) && >- (mapping.isObjectReferenceMapping() || >- mapping.isOneToManyMapping() || >- mapping.isManyToManyMapping()); >+ return (mapping != null) && mapping.isForeignReferenceMapping(); > } > > /** Returns true if the specified attribute denotes a single valued >Index: jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/jpql/JUnitJPQLComplexTestSuite.java >=================================================================== >--- jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/jpql/JUnitJPQLComplexTestSuite.java (revision 6703) >+++ jpa/eclipselink.jpa.test/src/org/eclipse/persistence/testing/tests/jpa/jpql/JUnitJPQLComplexTestSuite.java (working copy) >@@ -164,6 +164,9 @@ > suite.addTest(new JUnitJPQLComplexTestSuite("complexNavigatingTwoLevelOfEmbeddeds")); > suite.addTest(new JUnitJPQLComplexTestSuite("complexNamedQueryResultPropertiesTest")); > suite.addTest(new JUnitJPQLComplexTestSuite("complexOuterJoinQuery")); >+ suite.addTest(new JUnitJPQLComplexTestSuite("complexCountOuterJoinTest")); >+ suite.addTest(new JUnitJPQLComplexTestSuite("complexCountNestedOuterJoinTest")); >+ suite.addTest(new JUnitJPQLComplexTestSuite("complexOuterJoinGroupByTest")); > > suite.addTest(new JUnitJPQLComplexTestSuite("complexInheritanceTest")); > suite.addTest(new JUnitJPQLComplexTestSuite("complexInheritanceUsingNamedQueryTest")); >@@ -1711,6 +1714,78 @@ > Assert.assertNull ("Complex outer join query (2): unexpected result value (1, 3):", result1[3]); > > } >+ >+ /** >+ * Test bug 246211: Inner join used outside of the where clause causes unintentional filtering of results. >+ * Ie "Select count(a.b) from A a" can never return 0 since all As with 0 Bs get filtered out if an inner join >+ * is used. >+ */ >+ public void complexCountOuterJoinTest() >+ { >+ EntityManager em = createEntityManager(); >+ String jpql = "Select e.lastName, count(e.department) from Employee e group by e.lastName"; >+ Query q = em.createQuery(jpql); >+ List results = q.getResultList(); >+ boolean passed = false; >+ Iterator i = results.iterator(); >+ //check all rows until a row with count=0 is found >+ while (!passed && i.hasNext() ){ >+ Object[] row = (Object[])i.next(); >+ if (((Number)row[1]).intValue() == 0) { >+ passed = true; >+ } >+ } >+ >+ this.assertTrue("complexCountOuterJoinTest did not return results rows with a count of 0", passed); >+ } >+ >+ /** >+ * Test bug 246211: Inner join used outside of the where clause causes unintentional filtering of results. >+ * Ie "Select count(a.b.c) from A a" can never return 0 since all As with 0 Bs get filtered out if an inner join >+ * is used. This test tests that an outer join is used from A->B as well as B->C >+ */ >+ public void complexCountNestedOuterJoinTest() >+ { >+ EntityManager em = createEntityManager(); >+ String jpql = "Select e.lastName, count(e.department.managers) from Employee e group by e.lastName"; >+ Query q = em.createQuery(jpql); >+ List results = q.getResultList(); >+ >+ boolean passed = false; >+ Iterator i = results.iterator(); >+ //check all rows until a row with count=0 is found >+ while (!passed && i.hasNext() ){ >+ Object[] row = (Object[])i.next(); >+ if (((Number)row[1]).intValue() == 0) { >+ passed = true; >+ } >+ } >+ >+ this.assertTrue("complexCountNestedOuterJoinTest did not return results rows with a count of 0", passed); >+ } >+ >+ /** >+ * Test bug 246211: Inner join used outside of the where clause causes unintentional filtering of results. >+ * This tests verifies that the group by statement uses an outer join >+ */ >+ public void complexOuterJoinGroupByTest(){ >+ EntityManager em = createEntityManager(); >+ //Employee to department is a many to one relation. >+ String jpql = "Select count(e), e.department from Employee e group by e.department"; >+ Query q = em.createQuery(jpql); >+ List results = q.getResultList();Employee e; >+ boolean passed = false; >+ Iterator i = results.iterator(); >+ //check all rows until a row with null departments is found >+ while (!passed && i.hasNext() ){ >+ Object[] row = (Object[])i.next(); >+ if ( row[1] == null ){ >+ //verify that employees with a null department are included in the results >+ passed = true; >+ } >+ } >+ this.assertTrue("complexOuterJoinCountTest did not return results rows with a count of 0", passed); >+ } > > // Helper methods and classes for constructor query test cases >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 246211
: 160820