WIP: Bad Inventory asset price with FIFO method after a zero-priced movement
Context
We recently encountered a problem on Nexedi ERP5 with stocks: when one product was added to the stock for a price of 0 (zero), then the entire stock value would become zero on Inventory Reports. For reference, and since this issue cannot be reproduced with any valuation method, please note that we use the FIFO (First-In-First-Out) method for stock valuation.
After investigating, I digged a bit and was able to make a test reproducing this behavior. The test is almost self explanatory: it creates two simulation movements: one adding one item in a stock for a non-zero price, and another adding one item in the same stock for a zero price. No item has left the stock at any point, so the total stock value is expected to be equal to the price of the first item which entered the stock (since the second one is “free”).
Explanation and questions
However, the getInventoryAssetPrice
method will return a zero stock value. That is due to a weird record in the stock
table: the second movement is recorded with a NULL total_price
. Since operations on NULLs in SQL propagate the NULL value (eg. 1 + NULL = NULL
), the running_total_asset_price
counter on ZSQL method Resource_zGetAssetPrice
is NULL.
I am unsure of how to solve this issue. The second commit of this MR gives a quick fix that works, but which does not fix the issue at its root: the movement should likely not be saved in the SQL table with a NULL value in the first place. Please also note that it is likely an incomplete fix: I think at least FILO should be fixed in the same way, but haven’t tested all methods (I know that WeightedAverage, which I believe to be a widely used method, is not affected), since I want to know what to do first. My (open) questions are the following:
- am I right in thinking that NULL in SQL should be reserved to non-defined values, and not serve as a zero?
- if so, where may I find the piece of code that inserts this value in the table? I am still quite confused with how the simulation works at all;
- what fix would you recommend? I am quite sure that even if we can fix new records on the table, we do not want to rebuild it for old movements, so maybe both fixes (non-NULL values for new records and
IFNULL
) are needed. The latter can stay on Nexedi ERP5 repository if we are the only ones having this issue.