You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacopo Cappellato <ja...@gmail.com> on 2008/06/27 11:03:37 UTC

Re: svn commit: r672191 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/datasource/GenericDAO.java

Hi Jacques,

just curious, what was the bug that you fixed here?

Thanks,

Jacopo

PS: there are some minor formatting issues in the patch


On Jun 27, 2008, at 10:43 AM, jleroux@apache.org 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;
>
>


Re: svn commit: r672191 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/datasource/GenericDAO.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Jacopo,

From: "Jacopo Cappellato" <ja...@gmail.com>
> Jacques,
>
> thanks for your explanation. I don't need extra information from  you... I actually still don't understand this patch, but that's 
> not  really important if you and others are sure it is a correct fix.

Yes, it's a correct fix and have been seriously tested

> By the way, and sorry for being such a pain... there is room for more  formatting enhancements :-)

You don't have to be sorry. Actually it's my sole fault, sorry for that. It's commited in r672355 (and r672357 kind of joke, but 
it's not)

Jacques

> From
>
>>>> +            if (isGroupBy) {
>>>> +              while (resultSet.next()) count++;
>>>> +            }
>>>> +            else if (resultSet.next()) {
>>>
>
>
> to
>>>> +            if (isGroupBy) {
>>>> +              while (resultSet.next()) count++;
>>>> +            } else if (resultSet.next()) {
>>>
>
>
> and probably it would be better to always use { } even for one line  blocks (but this is less important).
>
> Jacopo
>
> On Jun 27, 2008, at 2:00 PM, Jacques Le Roux wrote:
>
>> Hi Jacopo,
>>
>> I just fixed the formatting issues, I did it too fast, sorry.
>> When you were using a group by with a count in a view-entity you did  not get all the possible results. It's easy to try by 
>> creating
>> a specific entity-view and looking for its results in webtools/ entities data maintenance(All).
>>
>> If you are interested I can send you more informations
>>
>> Jacques
>>
>> From: "Jacopo Cappellato" <ja...@gmail.com>
>>> Hi Jacques,
>>>
>>> just curious, what was the bug that you fixed here?
>>>
>>> Thanks,
>>>
>>> Jacopo
>>>
>>> PS: there are some minor formatting issues in the patch
>>>
>>>
>>> On Jun 27, 2008, at 10:43 AM, jleroux@apache.org 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;
>>>>
>>>>
>>>
>>
> 


Re: svn commit: r672191 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/datasource/GenericDAO.java

Posted by Jacopo Cappellato <ja...@gmail.com>.
Jacques,

thanks for your explanation. I don't need extra information from  
you... I actually still don't understand this patch, but that's not  
really important if you and others are sure it is a correct fix.
By the way, and sorry for being such a pain... there is room for more  
formatting enhancements :-)

From

>>> +            if (isGroupBy) {
>>> +              while (resultSet.next()) count++;
>>> +            }
>>> +            else if (resultSet.next()) {
>>


to
>>> +            if (isGroupBy) {
>>> +              while (resultSet.next()) count++;
>>> +            } else if (resultSet.next()) {
>>


and probably it would be better to always use { } even for one line  
blocks (but this is less important).

Jacopo

On Jun 27, 2008, at 2:00 PM, Jacques Le Roux wrote:

> Hi Jacopo,
>
> I just fixed the formatting issues, I did it too fast, sorry.
> When you were using a group by with a count in a view-entity you did  
> not get all the possible results. It's easy to try by creating
> a specific entity-view and looking for its results in webtools/ 
> entities data maintenance(All).
>
> If you are interested I can send you more informations
>
> Jacques
>
> From: "Jacopo Cappellato" <ja...@gmail.com>
>> Hi Jacques,
>>
>> just curious, what was the bug that you fixed here?
>>
>> Thanks,
>>
>> Jacopo
>>
>> PS: there are some minor formatting issues in the patch
>>
>>
>> On Jun 27, 2008, at 10:43 AM, jleroux@apache.org 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;
>>>
>>>
>>
>


Re: svn commit: r672191 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/datasource/GenericDAO.java

Posted by Jacques Le Roux <ja...@les7arts.com>.
Hi Jacopo,

I just fixed the formatting issues, I did it too fast, sorry.
When you were using a group by with a count in a view-entity you did not get all the possible results. It's easy to try by creating
a specific entity-view and looking for its results in webtools/entities data maintenance(All).

If you are interested I can send you more informations

Jacques

From: "Jacopo Cappellato" <ja...@gmail.com>
> Hi Jacques,
>
> just curious, what was the bug that you fixed here?
>
> Thanks,
>
> Jacopo
>
> PS: there are some minor formatting issues in the patch
>
>
> On Jun 27, 2008, at 10:43 AM, jleroux@apache.org 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;
>>
>>
>