nexedi / erp5

Looking more closely, I think one sort is missing.

The final structure, with a lot of <ecriture> and few <journal> is:

<comptabilite><exercice> <!-- AccountingTransactionModule_viewComptabiliteAsFECXML -->
  <journal> <!-- AccountingTransactionModule_viewJournalAsFECXML -->
    <ecriture>  <!-- AccountingTransaction_viewAsSourceFECXML ( or AccountingTransaction_viewAsDestinationFECXML ) -->

<ecriture> are created with this searchAndActivate and aggregated in <journal> by AccountingTransactionModule_aggregateFrenchAccountingTransactionFileForOneJournal, then the <journal> are aggregated by AccountingTransactionModule_aggregateFrenchAccountingTransactionFile.

Apparently the <journal> are not sorted. Isn't another sort missing ?

[edit to fix formatting]

isn't contentValues stable BTW ?

( I mean stable ordering )

As far as I remember it wasn't because I had to add this sorted() call.

I did not mention it clearly in 1a02ac9c (comment 66985) but the concern here is that this list will be really big. Currently we already have a limitation that this list has to fit in memory, otherwise it will fail later when building the page template.

When the page template do:

    <tal:block tal:repeat="result options/result_list"><tal:block tal:replace="structure python: result"/></tal:block>

if result_list is getResultList(), then it's an iterator, so we will have in memory at the same time the result page template content and the current result from the loop, but if result_list is a list, then we need more memory (the full list + the result page template). So if it can be an iterator ... it's better (but I don't really see how we can achieve this).

---

Because we only need stable sort, maybe we can just sort the zlib encoded blobs? ie. instead of doing:

result_list = [ result.detail.decode('zlib') for result in active_process.getResultList() ]
result_list.sort()

    <tal:block tal:repeat="result options/result_list"><tal:block tal:replace="structure python: result"/></tal:block>

do:

result_list = [ result.detail for result in active_process.getResultList() ]
result_list.sort()

    <tal:block tal:repeat="result options/result_list"><tal:block tal:replace="structure python: result.detail.decode('zlib')"/></tal:block>

... this way we still have a full list in memory, but the list item are still compressed, so it should be a bit smaller.

BTW, I was not saying this is a problem with this merge request, it's same problem that sometimes cause MemoryError in deferred style reports.

@jerome

Looking more closely, I think one sort is missing. Apparently the <journal> are not sorted. Isn't another sort missing ?

In AccountingTransactionModule_viewComptabiliteAsFECXML, <exercice> is a single tag (eg there is only one per file) and result_list are <journal> tags sorted in AccountingTransactionModule_aggregateFrenchAccountingTransactionFile. So in my understanding there is no missing sort.

Apparently the <journal> are not sorted. Isn't another sort missing ?

In AccountingTransactionModule_viewComptabiliteAsFECXML, <exercice> is a single tag (eg there is only one per file) and result_list are <journal> tags sorted in AccountingTransactionModule_aggregateFrenchAccountingTransactionFile. So in my understanding there is no missing sort.

I was wrong in my explanation, it's not <journal> that are not sorted, but the <ecriture> inside the <journal>.

There are two levels of searchAndActivate then merge results in page template:

1. AccountingTransactionModule_aggregateFrenchAccountingTransactionFileForOneJournal aggregate several <ecriture> in a <journal>
2. AccountingTransactionModule_aggregateFrenchAccountingTransactionFile aggregate several <journal> in a <exercice>.

In the first case it seems we don't sort.

Also, for future reference, can we explain a bit why this was needed ? For the normal usage of this export (ie. during a tax audit from administration), there's no need for this export to be stable.

@jerome

In the first case it seems we don't sort.

Indeed. The reason it was not done is because for our customer project we don't have multiple <ecriture> per <journal>.

Also, for future reference, can we explain a bit why this was needed ? For the normal usage of this export (ie. during a tax audit from administration), there's no need for this export to be stable.

AFAIR This is needed for Unit Tests and manual (user and developer) checks. Maybe @cedric.leninivin can point out another reason though as I have not been involved in this project lately.