Commit 8a9b7357 authored by Jérome Perrin's avatar Jérome Perrin

Inventory API: drop default group by in getMovementHistoryList

Over the years, we included almost all possible default group by criterions for getMovementHistoryList, so that in essence, we do not group ... for example, we did 7814c521 . It would be better for performance and more logical to just not group by at all, getMovementHistoryList should just return the list of movements.

This is still possible to use explicit group by parameters in getMovementHistoryList, for example grouping by [explanation_uid](https://lab.nexedi.com/nexedi/erp5/blob/c413d34b9d5db0beda2b9540d563529082855d91/product/ERP5/tests/testInventoryAPI.py#L2416) like we can do in [some accounting reports](https://lab.nexedi.com/nexedi/erp5/blob/c413d34b9d5db0beda2b9540d563529082855d91/bt5/erp5_accounting/SkinTemplateItem/portal_skins/erp5_accounting/AccountModule_getGeneralLedgerReportSectionList.py#L111).

I don't have a proper benchmark, but on the instance where we discover the issue, a simple getMovementHistoryList yieliding 112301 results went from several minutes spent in state *Removing duplicates*  (I killed the query after some time) to about 3 seconds.

cc @vpelletier @jm @gabriel 

/reviewed-on nexedi/erp5!171
parent 79d284ab
......@@ -1159,7 +1159,6 @@ class SimulationTool(BaseTool):
group_by_section_category=0,
group_by_section_category_strict_membership=0,
group_by_resource=None,
movement_list_mode=0,
group_by=None,
**ignored):
"""
......@@ -1170,12 +1169,7 @@ class SimulationTool(BaseTool):
If any group-by is provided, automatically group by resource aswell
unless group_by_resource is explicitely set to false.
If no group by is provided, use the default group by: movement, node and
resource, unless movement_list_mode is true, in that case, group by
movement, node, resource and date (this is historically the default in
getMovementHistoryList), section, mirror_section and payment (this is to
make sure two lines will appear when we are, for instance both source and
destination, implementation might not be optimal, because it uses lots of
group by statements in SQL).
resource.
"""
new_group_by_dict = {}
if not ignore_group_by and group_by is None:
......@@ -1192,12 +1186,6 @@ class SimulationTool(BaseTool):
new_group_by_dict['group_by_movement'] = 1
new_group_by_dict['group_by_node'] = 1
new_group_by_dict['group_by_resource'] = 1
if movement_list_mode:
new_group_by_dict['group_by_date'] = 1
new_group_by_dict['group_by_mirror_node'] = 1
new_group_by_dict['group_by_section'] = 1
new_group_by_dict['group_by_mirror_section'] = 1
new_group_by_dict['group_by_payment'] = 1
return new_group_by_dict
security.declareProtected(Permissions.AccessContentsInformation,
......@@ -1976,9 +1964,6 @@ class SimulationTool(BaseTool):
brains. The initial values can be passed, in case you want to have an
"initial summary line".
"""
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)
......
......@@ -59,7 +59,7 @@ FROM
<dtml-if selection_report><dtml-let expression="portal_selections.buildSQLJoinExpressionFromDomainSelection(selection_report, category_table_alias='report_category')"><dtml-if expression>, <dtml-var expression></dtml-if></dtml-let></dtml-if>
WHERE
1 = 1
stock.uid = catalog.uid
<dtml-if where_expression>
AND <dtml-var where_expression>
</dtml-if>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment