Uploaded image for project: 'OFBiz'
  1. OFBiz
  2. OFBIZ-3666

Errors attempting to use quantities with more than 2 decimals of precision

    Details

      Description

      We have a need to handle inventory items with quantities up to four decimal places. The data model currently supports 6 decimals so I thought I would try to complete a purchase order, sales order, and a return for an item that has this many digits of precision. What I found was ...

      • The order item was calculated properly (including the total for the line) but the totals for the entire sales order were incorrect. This was because when OrderReadHelper was getting the OrderItemQuantity as part of calculating the subtotal it would round it to the order's configured scale (2). Since this is being used as a calculation it should not be rounded – the total itself should be rounded.
      • In a number of spots in issuance and reservation services values were (sometimes) being rounded. For example, when an order item was created it would create an InventoryItemDetail record that would reduce the ATP by the precise quantity, however when the order was placed the QOH was reduced by the rounded value. All of these issues were the result of the mini-lang calculate element being used which by default uses a scale of 2 decimals (from the dtd). The solution was when working with quantities pass in the precision scale (6) to ensure we do not lose precision. Again, when order totals, taxes, things like that are being done the configured scale should take over, but the intermediate calculations should not be losing precision.

        Activity

        Hide
        adrianc@hlmksw.com Adrian Crum added a comment -

        Why specify 6 decimals? What if another user needed a different precision?

        These issues are another example of why we need better data typing.

        You don't need a calculate element to perform negation:

        <set field="createDetailMap.availableToPromiseDiff" value="${-parameters.quantityNotIssued?bigDecimal}"/>

        Show
        adrianc@hlmksw.com Adrian Crum added a comment - Why specify 6 decimals? What if another user needed a different precision? These issues are another example of why we need better data typing. You don't need a calculate element to perform negation: <set field="createDetailMap.availableToPromiseDiff" value="${-parameters.quantityNotIssued?bigDecimal}"/>
        Hide
        bmorley Bob Morley added a comment -

        I agree specifying any precision seemed incorrect; but the reality is these services were specifying a precision of 2 and I simply wanted to correct this with the maximum precision of the data type so that rounding would not occur. So in this case I am just making it work without a "higher risk" change that may break the implementation.

        As for the set field; we are a little out of step but has the type conversion changed? I would have thought that value would be picked up by the FlexibleStringExpander so you would get a properly typed result which would then be converted to a String and assigned to the field. We ran into problems with this when we corrected localization which would cause the value to be formatted with a comma which would later cause problems parsing back to the numeric field. Keep in mind we are working with a trunk that we last merged about six months ago, so I understand some of this may have changed.

        Show
        bmorley Bob Morley added a comment - I agree specifying any precision seemed incorrect; but the reality is these services were specifying a precision of 2 and I simply wanted to correct this with the maximum precision of the data type so that rounding would not occur. So in this case I am just making it work without a "higher risk" change that may break the implementation. As for the set field; we are a little out of step but has the type conversion changed? I would have thought that value would be picked up by the FlexibleStringExpander so you would get a properly typed result which would then be converted to a String and assigned to the field. We ran into problems with this when we corrected localization which would cause the value to be formatted with a comma which would later cause problems parsing back to the numeric field. Keep in mind we are working with a trunk that we last merged about six months ago, so I understand some of this may have changed.
        Hide
        adrianc@hlmksw.com Adrian Crum added a comment -

        No, the type conversion hasn't changed. The issue is there is no type to convert to. Quantity should be a type, and Money should be a type. Strong data typing would prevent applying quantity precision to a money, and money precision to a quantity.

        Show
        adrianc@hlmksw.com Adrian Crum added a comment - No, the type conversion hasn't changed. The issue is there is no type to convert to. Quantity should be a type, and Money should be a type. Strong data typing would prevent applying quantity precision to a money, and money precision to a quantity.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        I completly agree about stronger data types. But I have often found that increasing the currency decimals number to its maximum (6) is a workaround when rouding issues occur. I do it directly in DB fieldtypes and add or change (depending on the fastness the client wants). Certainly not the best way to do it, but surely the fastest.

        Show
        jacques.le.roux Jacques Le Roux added a comment - I completly agree about stronger data types. But I have often found that increasing the currency decimals number to its maximum (6) is a workaround when rouding issues occur. I do it directly in DB fieldtypes and add or change (depending on the fastness the client wants). Certainly not the best way to do it, but surely the fastest.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Thanks Bob,

        Your patch is finally in
        trunk r1650938
        R14.12 r1650939
        R13.07 r1650940
        R12.04 r1650941

        Adrian suggested to rather introduce types for quantity and money. This was almost 5 years ago. Because these types has not been introduced since I decided to pragmatically fix this 5 years old bug!

        Show
        jacques.le.roux Jacques Le Roux added a comment - Thanks Bob, Your patch is finally in trunk r1650938 R14.12 r1650939 R13.07 r1650940 R12.04 r1650941 Adrian suggested to rather introduce types for quantity and money. This was almost 5 years ago. Because these types has not been introduced since I decided to pragmatically fix this 5 years old bug!

          People

          • Assignee:
            jacques.le.roux Jacques Le Roux
            Reporter:
            bmorley Bob Morley
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development