Merge lp:~numerigraphe/openobject-server/trunk-read_group-having into lp:openobject-server

[Numérigraphe]

This branch adds a new parameter named "having" to orm.read_group(), that lets clients pass the information to filter the grouped records based on the sum (or avg, etc.) of a value.
This parameter should contain a domain-like list of tuples, that the ORM will use to add a "HAVING" clause to the SQL query.

For an example of how this feature is useful, please see lp:~numerigraphe/openobject-addons/trunk-stock-analysis-filter-zero, where we use the "having" parameter to filter out results with quantity = 0 in stock analysis. This can only be done after the records have been grouped (the goods may have gone in then out of a location).

This is only really useful if clients support it too. Support for the GTK client has been added in lp:~numerigraphe/openobject-client/trunk-read_group-having.

Lionel Sausin & Philippe Rossi.

[Olivier Dony (Odoo) (odo-openerp)]

Hi Philippe and Lionel,

The need to allow filtering on the _result_ of read_group queries rather than the source rows is dire and has been discussed in the past, so it is great to see you work on it!
Please have a look at comment #11 of bug 743715, I think it describes the context in more details.

Based on what I wrote in that comment, I think we may perhaps find an elegant approach to solving this issue that does not require to modify the OpenERP clients or introduce more context parameters, etc. The idea would be to completely switch the behavior of read_group from using a WHERE clause to a HAVING clause as soon as we are in "analysis" mode, i.e. when `group_by_no_leaf` is passed in the context. It seems a reasonably logical behavior with no much side effects, and it looks like it would certainly work for your stock analysis use case.

What do you guys think? Does it seem to make sense to you too? If yes, would you be willing to validate this approach with a rework of your patch in that direction?

Thanks!

[Numérigraphe (numerigraphe)]

It could indeed simplify the problem. I've pushed a quick hack in that direction at lp:~numerigraphe/openobject-server/trunk-read_group-having-alternate
But when I try it, Postgres complains that I didn't group by date, state and every field I have a filter condition on.
So I don't think it will work for our use case because we need to filter both before ( moves done, move dates...) and after grouping (total quantity).
It's quite possible that I missed something, please just tell me.
Lionel.

[hifly (csnlca)]

Hi Everyone,
I had a test on this great idea. (About one month ago, our customer
submitted the question.)
It works but I want to know how to define the views (maybe search view?)
to pass the having parameter to the function read_group(..... , having) .

Thanks

 Andy Lu

2012/5/7 Numérigraphe <email address hidden>

> It could indeed simplify the problem. I've pushed a quick hack in that
> direction at
> lp:~numerigraphe/openobject-server/trunk-read_group-having-alternate
> But when I try it, Postgres complains that I didn't group by date, state
> and every field I have a filter condition on.
> So I don't think it will work for our use case because we need to filter
> both before ( moves done, move dates...) and after grouping (total
> quantity).
> It's quite possible that I missed something, please just tell me.
> Lionel.
> --
>
> https://code.launchpad.net/~numerigraphe/openobject-server/trunk-read_group-having/+merge/104579
> Your team OpenERP Framework Experts is requested to review the proposed
> merge of lp:~numerigraphe/openobject-server/trunk-read_group-having into
> lp:openobject-server.
>
> _______________________________________________
> Mailing list: https://launchpad.net/~openerp-expert-framework
> Post to : <email address hidden>
> Unsubscribe : https://launchpad.net/~openerp-expert-framework
> More help : https://help.launchpad.net/ListHelp
>

[Numérigraphe (numerigraphe)]

What we propose is to add a context key called "having", containing a "domain-like" list of triplets:
    <filter name="group_lot"
        context="{'group_by':'prodlot_id', 'having': [('product_qty', '&lt;&gt;', 0.0)]}"/>
But of course we always welcome suggestions to improve.
Lionel

[hifly (csnlca)]

Hi Lionel,

