Proposed change to group-by treatment in view entity defs.

[Jacopo Cappellato]

Hi all,

I'm going to commit the first draft (as a proof of concept) for a fully
integrated OFBiz framework for business intelligence.
Before I go, I need to commit the attached patch (or a variant of it).
After the patch will be applied the following behavior will change:
in a view entity definition, the fields with a group-by clause will be
put in the sql "GROUP BY" section if and only if they are also in the
"SELECT" section. Right now the behavior is different: the fields with
group-by="true" in the view entity definition are always put in the
"GROUP BY" section.
Do you think this is going to be a problem?
If the answer is yes (as I suspect), what about adding a new optional
attribute to the <alias/> element: "always-in-group-by" (default to "true")?

For example:

<alias field="productId" group-by="true"/> will work as now (productId
will always be in the group by section).

<alias field="productId" group-by="true" always-in-group-by="false"/>

productId will be in group by only if it is in the select section.

Please, let me know your opinion! Is there a better name for the new
attribute?

Thanks,

Jacopo

Index: framework/entity/src/org/ofbiz/entity/datasource/GenericDAO.java
===================================================================
--- framework/entity/src/org/ofbiz/entity/datasource/GenericDAO.java (revision 584577)
+++ framework/entity/src/org/ofbiz/entity/datasource/GenericDAO.java (working copy)
@@ -711,7 +711,7 @@
         // GROUP BY clause for view-entity
         if (modelEntity instanceof ModelViewEntity) {
             ModelViewEntity modelViewEntity = (ModelViewEntity) modelEntity;
-            String groupByString = modelViewEntity.colNameString(modelViewEntity.getGroupBysCopy(), ", ", "", false);
+            String groupByString = modelViewEntity.colNameString(modelViewEntity.getGroupBysCopy(selectFields), ", ", "", false);
 
             if (UtilValidate.isNotEmpty(groupByString)) {
                 sqlBuffer.append(" GROUP BY ");

[BJ Freeman]

clarification:
so if it is not in the sql group by it would be in the minilang group by.



Jacopo Cappellato sent the following on 10/15/2007 1:19 AM:

> Hi all,
>
> I'm going to commit the first draft (as a proof of concept) for a fully
> integrated OFBiz framework for business intelligence.
> Before I go, I need to commit the attached patch (or a variant of it).
> After the patch will be applied the following behavior will change:
> in a view entity definition, the fields with a group-by clause will be
> put in the sql "GROUP BY" section if and only if they are also in the
> "SELECT" section. Right now the behavior is different: the fields with
> group-by="true" in the view entity definition are always put in the
> "GROUP BY" section.
> Do you think this is going to be a problem?
> If the answer is yes (as I suspect), what about adding a new optional
> attribute to the <alias/> element: "always-in-group-by" (default to
> "true")?
>
> For example:
>
> <alias field="productId" group-by="true"/> will work as now (productId
> will always be in the group by section).
>
> <alias field="productId" group-by="true" always-in-group-by="false"/>
>
> productId will be in group by only if it is in the select section.
>
> Please, let me know your opinion! Is there a better name for the new
> attribute?
>
> Thanks,
>
> Jacopo

[David E Jones]

Wouldn't it be an SQL error, or at least be kind of meaningless, for  
a field to be in the group-by list but not in the select list?

My take is that this is pretty safe to do, and we can probably leave  
that extra attribute out for now. In general in OFBiz if you don't  
select a field it is out of the results, and so generally no need to  
group by it.

I'm no SQL expert though, and maybe there is some funky trick you can  
only effectively do by grouping by fields that aren't selected... my  
imagination isn't coming up with anything for that though... ;)

-David


On Oct 15, 2007, at 2:19 AM, Jacopo Cappellato wrote:

> Hi all,
>
> I'm going to commit the first draft (as a proof of concept) for a  
> fully integrated OFBiz framework for business intelligence.
> Before I go, I need to commit the attached patch (or a variant of it).
> After the patch will be applied the following behavior will change:
> in a view entity definition, the fields with a group-by clause will  
> be put in the sql "GROUP BY" section if and only if they are also  
> in the "SELECT" section. Right now the behavior is different: the  
> fields with group-by="true" in the view entity definition are  
> always put in the "GROUP BY" section.
> Do you think this is going to be a problem?
> If the answer is yes (as I suspect), what about adding a new  
> optional attribute to the <alias/> element: "always-in-group-
> by" (default to "true")?
>
> For example:
>
> <alias field="productId" group-by="true"/> will work as now  
> (productId will always be in the group by section).
>
> <alias field="productId" group-by="true" always-in-group-by="false"/>
>
> productId will be in group by only if it is in the select section.
>
> Please, let me know your opinion! Is there a better name for the  
> new attribute?
>
> Thanks,
>
> Jacopo
> Index: framework/entity/src/org/ofbiz/entity/datasource/
> GenericDAO.java
> ===================================================================
> --- framework/entity/src/org/ofbiz/entity/datasource/
> GenericDAO.java (revision 584577)
> +++ framework/entity/src/org/ofbiz/entity/datasource/
> GenericDAO.java (working copy)
> @@ -711,7 +711,7 @@
>          // GROUP BY clause for view-entity
>          if (modelEntity instanceof ModelViewEntity) {
>              ModelViewEntity modelViewEntity = (ModelViewEntity)  
> modelEntity;
> -            String groupByString = modelViewEntity.colNameString
> (modelViewEntity.getGroupBysCopy(), ", ", "", false);
> +            String groupByString = modelViewEntity.colNameString
> (modelViewEntity.getGroupBysCopy(selectFields), ", ", "", false);
>
>              if (UtilValidate.isNotEmpty(groupByString)) {
>                  sqlBuffer.append(" GROUP BY ");