Thanks for the details Jacques. I was able to reproduce the problem,
and discovered some interesting SQL quirks along the way.
The biggest problem I had with this fix was that it was a serious hack
and involved iterating over all of the results to count them, which
will not perform well for large databases.
In SVN rev 673014 I've committed an alternative that will have the
database do the counting so it performs much better.
-David
On Jun 30, 2008, at 5:54 AM, Jacques Le Roux wrote:
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;
