Merge lp:~openerp-dev/openobject-addons/7.0-pos-performance-cbi into lp:openobject-addons/7.0

[Olivier Dony (Odoo)]

Fixes the algorithm for pricelist calculation to drastically improve performance.
This is still a work-in-progress (API needs to be corrected not to break the stable API), but currently brings down the time to load 28k products in the POS interfaces (that caches the prices) from 15min to 12sec.

Any feedback appreciated.

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

The WITH RECURSIVE query requires postgres 8.4, so we might keep the original query for OpenERP 6.1 or slightly improve it. Without it the POS can still load 28k products in 40s.

[qdp (OpenERP) (qdp)]

let me be a reviewer, this piece of code is soooo exciting o_O

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

Here are my comments after a surface scan (I did not test):

1/ First, thanks for restoring the compatibility with the original 7.0 API. However the way price_get_multi now works is not equivalent to the original code. As I understand it, price_get_multi() was working as follows:
  - IN: `pricelist_ids`: list of pricelists to compute
  - IN: `products_by_qty_by_partner`: list of (product,qty,partner) to compute, with each product supposed to be provided only once
  - OUT: { prod_id1: {pricelist_id1: price, pricelist_id2: price},
           prod_id2: {pricelist_id1: price, pricelist_id2: price},
           (...)
           'item_id': id_of_last_pricelist_line_matched }

where the returned prices will match the qty and partner that was provided *in the same tuple*.
Yes this API is nonsense (especially the 'item_id') and due to debatable past refactorings, but that's how it is :-(

With the current state of this merge proposal it seems that `products_by_qty_by_partner` is only used to get the list of products to compute, while the qty and partner are forced to be the ones provided as keyword arguments. So it will work only if partner and qty are the same for all products being computed.

I understand that it is hard to optimize the query for fetching the `product.pricelist.item`s if the qty and partner can be different for each product, so maybe we should not do it that way.

Could we instead do the following:
 - the price_get_batch() method becomes responsible for computing the price for several products for a single `qty` and `partner`, and all the code moves in it. The signature would be:
     def price_get_batch(self, cr, uid, ids, prod_ids, qty, partner=None, context=None)
 - the price_get_multi() method keeps the original 7.0 signature (no extra parameter), but it groups all entries from `products_by_qty_by_partner` with the same `qty` and `partner` and passes all the groups to price_get_batch().
 - this way the API still works exactly as before and the optimization works too because the POS will call it with qty=1 and partner=None for all products, so there will only be one group to pass to price_get_batch().

2/ what is the point of adding the extra argument "pricelist_version_ids" to price_get_batch() and price_get_multi()? It forces you to copy the search() for versions in product._product_price() and in pricelist.price_get(). On the other hand it seems to be only computed once, so it could still be done at the beginning of price_get_multi(). Why does price_get() now ignore the `date` in context?

4/ the big `if` with the recursive query should probably be moved to a separate private method that abstracts the logic of computing res_per_prod. i.e.:
   res_per_prod = self._pricelist_table(self, cr, uid, pricelist_ids, products, partner, qty, ...)
The _pricelist_table() method could then be split into 2 methods _pricelist_table_pg83() and _pricelist_table_pg84() that implement each variant of the query.

5/ (not in your code, was already present) l.433-449: the logic to find the proper supplier for a given product should not be hardcoded, the product.product model has a `seller_id` function field that provides this info, so it should be...

review: Needs Fixing (surface)

[Kevin Shenk (batonac)]

If this code is fixed and production ready, when will it be merged into the main edition?

[van der Essen Frédéric (OpenERP) (fva-openerp)]

Any estimation on when this is going to be merged ? This bug is a showstopper for a _lot_ of users.

[Fabien (Open ERP) (fp-tinyerp)]

This was an important improvement, but a better fix has been made in trunk-website-al.

review: Disapprove

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

The optimized WITH RECURSIVE query and the workaround for seller_info_id field are still quite useful on top of the future trunk improvement [1], so setting back to Needs Review so that at least those changes are reviewed at some point for trunk.
The branch is also usable as is for 7.0 in the mean time (and probably 6.1), for customers affected by slow performance during POS initial load.

[1] http://bazaar.launchpad.net/~<email address hidden>

[Eneldo Serrata (eneldoserrata)]

Hello Oliver i try to patch but i get conflict i'm using Versión 7.0-20131224-002412 any recommendation thanks

review: Needs Fixing

Unmerged revisions

8605. By Chris Biersbach (OpenERP) on 2013-03-01

[FIX] Now the fall-back to 8.3 is also functional again

8604. By Chris Biersbach (OpenERP) on 2013-03-01

[IMP] Added a comment

8603. By Chris Biersbach (OpenERP) on 2013-03-01

[IMP] Confusing read and search is a surefire sign you do not drink enough coffee...

8602. By Chris Biersbach (OpenERP) on 2013-03-01

[IMP] Removed some remaining debug code

8601. By Chris Biersbach (OpenERP) on 2013-03-01

[IMP] This takes into account the comments by ODO... various refactorings and small fixes

