From c708760b5de5873dbdfda8c8d0f11561b5f4c1e5 Mon Sep 17 00:00:00 2001
From: Kazuhiko Shiozaki <kazuhiko@nexedi.com>
Date: Wed, 7 Oct 2015 21:51:34 +0200
Subject: [PATCH] Support order_by_list in Resource_zGetMovementHistoryList.

Now we have explicit ORDER BY in the outer query so that we no longer need
to disable derived_merge optimizer.
c.f. https://bugs.launchpad.net/maria/+bug/985828
---
 product/ERP5/Tool/SimulationTool.py           | 23 +++++++++++++------
 .../Resource_zGetMovementHistoryList.xml      |  8 +++++++
 product/ERP5/tests/testAccountingReports.py   | 13 +++++------
 3 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/product/ERP5/Tool/SimulationTool.py b/product/ERP5/Tool/SimulationTool.py
index 1ab5ac35ef..ea88cd62f2 100644
--- a/product/ERP5/Tool/SimulationTool.py
+++ b/product/ERP5/Tool/SimulationTool.py
@@ -481,7 +481,7 @@ class SimulationTool(BaseTool):
         new_kw = new_kw.copy()
 
         # Group-by expression  (eg. group_by=['node_uid'])
-        group_by = new_kw.pop('group_by', [])
+        group_by = new_kw.pop('group_by_list', [])
 
         # group by from stock table (eg. group_by_node=True)
         # prepend table name to avoid ambiguities.
@@ -496,14 +496,14 @@ class SimulationTool(BaseTool):
 
         # group by involving a related key (eg. group_by=['product_line_uid'])
         related_key_dict_passthrough_group_by = new_kw.get(
-                'related_key_dict_passthrough', {}).pop('group_by', [])
+                'related_key_dict_passthrough', {}).pop('group_by_list', [])
         if isinstance(related_key_dict_passthrough_group_by, basestring):
           related_key_dict_passthrough_group_by = (
                 related_key_dict_passthrough_group_by,)
         group_by.extend(related_key_dict_passthrough_group_by)
 
         if group_by:
-          new_kw['group_by'] = group_by
+          new_kw['group_by_list'] = group_by
 
         # select expression
         select_dict = new_kw.setdefault('select_dict', {})
@@ -810,7 +810,7 @@ class SimulationTool(BaseTool):
           else:
             related_group_by.append(value)
         if len(related_group_by):
-          new_kw['related_key_dict_passthrough']['group_by'] = related_group_by
+          new_kw['related_key_dict_passthrough']['group_by_list'] = related_group_by
 
       #variation_category_uid_list = self._generatePropertyUidList(variation_category)
       #if len(variation_category_uid_list) :
@@ -1536,14 +1536,14 @@ class SimulationTool(BaseTool):
           group_by_id_list_append(group_by_id)
       # Add related key group by
       if 'select_list' in new_kw.get("related_key_dict_passthrough", []):
-        for group_by_id in new_kw["related_key_dict_passthrough"]['group_by']:
+        for group_by_id in new_kw["related_key_dict_passthrough"]['group_by_list']:
           if group_by_id in new_kw["related_key_dict_passthrough"]["select_list"]:
             group_by_id_list_append(group_by_id)
           else:
             # XXX-Aurel : to review & change, must prevent coming here before
             raise ValueError, "Impossible to group by %s" %(group_by_id)
       elif "group_by" in new_kw.get("related_key_dict_passthrough", []):
-        raise ValueError, "Impossible to group by %s" %(new_kw["related_key_dict_passthrough"]['group_by'],)
+        raise ValueError, "Impossible to group by %s" %(new_kw["related_key_dict_passthrough"]['group_by_list'],)
 
       if len(group_by_id_list):
         def getInventoryListKey(line):
@@ -1705,7 +1705,7 @@ class SimulationTool(BaseTool):
       # clause. Note that the call to 'mergeZRDBResults' will crash if the GROUP
       # clause contains a column not requested in the SELECT clause.
       kw.update(self._getDefaultGroupByParameters(**kw), ignore_group_by=1)
-      group_by_list = self._generateKeywordDict(**kw)[1].get('group_by', ())
+      group_by_list = self._generateKeywordDict(**kw)[1].get('group_by_list', ())
 
       results = []
       edit_result = {}
