Openbravo Issue Tracking System - Openbravo ERP |
| View Issue Details |
|
| ID | Project | Category | View Status | Date Submitted | Last Update |
| 0045909 | Openbravo ERP | 09. Financial management | public | 2021-02-16 17:23 | 2021-02-24 11:13 |
|
| Reporter | malsasua | |
| Assigned To | vmromanos | |
| Priority | normal | Severity | major | Reproducibility | always |
| Status | closed | Resolution | fixed | |
| Platform | | OS | 5 | OS Version | |
| Product Version | | |
| Target Version | | Fixed in Version | PR21Q2 | |
| Merge Request Status | |
| Review Assigned To | |
| OBNetwork customer | |
| Web browser | |
| Modules | Core |
| Support ticket | |
| Regression level | |
| Regression date | |
| Regression introduced in release | |
| Regression introduced by commit | |
| Triggers an Emergency Pack | No |
|
| Summary | 0045909: Performance problem with conversion rate document functionality |
| Description | With massive data, the trigger c_conversion_rate_doc_trg2 is taking several seconds for each record.
Analyzing it, it is not efficient, as for each record, it is checking the whole table, not only the record inserted or updated |
| Steps To Reproduce | n/a |
| Proposed Solution | attached a possible fix |
| Additional Information | |
| Tags | No tags attached. |
| Relationships | |
| Attached Files |  patch.diff (3,366) 2021-02-16 17:23
https://issues.openbravo.com/file_download.php?file_id=15327&type=bug |
|
| Issue History |
| Date Modified | Username | Field | Change |
| 2021-02-16 17:23 | malsasua | New Issue | |
| 2021-02-16 17:23 | malsasua | Assigned To | => Triage Finance |
| 2021-02-16 17:23 | malsasua | File Added: patch.diff | |
| 2021-02-16 17:23 | malsasua | Modules | => Core |
| 2021-02-16 17:23 | malsasua | Resolution time | => 1614812400 |
| 2021-02-16 17:23 | malsasua | Triggers an Emergency Pack | => No |
| 2021-02-16 18:23 | vmromanos | Note Added: 0126128 | |
| 2021-02-16 18:23 | vmromanos | Status | new => acknowledged |
| 2021-02-17 17:00 | vmromanos | Status | acknowledged => scheduled |
| 2021-02-17 17:00 | vmromanos | Assigned To | Triage Finance => vmromanos |
| 2021-02-17 17:08 | vmromanos | Note Added: 0126144 | |
| 2021-02-18 09:54 | hgbot | Note Added: 0126149 | |
| 2021-02-22 13:52 | hgbot | Note Added: 0126265 | |
| 2021-02-22 13:52 | hgbot | Resolution | open => fixed |
| 2021-02-22 13:52 | hgbot | Status | scheduled => closed |
| 2021-02-22 13:52 | hgbot | Note Added: 0126266 | |
| 2021-02-22 13:52 | hgbot | Fixed in Version | => PR21Q2 |
| 2021-02-22 13:52 | hgbot | Note Added: 0126267 | |
| 2021-02-22 13:52 | hgbot | Note Added: 0126268 | |
| 2021-02-22 13:52 | hgbot | Note Added: 0126269 | |
| 2021-02-23 09:29 | hgbot | Note Added: 0126291 | |
| 2021-02-23 09:29 | hgbot | Note Added: 0126292 | |
| 2021-02-23 09:29 | hgbot | Note Added: 0126293 | |
| 2021-02-23 09:29 | hgbot | Note Added: 0126294 | |
| 2021-02-23 09:32 | vmromanos | Note Added: 0126295 | |
| 2021-02-23 09:32 | vmromanos | Status | closed => new |
| 2021-02-23 09:32 | vmromanos | Resolution | fixed => open |
| 2021-02-23 09:32 | vmromanos | Fixed in Version | PR21Q2 => |
| 2021-02-23 11:45 | hgbot | Note Added: 0126297 | |
| 2021-02-23 11:48 | vmromanos | Status | new => scheduled |
| 2021-02-24 11:13 | hgbot | Resolution | open => fixed |
| 2021-02-24 11:13 | hgbot | Status | scheduled => closed |
| 2021-02-24 11:13 | hgbot | Note Added: 0126318 | |
| 2021-02-24 11:13 | hgbot | Fixed in Version | => PR21Q2 |
| 2021-02-24 11:13 | hgbot | Note Added: 0126319 | |
| 2021-02-24 11:13 | hgbot | Note Added: 0126320 | |
| 2021-02-24 11:13 | hgbot | Note Added: 0126321 | |
| 2021-02-24 11:13 | hgbot | Note Added: 0126322 | |
| 2021-02-24 11:13 | hgbot | Note Added: 0126323 | |
|
Notes |
|
|
|
WARNING: The proposed patch would create a mutating table issue as the trigger is querying its own table (that's why the trigger is originally defined at statement level, so the mutating table is avoided).
I tend to think an EventEntityObserver is a better option in this case. |
|
|
|
|
Test plan:
Go to Sales Invoice window and create a new header for España - Región Norte.
Go to Exchange Rates tab and create a new record:
- To Currency: USD
- Rate: 1.12
Create a new record:
- To Currency: USD
- Rate: 1.22
Verify an error is thrown and the second record can't be saved.
This exercise should be repeated in GL Journal, Payment and Financial Transaction, in both Oracle and PG.
Automatic tests should be developed to simplify |
|
|
|
(0126149)
|
|
hgbot
|
|
2021-02-18 09:54
|
|
|
|
|
(0126265)
|
|
hgbot
|
|
2021-02-22 13:52
|
|
|
|
|
(0126266)
|
|
hgbot
|
|
2021-02-22 13:52
|
|
|
|
|
(0126267)
|
|
hgbot
|
|
2021-02-22 13:52
|
|
Directly closing issue as related merge request is already approved.
Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: 9c2278bb6b1f1c60f5cc0952d0ca21d47ba903f2
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-22T10:47:50+00:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/9c2278bb6b1f1c60f5cc0952d0ca21d47ba903f2 [^]
Fixed BUG-45909: Remove trigger and improve unique constraint
The trigger has been replaced by an unique index that includes
the currencies and a coalesce between all the possibles document ids
that can be involved in a conversion rate document. Note that at least
one of them must be not-null.
This new unique index is the improved version of the existing unique constraint
(C_CONVERSIONRATEDOC_ONCE) that has been removed as it wasn't working properly.
With the new unique index there is no need to have the C_CONVERSION_RATE_DOC_TRG2
trigger as it was implementing the same functionality that the new unique index is
doing.
Changed existing error message key to match the unique constraint name.
Transform some indexes to partial index after trigger removal, as there is no real
advantage of having those indexes as full indexes anymore.
---
M src-db/database/model/tables/C_CONVERSION_RATE_DOCUMENT.xml
M src-db/database/sourcedata/AD_MESSAGE.xml
R src-db/database/model/triggers/C_CONVERSION_RATE_DOC_TRG2.xml
---
|
|
|
|
(0126268)
|
|
hgbot
|
|
2021-02-22 13:52
|
|
Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: c36d47496705e62a52e53051159b79231554fb0b
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-22T10:47:50+00:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/c36d47496705e62a52e53051159b79231554fb0b [^]
Related to BUG-45909: Added JUnit tests
---
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueFinTransactionTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueGLJournalTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueInvoiceTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniquePaymentTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTestSuite.java
M src-test/src/org/openbravo/test/AllAntTaskTests.java
---
|
|
|
|
(0126269)
|
|
hgbot
|
|
2021-02-22 13:52
|
|
Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: fe647fb22b726595abd4549e0cbf54eb42f9b54a
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-22T10:47:50+00:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/fe647fb22b726595abd4549e0cbf54eb42f9b54a [^]
Related to BUG-45909: JUnit tests stabilization
In Oracle the tests were working fine because the transaction was kept alive,
so the code was able to properly remove the changes in the database in @After.
However, PG has a different transaction management and the transaction was aborted
in the moment the exception was uncaught.
To fix this we make sure the last executed test is the one that must fail, and in
the @After we just rollback the transaction
---
M src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTest.java
---
|
|
|
|
(0126291)
|
|
hgbot
|
|
2021-02-23 09:29
|
|
|
|
|
(0126292)
|
|
hgbot
|
|
2021-02-23 09:29
|
|
Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: 476d70b28656bda72696c345fdabd4ddd6afaeec
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-23T09:27:11+01:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/476d70b28656bda72696c345fdabd4ddd6afaeec [^]
Revert "Related to BUG-45909: Added JUnit tests"
This reverts commit c36d47496705e62a52e53051159b79231554fb0b.
---
M src-test/src/org/openbravo/test/AllAntTaskTests.java
R src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueFinTransactionTest.java
R src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueGLJournalTest.java
R src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueInvoiceTest.java
R src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniquePaymentTest.java
R src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTest.java
R src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTestSuite.java
---
|
|
|
|
(0126293)
|
|
hgbot
|
|
2021-02-23 09:29
|
|
|
|
|
(0126294)
|
|
hgbot
|
|
2021-02-23 09:29
|
|
|
|
|
|
Reopened as the original solution breaks retail integration. Scenario where it's reproducible:
Create a ticket and pay in EUR (OK)
Create a similar ticket and pay in USD (paying more amount), and the change in EUR. This fails and it shouldn't. |
|
|
|
(0126297)
|
|
hgbot
|
|
2021-02-23 11:45
|
|
|
|
|
(0126318)
|
|
hgbot
|
|
2021-02-24 11:13
|
|
|
|
|
(0126319)
|
|
hgbot
|
|
2021-02-24 11:13
|
|
Directly closing issue as related merge request is already approved.
Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: 748fb89b581b5cb36834d5847ba9cad5712396cd
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-24T10:12:17+00:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/748fb89b581b5cb36834d5847ba9cad5712396cd [^]
Fixed BUG-45909: Remove trigger and improve unique constraint
The trigger has been replaced by an unique index that includes
the currencies and a coalesce between all the possibles document ids
that can be involved in a conversion rate document. Note that at least
one of them must be not-null.
This new unique index is the improved version of the existing unique constraint
(C_CONVERSIONRATEDOC_ONCE) that has been removed as it wasn't working properly.
With the new unique index there is no need to have the C_CONVERSION_RATE_DOC_TRG2
trigger as it was implementing the same functionality that the new unique index is
doing.
Changed existing error message key to match the unique constraint name.
Transform some indexes to partial index after trigger removal, as there is no real
advantage of having those indexes as full indexes anymore.
---
M src-db/database/model/tables/C_CONVERSION_RATE_DOCUMENT.xml
M src-db/database/sourcedata/AD_MESSAGE.xml
R src-db/database/model/triggers/C_CONVERSION_RATE_DOC_TRG2.xml
---
|
|
|
|
(0126320)
|
|
hgbot
|
|
2021-02-24 11:13
|
|
Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: 7cc16094ae663de3fbf28043b9ab506abf973ea7
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-24T10:12:17+00:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/7cc16094ae663de3fbf28043b9ab506abf973ea7 [^]
Related to BUG-45909: Added JUnit tests
---
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueFinTransactionTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueGLJournalTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueInvoiceTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniquePaymentTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTest.java
A src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTestSuite.java
M src-test/src/org/openbravo/test/AllAntTaskTests.java
---
|
|
|
|
(0126321)
|
|
hgbot
|
|
2021-02-24 11:13
|
|
Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: 6e0bd8ecb0430c066ccf1f2e6478d0f0c0ba9dff
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-24T10:12:17+00:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/6e0bd8ecb0430c066ccf1f2e6478d0f0c0ba9dff [^]
Related to BUG-45909: JUnit tests stabilization
In Oracle the tests were working fine because the transaction was kept alive,
so the code was able to properly remove the changes in the database in @After.
However, PG has a different transaction management and the transaction was aborted
in the moment the exception was uncaught.
To fix this we make sure the last executed test is the one that must fail, and in
the @After we just rollback the transaction
---
M src-test/src/org/openbravo/test/conversionratedoc/ConversionRateDocUniqueTest.java
---
|
|
|
|
(0126322)
|
|
hgbot
|
|
2021-02-24 11:13
|
|
Repository: https://gitlab.com/openbravo/product/openbravo [^]
Changeset: 088708b9c52257ecce0cbe64df7fd6258556106f
Author: Víctor Martínez Romanos <victor.martinez@openbravo.com>
Date: 2021-02-24T10:12:17+00:00
URL: https://gitlab.com/openbravo/product/openbravo/-/commit/088708b9c52257ecce0cbe64df7fd6258556106f [^]
Related to ISSUE-45909: unique index is based on multiple coalesces
The unique index based on the coalesce was not working fine when the same UUID
was shared among different columns in different tuples. This was reproducible
in the case of FIN_PAYMENT_ID and APRM_FINACC_TRANSACTION_V_ID, where both share
the same UUID regardless there were two different tuples. Example:
CURRENCY, CURRENCYTO, FIN_PAYMENT_ID, APRM_FINACC_TRANSACTION_V_ID
100, 102, AAAAA, null
100, 102, null, AAAAA
The fix consists on adding a separate coalesce for each nullable column, that will
return the same dummy value if the column is null. This way we control the scenario
where the same UUID is found in different columns.
---
M src-db/database/model/tables/C_CONVERSION_RATE_DOCUMENT.xml
---
|
|
|
|
(0126323)
|
|
hgbot
|
|
2021-02-24 11:13
|
|
|