8600. By Chris Biersbach (OpenERP) on 2013-02-26

[IMP] Changed some method signatures and call to be in sync with the situation before my fixes

8599. By Chris Biersbach (OpenERP) on 2013-02-26

[FIX] small bug in the fall-back code

8598. By Chris Biersbach (OpenERP) on 2013-02-26

[IMP] Refactored the codeto include the fallback gor PostgreSQL prior to 8.4

8597. By Chris Biersbach (OpenERP) on 2013-02-26

[IMP] last small glitch, now all tests should go through. Also removed the debug code in the JS

8596. By Chris Biersbach (OpenERP) on 2013-02-25

[IMP] Another small improvement

=== modified file 'point_of_sale/static/src/js/models.js'
--- point_of_sale/static/src/js/models.js	2013-01-28 17:18:00 +0000
+++ point_of_sale/static/src/js/models.js	2013-03-01 15:13:44 +0000
@@ -94,11 +94,9 @@
         // loads all the needed data on the sever. returns a deferred indicating when all the data has loaded. 
         load_server_data: function(){
             var self = this;
-
             var loaded = self.fetch('res.users',['name','company_id'],[['id','=',this.session.uid]]) 
                 .then(function(users){
                     self.set('user',users[0]);
-
                     return self.fetch('res.company',
                     [
                         'currency_id',
@@ -113,16 +111,12 @@
                     [['id','=',users[0].company_id[0]]]);
                 }).then(function(companies){
                     self.set('company',companies[0]);
-
                     return self.fetch('res.partner',['contact_address'],[['id','=',companies[0].partner_id[0]]]);
                 }).then(function(company_partners){
                     self.get('company').contact_address = company_partners[0].contact_address;
-
                     return self.fetch('res.currency',['symbol','position','rounding','accuracy'],[['id','=',self.get('company').currency_id[0]]]);
                 }).then(function(currencies){
-                    console.log('Currency:',currencies[0]);
                     self.set('currency',currencies[0]);
-
                     return self.fetch('product.uom', null, null);
                 }).then(function(units){
                     self.set('units',units);
@@ -131,23 +125,18 @@
                         units_by_id[units[i].id] = units[i];
                     }
                     self.set('units_by_id',units_by_id);
-                    
                     return self.fetch('product.packaging', null, null);
                 }).then(function(packagings){
                     self.set('product.packaging',packagings);
-                    
                     return self.fetch('res.users', ['name','ean13'], [['ean13', '!=', false]]);
                 }).then(function(users){
                     self.set('user_list',users);
-
                     return self.fetch('res.partner', ['name','ean13'], [['ean13', '!=', false]]);
                 }).then(function(partners){
                     self.set('partner_list',partners);
-
                     return self.fetch('account.tax', ['amount', 'price_include', 'type']);
                 }).then(function(taxes){
                     self.set('taxes', taxes);
-
                     return self.fetch(
                         'pos.session', 
                         ['id', 'journal_ids','name','user_id','config_id','start_at','stop_at'],
@@ -155,7 +144,6 @@
                     );
                 }).then(function(sessions){
                     self.set('pos_session', sessions[0]);
-
                     return self.fetch(
                         'pos.config',
                         ['name','journal_ids','shop_id','journal_id',
@@ -172,7 +160,6 @@
                     self.iface_vkeyboard           =  !!pos_config.iface_vkeyboard; 
                     self.iface_self_checkout       =  !!pos_config.iface_self_checkout;
                     self.iface_cashdrawer          =  !!pos_config.iface_cashdrawer;
-
                     return self.fetch('sale.shop',[],[['id','=',pos_config.shop_id[0]]]);
                 }).then(function(shops){
                     self.set('shop',shops[0]);
@@ -180,11 +167,9 @@
                     return self.fetch('product.packaging',['ean','product_id']);
                 }).then(function(packagings){
                     self.db.add_packagings(packagings);
-
                     return self.fetch('pos.category', ['id','name','parent_id','child_id','image'])
                 }).then(function(categories){
                     self.db.add_categories(categories);
-
                     return self.fetch(
                         'product.product', 
                         ['name', 'list_price','price','pos_categ_id', 'taxes_id', 'ean13', 
@@ -194,7 +179,6 @@
                     );
                 }).then(function(products){
                     self.db.add_products(products);
-
                     return self.fetch(
                         'account.bank.statement',
                         ['account_id','currency','journal_id','state','name','user_id','pos_session_id'],
@@ -209,7 +193,6 @@
                     return self.fetch('account.journal', undefined, [['id','in', journals]]);
                 }).then(function(journals){
                     self.set('journals',journals);
-
                     // associate the bank statements with their journals. 
                     var bank_statements = self.get('bank_statements');
                     for(var i = 0, ilen = bank_statements.length; i < ilen; i++){

=== modified file 'product/pricelist.py'
--- product/pricelist.py	2012-12-06 14:56:32 +0000
+++ product/pricelist.py	2013-03-01 15:13:44 +0000
@@ -125,93 +125,114 @@
         'active': lambda *a: 1,
         "currency_id": _get_currency
     }
-
-    #def price_get_multi(self, cr, uid, product_ids, context=None):
-    def price_get_multi(self, cr, uid, pricelist_ids, products_by_qty_by_partner, context=None):
-        """multi products 'price_get'.
-           @param pricelist_ids:
-           @param products_by_qty:
-           @param partner:
-           @param context: {
-             'date': Date of the pricelist (%Y-%m-%d),}
-           @return: a dict of dict with product_id as key and a dict 'price by pricelist' as value
-        """
-
-        def _create_parent_category_list(id, lst):
-            if not id:
-                return []
-            parent = product_category_tree.get(id)
-            if parent:
-                lst.append(parent)
-                return _create_parent_category_list(parent, lst)
-            else:
-                return lst
-        # _create_parent_category_list
-
-        if context is None:
-            context = {}
-
-        date = time.strftime('%Y-%m-%d')
-        if 'date' in context:
-            date = context['date']
-
-        currency_obj = self.pool.get('res.currency')
-        product_obj = self.pool.get('product.product')
+    
+    def _pricelist_table(self, cr, uid, pricelist_ids, pricelist_version_ids, prod_ids, products_dict, partner, qty, context=None):
+        """ Calls a different method based on the version of PostgreSQL that is installed """
+    
+        # Recursive queries are only supported in PostgreSQL versions 8.4 or newer
+        if cr._cnx.server_version >= 80400:
+            return self._pricelist_table_pg84(cr, uid, pricelist_ids, pricelist_version_ids, prod_ids, partner, qty, context=context)
+        # fall back to the non-recursive query 
+        else:
+            return self._pricelist_table_pg83(cr, uid, pricelist_ids, pricelist_version_ids, prod_ids, products_dict, partner, qty, context=context)
+            
+    def _pricelist_table_pg84(self, cr, uid, pricelist_ids, pricelist_version_ids, prod_ids, partner, qty, context=None):
+        """ Version for PostgreSQL >= 8.4 """ 
+         
+        res_per_prod = {}
+    
+        if partner:
+            partner_where = 'base <> -2 OR %s IN (SELECT name FROM product_supplierinfo WHERE product_id = pp.product_tmpl_id) '
+            partner_args = (partner,)
+        else:         
+            partner_where = 'base <> -2 '
+            partner_args = ()
+
+        for pricelist_id in pricelist_ids:
+
+            cr.execute(
+                # -- categorytree return (category id, category id + ids of parents categories)
+                'WITH RECURSIVE categorytree(id, parent_ids) AS ( '
+                    'SELECT id, ARRAY[id]::int[] '
+                    'FROM product_category '
+                    'WHERE parent_id IS NULL '
+                'UNION ALL '
+                    'SELECT c.id, ct.parent_ids || c.id '
+                    'FROM product_category c '
+                    'JOIN categorytree ct ON ct.id = c.parent_id '
+                ') '
+                'SELECT pp.id AS requested_product_id, i.*, pl.currency_id, pl.id AS pricelist '
+                'FROM product_product pp '
+                'LEFT JOIN product_template pt ON (pp.product_tmpl_id = pt.id) '
+                'LEFT JOIN product_pricelist_item AS i ON ( '
+                                                     '((i.product_tmpl_id IS NOT NULL AND i.product_tmpl_id = pp.product_tmpl_id)'
+                                                       'OR (i.product_id IS NOT NULL AND i.product_id = pp.id) '
+                                                       'OR (i.categ_id IS NOT NULL AND  i.categ_id IN (SELECT unnest(parent_ids) FROM categorytree WHERE id = pt.categ_id)) '
+                                                       'OR (i.product_tmpl_id IS NULL AND i.product_id IS NULL AND i.categ_id IS NULL) '
+                                                     ' )'
+                                                     'AND (' + partner_where + ') '
+                                                      ') '
+                'LEFT JOIN product_pricelist_version AS v ON (i.price_version_id = v.id) '
+                'LEFT JOIN product_pricelist AS pl ON (v.pricelist_id = pl.id) '
+                'WHERE pp.id in %s '
+                    'AND price_version_id = %s '
+                    'AND (min_quantity IS NULL OR min_quantity <= %s) '
+                'ORDER BY sequence',
+                partner_args + (tuple(prod_ids),) + (pricelist_version_ids[0], qty))
+
+            for row in cr.dictfetchall():
+                res_per_prod.setdefault(row['requested_product_id'], [])   
+                res_per_prod[row['requested_product_id']].append(row)
+        
+        return res_per_prod   
+
+    def _pricelist_table_pg83(self, cr, uid, pricelist_ids, pricelist_version_ids, prod_ids, products_dict, partner, qty, context=None):
+        """ Version for PostgreSQL <= 8.3 """
+        
+        res_per_prod = {}
+           
         product_category_obj = self.pool.get('product.category')
-        product_uom_obj = self.pool.get('product.uom')
-        supplierinfo_obj = self.pool.get('product.supplierinfo')
-        price_type_obj = self.pool.get('product.price.type')
-
-        # product.pricelist.version:
-        if not pricelist_ids:
-            pricelist_ids = self.pool.get('product.pricelist').search(cr, uid, [], context=context)
-
-        pricelist_version_ids = self.pool.get('product.pricelist.version').search(cr, uid, [
-                                                        ('pricelist_id', 'in', pricelist_ids),
-                                                        '|',
-                                                        ('date_start', '=', False),
-                                                        ('date_start', '<=', date),
-                                                        '|',
-                                                        ('date_end', '=', False),
-                                                        ('date_end', '>=', date),
-                                                    ])
-        if len(pricelist_ids) != len(pricelist_version_ids):
-            raise osv.except_osv(_('Warning!'), _("At least one pricelist has no active version !\nPlease create or activate one."))
-
-        # product.product:
-        product_ids = [i[0] for i in products_by_qty_by_partner]
-        #products = dict([(item['id'], item) for item in product_obj.read(cr, uid, product_ids, ['categ_id', 'product_tmpl_id', 'uos_id', 'uom_id'])])
-        products = product_obj.browse(cr, uid, product_ids, context=context)
-        products_dict = dict([(item.id, item) for item in products])
-
-        # product.category:
-        product_category_ids = product_category_obj.search(cr, uid, [])
-        product_categories = product_category_obj.read(cr, uid, product_category_ids, ['parent_id'])
-        product_category_tree = dict([(item['id'], item['parent_id'][0]) for item in product_categories if item['parent_id']])
-
-        results = {}
-        for product_id, qty, partner in products_by_qty_by_partner:
-            for pricelist_id in pricelist_ids:
-                price = False
-
-                tmpl_id = products_dict[product_id].product_tmpl_id and products_dict[product_id].product_tmpl_id.id or False
-
-                categ_id = products_dict[product_id].categ_id and products_dict[product_id].categ_id.id or False
+            
+        if partner:
+            partner_where = 'base <> -2 OR %s IN (SELECT name FROM product_supplierinfo WHERE product_id = %s) '
+        else:
+            partner_where = 'base <> -2 '
+                                
+        for pricelist_id in pricelist_ids:
+
+            product_category_ids = product_category_obj.search(cr, uid, [])
+            product_categories = product_category_obj.read(cr, uid, product_category_ids, ['parent_id'])
+            product_category_tree = dict([(item['id'], item['parent_id'][0]) for item in product_categories if item['parent_id']])
+            
+            def _create_parent_category_list(id, lst):
+                if not id:
+                    return []
+                parent = product_category_tree.get(id)
+                if parent:
+                    lst.append(parent)
+                    return _create_parent_category_list(parent, lst)
+                else:
+                    return lst
+            
+            for product_id in prod_ids:
+
+                tmpl_id = products_dict[product_id]['product_tmpl_id'] and products_dict[product_id]['product_tmpl_id'][0] or False
+
+                categ_id = products_dict[product_id]['categ_id'] and products_dict[product_id]['categ_id'][0] or False
                 categ_ids = _create_parent_category_list(categ_id, [categ_id])
+
                 if categ_ids:
                     categ_where = '(categ_id IN (' + ','.join(map(str, categ_ids)) + '))'
                 else:
                     categ_where = '(categ_id IS NULL)'
 
                 if partner:
-                    partner_where = 'base <> -2 OR %s IN (SELECT name FROM product_supplierinfo WHERE product_id = %s) '
                     partner_args = (partner, tmpl_id)
                 else:
-                    partner_where = 'base <> -2 '
                     partner_args = ()
-
+                    
                 cr.execute(
-                    'SELECT i.*, pl.currency_id '
+                    'SELECT i.*, pl.currency_id , pl.id AS pricelist '
                     'FROM product_pricelist_item AS i, '
                         'product_pricelist_version AS v, product_pricelist AS pl '
                     'WHERE (product_tmpl_id IS NULL OR product_tmpl_id = %s) '
@@ -223,87 +244,212 @@
                         'AND i.price_version_id = v.id AND v.pricelist_id = pl.id '
                     'ORDER BY sequence',
                     (tmpl_id, product_id) + partner_args + (pricelist_version_ids[0], qty))
-                res1 = cr.dictfetchall()
-                uom_price_already_computed = False
-                for res in res1:
-                    if res:
-                        if res['base'] == -1:
-                            if not res['base_pricelist_id']:
-                                price = 0.0
-                            else:
-                                price_tmp = self.price_get(cr, uid,
-                                        [res['base_pricelist_id']], product_id,
-                                        qty, context=context)[res['base_pricelist_id']]
-                                ptype_src = self.browse(cr, uid, res['base_pricelist_id']).currency_id.id
-                                uom_price_already_computed = True
-                                price = currency_obj.compute(cr, uid, ptype_src, res['currency_id'], price_tmp, round=False)
-                        elif res['base'] == -2:
-                            # this section could be improved by moving the queries outside the loop:
-                            where = []
-                            if partner:
-                                where = [('name', '=', partner) ]
-                            sinfo = supplierinfo_obj.search(cr, uid,
-                                    [('product_id', '=', tmpl_id)] + where)
+
+                res_per_prod.setdefault(product_id, [])
+                row = cr.dictfetchone()
+                row['requested_product_id'] = product_id
+                res_per_prod[product_id].append(row)
+                
+        return res_per_prod
+
+    def price_get_batch(self, cr, uid, pricelist_ids, prod_ids, qty, partner=None, context=None):
+        """ Return the price for the products in prod_ids according to the pricelists, qty and partner provided
+            This method should only be called directly when multiple products' prices are needed and the quantity and partner are the same for all the products.
+           @param context: {
+             'date': Date of the pricelist (%Y-%m-%d), used to compute the correct pricelist version}
+           @return: a dict of dict with product_id as key and a dict 'price by pricelist' as value
+        """
+
+        if context is None:
+            context = {}
+
+        date = context.get('date') and context['date'] or time.strftime('%Y-%m-%d')
+
+        if not pricelist_ids:
+            pricelist_ids = self.search(cr, uid, [], context=context)
+            
+        pricelist_version_ids = self.pool.get('product.pricelist.version').search(cr, uid, [
+                                                        ('pricelist_id', 'in', pricelist_ids),
+                                                        '|',
+                                                        ('date_start', '=', False),
+                                                        ('date_start', '<=', date),
+                                                        '|',
+                                                        ('date_end', '=', False),
+                                                        ('date_end', '>=', date),                                                    
+                                                        ])
+
+        if type(pricelist_version_ids) == int:
+            pricelist_version_ids = [pricelist_version_ids]
+
+        if len(pricelist_ids) != len(pricelist_version_ids):
+            raise osv.except_osv(_('Warning!'), _("At least one pricelist has no active version !\nPlease create or activate one!\n"))
+            
+        currency_obj = self.pool.get('res.currency')
+        product_obj = self.pool.get('product.product')
+        product_uom_obj = self.pool.get('product.uom')
+        supplierinfo_obj = self.pool.get('product.supplierinfo')
+        price_type_obj = self.pool.get('product.price.type')
+
+        product_data = product_obj.read(cr, uid, prod_ids, ['id', 'uos_id', 'uom_id', 'price_margin', 'price_extra', 'list_price', 'standard_price', 'product_tmpl_id', 'categ_id'], context=context)
+        products_dict = {}
+
+        for product in product_data:
+            products_dict[product['id']] = product
+
+        pricelists = {}
+        suppliers = {}
+        price_types = {}
+        categ_ids_dict = {}
+        currencies = {}
+
+        results = {}
+
+        res_per_prod = self._pricelist_table(cr, uid, pricelist_ids, pricelist_version_ids, prod_ids, products_dict, partner, qty, context=context)
+
+        for product_id in prod_ids:
+            price = False
+
+            uom_price_already_computed = False
+            
+            chosen_pricelist = False
+            
+            for res in res_per_prod[product_id]:
+                if res and res['product_id'] in (None, product_id):
+                    if res['base'] == -1:
+                        if not res['base_pricelist_id']:
                             price = 0.0
-                            if sinfo:
-                                qty_in_product_uom = qty
-                                product_default_uom = product_obj.read(cr, uid, [product_id], ['uom_id'])[0]['uom_id'][0]
-                                supplier = supplierinfo_obj.browse(cr, uid, sinfo, context=context)[0]
-                                seller_uom = supplier.product_uom and supplier.product_uom.id or False
-                                if seller_uom and product_default_uom and product_default_uom != seller_uom:
-                                    uom_price_already_computed = True
-                                    qty_in_product_uom = product_uom_obj._compute_qty(cr, uid, product_default_uom, qty, to_uom_id=seller_uom)
-                                cr.execute('SELECT * ' \
-                                        'FROM pricelist_partnerinfo ' \
-                                        'WHERE suppinfo_id IN %s' \
-                                            'AND min_quantity <= %s ' \
-                                        'ORDER BY min_quantity DESC LIMIT 1', (tuple(sinfo),qty_in_product_uom,))
-                                res2 = cr.dictfetchone()
-                                if res2:
-                                    price = res2['price']
                         else:
-                            price_type = price_type_obj.browse(cr, uid, int(res['base']))
+                            price_tmp = self.price_get(cr, uid,
+                                    [res['base_pricelist_id']], product_id,
+                                    qty, context=context)[res['base_pricelist_id']]
+                            if not pricelists.get(res['base_pricelist_id']):
+                                pricelists[res['base_pricelist_id']] = self.browse(cr, uid, res['base_pricelist_id'])
+                            ptype_src = pricelists[res['base_pricelist_id']].currency_id
                             uom_price_already_computed = True
-                            price = currency_obj.compute(cr, uid,
-                                    price_type.currency_id.id, res['currency_id'],
-                                    product_obj.price_get(cr, uid, [product_id],
-                                    price_type.field, context=context)[product_id], round=False, context=context)
-
-                        if price is not False:
-                            price_limit = price
-                            price = price * (1.0+(res['price_discount'] or 0.0))
-                            price = rounding(price, res['price_round']) #TOFIX: rounding with tools.float_rouding
-                            price += (res['price_surcharge'] or 0.0)
-                            if res['price_min_margin']:
-                                price = max(price, price_limit+res['price_min_margin'])
-                            if res['price_max_margin']:
-                                price = min(price, price_limit+res['price_max_margin'])
-                            break
+                            if not currencies.get(res['currency_id']):
+                                currencies[res['currency_id']] = currency_obj.browse(cr, uid, res['currency_id'])
+                            price = currency_obj.compute(cr, uid, ptype_src, currencies[res['currency_id']], price_tmp, round=False)
+                    # this section could be improved by moving the queries outside the loop:
+                    elif res['base'] == -2:
+                        # /!\ this second read is currently needed, because including seller_info_id in the batch read leads to a huge performance overhead
+                        # (the call to name_get slows things 
+                        products_dict[product_id]['seller_info_id'] = product_obj.read(cr, uid, product_id, ['seller_info_id'], context=context)['seller_info_id']
+                        sinfo = products_dict[product_id]['seller_info_id'] and products_dict[product_id]['seller_info_id'][0] or False
+                        price = 0.0
+                        if sinfo:
+                            qty_in_product_uom = qty
+                            product_default_uom = products_dict[product_id]['uom_id'] and products_dict[product_id]['uom_id'][0] or False                      
+                            if not suppliers.get(sinfo):
+                                suppliers[sinfo] = supplierinfo_obj.browse(cr, uid, sinfo, context=context)
+                            supplier = suppliers[sinfo]
+                            seller_uom = supplier.product_uom and supplier.product_uom.id or False
+                            if seller_uom and product_default_uom and product_default_uom != seller_uom:
+                                uom_price_already_computed = True
+                                qty_in_product_uom = product_uom_obj._compute_qty(cr, uid, product_default_uom, qty, to_uom_id=seller_uom)   
+                              
+                            cr.execute('SELECT * ' \
+                                    'FROM pricelist_partnerinfo ' \
+                                    'WHERE suppinfo_id IN %s' \
+                                        'AND min_quantity <= %s ' \
+                                    'ORDER BY min_quantity DESC LIMIT 1', (tuple([sinfo]),qty_in_product_uom,))
+                            res2 = cr.dictfetchone()
+                            if res2:
+                                price = res2['price']
 
                     else:
-                        # False means no valid line found ! But we may not raise an
-                        # exception here because it breaks the search
-                        price = False
-
-                if price:
-                    results['item_id'] = res['id']
-                    if 'uom' in context and not uom_price_already_computed:
-                        product = products_dict[product_id]
-                        uom = product.uos_id or product.uom_id
-                        price = product_uom_obj._compute_price(cr, uid, uom.id, price, context['uom'])
-
-                if results.get(product_id):
-                    results[product_id][pricelist_id] = price
+                        index = int(res['base'])
+                        if not price_types.get(index):
+                            price_types[index] = price_type_obj.browse(cr, uid, index)
+                        price_type = price_types[index]
+                        uom_price_already_computed = True
+                        currency_id = res['currency_id']
+                        if not currencies.get(currency_id):
+                                currencies[currency_id] = currency_obj.browse(cr, uid, currency_id)
+                        price = currency_obj.compute(cr, uid,
+                                price_type.currency_id, currencies[currency_id],
+                                product_obj.price_get(cr, uid, [product_id],
+                                price_type.field, context=context, products=[products_dict[product_id]])[product_id], round=False, context=context)
+
+                    if price is not False:
+                        price_limit = price
+                        price = price * (1.0+(res['price_discount'] or 0.0))
+                        price = rounding(price, res['price_round']) #TOFIX: rounding with tools.float_rouding
+                        price += (res['price_surcharge'] or 0.0)
+                        if res['price_min_margin']:
+                            price = max(price, price_limit+res['price_min_margin'])
+                        if res['price_max_margin']:
+                            price = min(price, price_limit+res['price_max_margin'])
+                        chosen_pricelist = res['pricelist']
+                        break
+
                 else:
-                    results[product_id] = {pricelist_id: price}
+                    # False means no valid line found ! But we may not raise an
+                    # exception here because it breaks the search
+                    price = False
+
+            if price:
+                if 'uom' in context and not uom_price_already_computed:
+                    product = products_dict[product_id]
+                    uom = product['uos_id'] or product['uom_id']
+                    price = product_uom_obj._compute_price(cr, uid, uom.id, price, context['uom'])
+                if not results.get(product_id):
+                    results[product_id] = {}
+                results[product_id][chosen_pricelist] = price
 
         return results
 
     def price_get(self, cr, uid, ids, prod_id, qty, partner=None, context=None):
-        res_multi = self.price_get_multi(cr, uid, pricelist_ids=ids, products_by_qty_by_partner=[(prod_id, qty, partner)], context=context)
-        res = res_multi[prod_id]
-        res.update({'item_id': {ids[-1]: res_multi.get('item_id', ids[-1])}})
-        return res
+        """ Return the price for one product according to the pricelists, qty and partner provided 
+            This method should be called when a single product's price is needed.
+            
+            @return: a dict 'price by pricelist'
+        """
+
+        if isinstance(prod_id, int):
+            prod_id = [prod_id]
+
+        if isinstance(ids, int):
+            ids = [ids]
+
+        return self.price_get_batch(cr, uid, ids, prod_id, qty, partner=partner, context=context)[prod_id[0]]
+
+    def price_get_multi(self, cr, uid, pricelist_ids, products_by_qty_by_partner, context=None):
+        """ Returns the prices for the products in products_by_qty_by_partner according to the pricelists in pricelist_ids
+            and the provided qty and partner.
+            This method should be called when multiple products' prices are needed and the quantities and partners differ.
+            
+           @param pricelist_ids: The pricelists to be considered in the computations
+           @param products_by_qty_by_partner: a list of tuples (product_id, qty, partner_id), specifying for each product the quantity and partner
+           @param context: {
+             'date': Date of the pricelist (%Y-%m-%d), used to compute the correct pricelist version}
+           @return: a dict of dict with product_id as key and a dict 'price by pricelist' as value
+        """
+
+        groups = {}
+        res= {}
+
+        # split products_by_qty_by_partner into groups with equal qty and partner
+        # structure: dictionary with qty as keys of dictionary with partner as keys of lists of ids
+        for product, qty, partner in products_by_qty_by_partner:
+            if not groups.get(qty):
+                groups[qty] = {}
+            if not groups[qty].get(partner):
+                groups[qty][partner] = []
+            # all products should be unique, but it can't hurt to do this test anyway
+            if not product in groups[qty][partner]:
+                groups[qty][partner].append(product)
+
+        # call price_get_batch for every group, merge results into res dictionary  
+        for qty in groups:
+            for partner in groups[qty]:
+                current_res = self.price_get_batch(cr, uid, ids, groups[qty][partner], qty, partner=partner, context=context)
+                for product in current_res:
+                    if not res.get(product):
+                        res[product] = {}
+                    for pricelist in current_res[product]:
+                        res[product][pricelist] = current_res[product][pricelist]
+
+        return res 
 
 product_pricelist()
 
@@ -405,13 +551,13 @@
     _columns = {
         'name': fields.char('Rule Name', size=64, help="Explicit rule name for this pricelist line."),
         'price_version_id': fields.many2one('product.pricelist.version', 'Price List Version', required=True, select=True, ondelete='cascade'),
-        'product_tmpl_id': fields.many2one('product.template', 'Product Template', ondelete='cascade', help="Specify a template if this rule only applies to one product template. Keep empty otherwise."),
-        'product_id': fields.many2one('product.product', 'Product', ondelete='cascade', help="Specify a product if this rule only applies to one product. Keep empty otherwise."),
-        'categ_id': fields.many2one('product.category', 'Product Category', ondelete='cascade', help="Specify a product category if this rule only applies to products belonging to this category or its children categories. Keep empty otherwise."),
+        'product_tmpl_id': fields.many2one('product.template', 'Product Template', ondelete='cascade', help="Specify a template if this rule only applies to one product template. Keep empty otherwise.", select=1),
+        'product_id': fields.many2one('product.product', 'Product', ondelete='cascade', help="Specify a product if this rule only applies to one product. Keep empty otherwise.", select=1),
+        'categ_id': fields.many2one('product.category', 'Product Category', ondelete='cascade', help="Specify a product category if this rule only applies to products belonging to this category or its children categories. Keep empty otherwise.", select=1),
 
-        'min_quantity': fields.integer('Min. Quantity', required=True, help="Specify the minimum quantity that needs to be bought/sold for the rule to apply."),
+        'min_quantity': fields.integer('Min. Quantity', required=True, help="Specify the minimum quantity that needs to be bought/sold for the rule to apply.", select=1),
         'sequence': fields.integer('Sequence', required=True, help="Gives the order in which the pricelist items will be checked. The evaluation gives highest priority to lowest sequence and stops as soon as a matching item is found."),
-        'base': fields.selection(_price_field_get, 'Based on', required=True, size=-1, help="Base price for computation."),
+        'base': fields.selection(_price_field_get, 'Based on', required=True, size=-1, help="Base price for computation.", select=1),
         'base_pricelist_id': fields.many2one('product.pricelist', 'Other Pricelist'),
 
         'price_surcharge': fields.float('Price Surcharge',
@@ -422,11 +568,11 @@
             help="Sets the price so that it is a multiple of this value.\n" \
               "Rounding is applied after the discount and before the surcharge.\n" \
               "To have prices that end in 9.99, set rounding 10, surcharge -0.01" \
-            ),
+            ,select=1),
         'price_min_margin': fields.float('Min. Price Margin',
             digits_compute= dp.get_precision('Product Price'), help='Specify the minimum amount of margin over the base price.'),
         'price_max_margin': fields.float('Max. Price Margin',
-            digits_compute= dp.get_precision('Product Price'), help='Specify the maximum amount of margin over the base price.'),
+            digits_compute= dp.get_precision('Product Price'), help='Specify the maximum amount of margin over the base price.', select=1),
         'company_id': fields.related('price_version_id','company_id',type='many2one',
             readonly=True, relation='res.company', string='Company', store=True)
     }

=== modified file 'product/product.py'
--- product/product.py	2013-01-22 05:00:57 +0000
+++ product/product.py	2013-03-01 15:13:44 +0000
@@ -21,6 +21,7 @@
 
 import math
 import re
+import time
 
 from _common import rounding
 
@@ -413,22 +414,24 @@
         return res
 
     def _product_price(self, cr, uid, ids, name, arg, context=None):
+
         res = {}
         if context is None:
             context = {}
+        if isinstance(ids, int):
+            ids = [ids]
         quantity = context.get('quantity') or 1.0
         pricelist = context.get('pricelist', False)
         partner = context.get('partner', False)
+        pricelist_obj = self.pool.get('product.pricelist')
         if pricelist:
-            for id in ids:
-                try:
-                    price = self.pool.get('product.pricelist').price_get(cr,uid,[pricelist], id, quantity, partner=partner, context=context)[pricelist]
-                except:
-                    price = 0.0
-                res[id] = price
+            res = pricelist_obj.price_get_batch(cr, uid, [pricelist], ids, quantity, partner=partner, context=context)
         for id in ids:
-            res.setdefault(id, 0.0)
-        return res
+            res.setdefault(id, {pricelist: 0.0})
+        to_return = {}
+        for product, prices in res.iteritems():
+            to_return[product] = prices[pricelist]
+        return to_return
 
     def _get_product_available_func(states, what):
         def _product_available(self, cr, uid, ids, name, arg, context=None):
@@ -681,10 +684,15 @@
         result = self.name_get(cr, user, ids, context=context)
         return result
 
-    #
-    # Could be overrided for variants matrices prices
-    #
-    def price_get(self, cr, uid, ids, ptype='list_price', context=None):
+    def price_get(self, cr, uid, ids, ptype='list_price', context=None, products=None):
+        # /!\ Attention: products contains dictionaries that result from calls to search, not browse records
+        
+        if type(ids) is int:
+            ids = [ids]
+        
+        if products is None:
+            products = self.read(cr, uid, ids, ['id', ptype, 'price_margin', 'price_extra', 'uom_id', 'uos_id'], context=context)
+    
         if context is None:
             context = {}
 
@@ -695,22 +703,25 @@
 
         res = {}
         product_uom_obj = self.pool.get('product.uom')
-        for product in self.browse(cr, uid, ids, context=context):
-            res[product.id] = product[ptype] or 0.0
+        for product in products:
+            res[product['id']] = product[ptype] or 0.0
             if ptype == 'list_price':
-                res[product.id] = (res[product.id] * (product.price_margin or 1.0)) + \
-                        product.price_extra
+                res[product['id']] = (res[product['id']] * (product['price_margin'] or 1.0)) + \
+                        product['price_extra']
             if 'uom' in context:
-                uom = product.uom_id or product.uos_id
-                res[product.id] = product_uom_obj._compute_price(cr, uid,
-                        uom.id, res[product.id], context['uom'])
+                uom = product['uom_id'] or product['uos_id']
+                if isinstance(uom, tuple):
+                    uom_id = uom and uom[0]
+                else:
+                    uom_id = uom.id
+                res[product['id']] = product_uom_obj._compute_price(cr, uid,
+                        uom_id, res[product['id']], context['uom'])
             # Convert from price_type currency to asked one
             if 'currency_id' in context:
                 # Take the price_type currency from the product field
                 # This is right cause a field cannot be in more than one currency
-                res[product.id] = self.pool.get('res.currency').compute(cr, uid, price_type_currency_id,
-                    context['currency_id'], res[product.id],context=context)
-
+                res[product['id']] = self.pool.get('res.currency').compute(cr, uid, price_type_currency_id,
+                    context['currency_id'], res[product['id']],context=context)
         return res
 
     def copy(self, cr, uid, id, default=None, context=None):

=== modified file 'product/product_pricelist_demo.yml'
--- product/product_pricelist_demo.yml	2012-11-29 22:26:45 +0000
+++ product/product_pricelist_demo.yml	2013-03-01 15:13:44 +0000
@@ -52,11 +52,11 @@
               base: -2
 -
   !record {model: pricelist.partnerinfo, id: supplier_pricelist0_product_pc2}:
-    suppinfo_id: product_supplierinfo_2
+    suppinfo_id: product_supplierinfo_1
     min_quantity: 3
     price: 785
 -
   !record {model: pricelist.partnerinfo, id: supplier_pricelist1_product_pc2}:
-    suppinfo_id: product_supplierinfo_2
+    suppinfo_id: product_supplierinfo_1
     min_quantity: 1
     price: 790