@@ -2015,6 +2015,15 @@ class SimulationTool(BaseTool):
       """
       kw['movement_list_mode'] = 1
       kw.update(self._getDefaultGroupByParameters(**kw))
+
+      # Extend select_dict by order_by_list columns.
+      catalog = self.getPortalObject().portal_catalog.getSQLCatalog()
+      kw = catalog.getCannonicalArgumentDict(kw)
+      extra_column_set = {i[0] for i in kw.get('order_by_list', ())}
+      kw.setdefault('select_dict', {}).update(
+        (x.replace('.', '_') + '__ext__', x)
+        for x in extra_column_set if not x.endswith('__score__'))
+
       sql_kw = self._generateSQLKeywordDict(**kw)
 
       return self.Resource_zGetMovementHistoryList(
diff --git a/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Resource_zGetMovementHistoryList.xml b/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Resource_zGetMovementHistoryList.xml
index 81ba23ea16..4869e0d923 100644
--- a/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Resource_zGetMovementHistoryList.xml
+++ b/product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/Resource_zGetMovementHistoryList.xml
@@ -419,6 +419,7 @@
             <key> <string>arguments_src</string> </key>
             <value> <string>from_expression\r\n
 from_table_list:list\r\n
+select_expression\r\n
 where_expression\r\n
 order_by_expression\r\n
 group_by_expression\r\n
@@ -512,6 +513,9 @@ SELECT\n
   stock.payment_request_uid AS payment_request_uid,\n
   stock.node_uid AS node_uid,\n
   stock.section_uid AS section_uid\n
+<dtml-if select_expression>\n
+  ,<dtml-var select_expression>\n
+</dtml-if>\n
 FROM\n
 <dtml-if from_expression>\n
   <dtml-var from_expression>\n
@@ -623,6 +627,10 @@ LIMIT\n
 </dtml-if>\n
 \n
 ) AS q1\n
+<dtml-if order_by_expression>\n
+ORDER BY\n
+  <dtml-var order_by_expression>\n
+</dtml-if>\n
 
 
 ]]></string> </value>
diff --git a/product/ERP5/tests/testAccountingReports.py b/product/ERP5/tests/testAccountingReports.py
index 8851668159..7a8f708274 100644
--- a/product/ERP5/tests/testAccountingReports.py
+++ b/product/ERP5/tests/testAccountingReports.py
@@ -35,7 +35,6 @@ from DateTime import DateTime
 
 from Products.ERP5.tests.testAccounting import AccountingTestCase
 from Products.ERP5Type.tests.ERP5TypeTestCase import ERP5ReportTestCase
-from Products.ERP5Type.tests.utils import todo_erp5
 from Products.ERP5Type.UnrestrictedMethod import UnrestrictedMethod
 
 class TestAccountingReports(AccountingTestCase, ERP5ReportTestCase):
@@ -799,18 +798,18 @@ class TestAccountingReports(AccountingTestCase, ERP5ReportTestCase):
                      dict(source_value=account_module.goods_sales,
                           source_credit=300.0)))
 
-  @todo_erp5
   def test_Resource_zGetMovementHistoryList(self):
-    # TODO: Fix Resource_zGetMovementHistoryList so that we don't need to workaround
-    #       new behaviour of MariaDB.
-    #       Indeed, https://bugs.launchpad.net/maria/+bug/985828 has been marked
-    #       as WONTFIX.
+    # Check if Resource_zGetMovementHistoryList works fine with derived_merge optimizer.
+    # see https://bugs.launchpad.net/maria/+bug/985828
     q = self.portal.erp5_sql_connection.manage_test
+    tmp = q("show variables like 'optimizer_switch'")
+    optimizer_switch_dict = dict([x.split('=') for x in tmp[0][1].split(',')])
     q("SET optimizer_switch = 'derived_merge=on'")
     try:
       self.testAccountStatement()
     finally:
-      q("SET optimizer_switch = 'derived_merge=off'")
+      q("SET optimizer_switch = 'derived_merge=%s'" % \
+          optimizer_switch_dict.get('derived_merge', 'off'))
 
   def testAccountStatement(self):
     # Simple Account Statement for "Receivable" account
-- 
2.30.9