Hi David,

It's easy to test simply create the following view-entity
<view-entity entity-name="TestGrouping" package-name="org.ofbiz.learning">
<member-entity entity-alias="PA" entity-name="PostalAddress"/>
<alias entity-alias="PA" name="count" field="contactMechId" function="count"/>
<alias entity-alias="PA" name="city" group-by="true"/>
</view-entity>

and check it in both cases in Entity Data Maintenance (numbers of each city)

For ease of testing here is the concerned snippet of code (near 
GenericDAO.java[570]
try {
sqlP.executeQuery();
long count = 0;
ResultSet resultSet = sqlP.getResultSet();
if (resultSet.next()) {
// boolean isGroupBy = false;
// if (modelEntity instanceof ModelViewEntity) {
// ModelViewEntity modelViewEntity = (ModelViewEntity) modelEntity;
// String groupByString = modelViewEntity.colNameString(modelViewEntity.getGroupBysCopy(), ", 
", "", false);
// if (UtilValidate.isNotEmpty(groupByString)) {
// isGroupBy = true;
// }
// }
// if (isGroupBy) {
// while (resultSet.next()) {
// count++;
// }
// } else if (resultSet.next()) {
count = resultSet.getLong(1);
}
return count;

I did not digg into details but I guess the genuine code was written at a time we were using Minerva, and I suppose it has been tested. Could it be that DBCP has a different behaviour regarding this aspect ? Or maybe this bug has never been catched before ?

Jacques

From: "Jacques Le Roux" <[EMAIL PROTECTED]>
David,

There is something, I will write that tomorrow. Hopefully, it will be better 
than the other fix...

Jacques

From: "David E Jones" <[EMAIL PROTECTED]>

I'm going to complain about this change too. It doesn't make a lot of  sense.

Let me describe what I'm reading, just to check: if there is a group  by, then 
iterate through all results to count them instead
of getting  the count result as a single number.

If that is true, it means the database is ignoring the "COUNT(*)" when  there is a 
"GROUP BY", and I've never heard of a problem
like that  before.

So, in fact this seems like it would BREAK a count when there is a  GROUP BY if 
the above statement is not true. It would result
in a  single row with a count value, but the count value would be ignored by  
this code, always returning 1 because there is only
one row returned  (the row with the count).

Is there a test case for this or anything that was not working before  but is 
working after? If there isn't anything OOTB in
OFBiz then let's  write something, like a unit test or something crazy like 
that.

-David


On Jun 27, 2008, at 2:43 AM, [EMAIL PROTECTED] wrote:

Author: jleroux
Date: Fri Jun 27 01:43:00 2008
New Revision: 672191

URL: http://svn.apache.org/viewvc?rev=672191&view=rev
Log:
Fix a Group By bug when using Count (View Entities)

Modified:
   ofbiz/trunk/framework/entity/src/org/ofbiz/entity/datasource/ GenericDAO.java

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/ 
datasource/GenericDAO.java
URL:
http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/datasource/GenericDAO.java?rev=672191&r1=672190&r2=672191&view=diff
= = = = = = = = 
======================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/datasource/ 
GenericDAO.java (original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/datasource/ 
GenericDAO.java Fri Jun 27 01:43:00 2008
@@ -970,7 +970,18 @@
            sqlP.executeQuery();
            long count = 0;
            ResultSet resultSet = sqlP.getResultSet();
-            if (resultSet.next()) {
+            boolean isGroupBy = false;
+            if (modelEntity instanceof ModelViewEntity) {
+              ModelViewEntity modelViewEntity = (ModelViewEntity)  modelEntity;
+              String groupByString =  
modelViewEntity.colNameString(modelViewEntity.getGroupBysCopy(), ",  ", "", 
false);
+
+              if (UtilValidate.isNotEmpty(groupByString)) isGroupBy  = true;
+            }
+
+            if (isGroupBy) {
+              while (resultSet.next()) count++;
+            }
+            else if (resultSet.next()) {
                count = resultSet.getLong(1);
            }
            return count;





Reply via email to