Thanks for your reply.
Yes, we need define the search view with context['having'] = [(,,)].
But I think the web or GTK client need to support the 'having' key of
context.
How do this on the client side and can you give me some proposals?

Thanks.

2012/5/9 Numérigraphe <email address hidden>

> What we propose is to add a context key called "having", containing a
> "domain-like" list of triplets:
> <filter name="group_lot"
> context="{'group_by':'prodlot_id', 'having': [('product_qty',
> '&lt;&gt;', 0.0)]}"/>
> But of course we always welcome suggestions to improve.
> Lionel
> --
>
> https://code.launchpad.net/~numerigraphe/openobject-server/trunk-read_group-having/+merge/104579
> Your team OpenERP Framework Experts is requested to review the proposed
> merge of lp:~numerigraphe/openobject-server/trunk-read_group-having into
> lp:openobject-server.
>
> _______________________________________________
> Mailing list: https://launchpad.net/~openerp-expert-framework
> Post to : <email address hidden>
> Unsubscribe : https://launchpad.net/~openerp-expert-framework
> More help : https://help.launchpad.net/ListHelp
>

[Numérigraphe (numerigraphe)]

Hi hifly (csnlca),
We've added support to the GTK client in http://bazaar.launchpad.net/~numerigraphe/openobject-client/trunk-read_group-having/revision/2060.
We don't have plans for the web client yet so we'd be grateful if you give it a try.
Lionel.

[lp:~numerigraphe/openobject-server/trunk-read_group-having]

4151. By Numerigraphe - Lionel Sausin <email address hidden> on 2012-06-05

[FIX] properly initialize context and having in read_group()

Unmerged revisions

4151. By Numerigraphe - Lionel Sausin <email address hidden> on 2012-06-05

[FIX] properly initialize context and having in read_group()

4150. By Philippe Rossi & Lionel Sausin on 2012-05-03

[IMP] orm: add a 'having' parameter to read_group() to filter grouped records

=== modified file 'openerp/osv/orm.py'
--- openerp/osv/orm.py	2012-05-18 14:36:25 +0000
+++ openerp/osv/orm.py	2012-06-05 15:51:35 +0000
@@ -2491,7 +2491,7 @@
                 append_right(all_groups.pop(0))
         return result
 
-    def read_group(self, cr, uid, domain, fields, groupby, offset=0, limit=None, context=None, orderby=False):
+    def read_group(self, cr, uid, domain, fields, groupby, offset=0, limit=None, context=None, orderby=False, having=None):
         """
         Get the list of records in list view grouped by the given ``groupby`` fields
 
@@ -2517,7 +2517,10 @@
                             * if user tries to bypass access rules for read on the requested object
 
         """
-        context = context or {}
+        if context is None:
+            context = {}
+        if having is None:
+            having = []
         self.check_read(cr, uid)
         if not fields:
             fields = self._columns.keys()
@@ -2578,7 +2581,15 @@
         offset_str = offset and ' offset %d' % offset or ''
         if len(groupby_list) < 2 and context.get('group_by_no_leaf'):
             group_count = '_'
-        cr.execute('SELECT min(%s.id) AS id, count(%s.id) AS %s_count' % (self._table, self._table, group_count) + (flist and ',') + flist + ' FROM ' + from_clause + where_clause + gb + limit_str + offset_str, where_clause_params)
+
+        having_conditions = []
+        for field, operator, value in having:
+            group_operator = fget[field].get('group_operator', 'sum')
+            having_conditions.append('%s(%s) %s' % (group_operator, field, operator) + ' %s')
+            where_clause_params.append(value)
+        having_clause = having_conditions and (' HAVING ' + ' AND '.join(having_conditions)) or ''
+                   
+        cr.execute('SELECT min(%s.id) AS id, count(%s.id) AS %s_count' % (self._table, self._table, group_count) + (flist and ',') + flist + ' FROM ' + from_clause + where_clause + gb + having_clause + limit_str + offset_str, where_clause_params)
         alldata = {}
         groupby = group_by
         for r in cr.dictfetchall():