Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: JDO 2 maintenance release 2 (2.2)
    • Fix Version/s: JDO 3.1-rc1
    • Component/s: specification, tck
    • Labels:
      None

      Description

      Aggregate type specification at page 175 claims that avg(expression) return type is the same of the enclosed expression.
      This is wrong from a strictly mathematical point of view.
      In fact, if you select an avg on an int field member the result is truncated.
      The result type should always be double.

      1. JDO-643.patch
        1 kB
        Andy Jefferson

        Activity

        Hide
        Andy Jefferson added a comment -

        +1 on this. Perhaps we could at least get agreement that this is how it ought to be and I can change the RI and any test accordingly.

        Show
        Andy Jefferson added a comment - +1 on this. Perhaps we could at least get agreement that this is how it ought to be and I can change the RI and any test accordingly.
        Hide
        Craig L Russell added a comment -

        I'd like to see this changed as well. Can someone write up what the return type should be for all of the numeric types that JDO supports?

        I don't think it's obvious that double is the right type for average of BigInteger or BigDecimal. And what about float?

        Show
        Craig L Russell added a comment - I'd like to see this changed as well. Can someone write up what the return type should be for all of the numeric types that JDO supports? I don't think it's obvious that double is the right type for average of BigInteger or BigDecimal. And what about float?
        Hide
        Andy Jefferson added a comment -

        My vote would go to

        short, int, long, double, float should return double
        BigInteger, BigDecimal should return BigDecimal

        Show
        Andy Jefferson added a comment - My vote would go to short, int, long, double, float should return double BigInteger, BigDecimal should return BigDecimal
        Hide
        Michael Bouschen added a comment -

        I agree, avg(expression) should return

        • Double for integral types (byte, short, int, long), float, double and their wrapper classes
        • BigDecimal for BigInteger and BigDecimal

        Two remarks:

        • We need to point out that this is an incompatible change, meaning a valid JDO 3.0 query might run into a RuntimeException.
        • JPA always uses Double as return type
        Show
        Michael Bouschen added a comment - I agree, avg(expression) should return Double for integral types (byte, short, int, long), float, double and their wrapper classes BigDecimal for BigInteger and BigDecimal Two remarks: We need to point out that this is an incompatible change, meaning a valid JDO 3.0 query might run into a RuntimeException. JPA always uses Double as return type
        Hide
        Andy Jefferson added a comment -

        Attached is a patch for the two tests affected by this proposed change.

        Show
        Andy Jefferson added a comment - Attached is a patch for the two tests affected by this proposed change.
        Hide
        Craig L Russell added a comment -

        What about the case where the JDO implementation lets the database perform the avg function? If the database always returns a double value anyway, are we kidding ourselves that the result is actually a BigDecimal? What should the precision be in this case?

        Obviously, either the user or the implementation could take a double result and convert it into a BigDecimal. Is there a good reason why BigDecimal is preferable to return to the user instead of double? Would a user expect the result to be BigDecimal (without reading the JDO specification carefully)?

        Is there a good enough reason for JDO to be different from JPA in this case?

        Show
        Craig L Russell added a comment - What about the case where the JDO implementation lets the database perform the avg function? If the database always returns a double value anyway, are we kidding ourselves that the result is actually a BigDecimal? What should the precision be in this case? Obviously, either the user or the implementation could take a double result and convert it into a BigDecimal. Is there a good reason why BigDecimal is preferable to return to the user instead of double? Would a user expect the result to be BigDecimal (without reading the JDO specification carefully)? Is there a good enough reason for JDO to be different from JPA in this case?
        Hide
        Andy Jefferson added a comment -

        If the datastore indeed does evaluate it as double (equivalent) then your comment makes sense.

        JPA doesn't allow specification of candidate instances and hence allow in-memory evaluation whereas JDO does. That is one case where avg(BigInteger) returning BigDecimal makes sense. If it's the only case then it likely shouldn't effect a change to the general rule.

        Show
        Andy Jefferson added a comment - If the datastore indeed does evaluate it as double (equivalent) then your comment makes sense. JPA doesn't allow specification of candidate instances and hence allow in-memory evaluation whereas JDO does. That is one case where avg(BigInteger) returning BigDecimal makes sense. If it's the only case then it likely shouldn't effect a change to the general rule.
        Hide
        Michael Bouschen added a comment -

        I agree there is no good reason to differ from JPA here, which means avg(expressin) would alwas return Double.

        I think the patch JDO-643.patch is good to be checked in.

        Show
        Michael Bouschen added a comment - I agree there is no good reason to differ from JPA here, which means avg(expressin) would alwas return Double. I think the patch JDO-643 .patch is good to be checked in.
        Hide
        Craig L Russell added a comment -

        I agree. With the patch, this should now be just a specification (and RI) issue.

        Show
        Craig L Russell added a comment - I agree. With the patch, this should now be just a specification (and RI) issue.
        Hide
        Andy Jefferson added a comment -

        Patch applied. DN 2.2.2 (not yet released, but downloadable as a nightly build), and 3.x abide by this JDO spec change

        Show
        Andy Jefferson added a comment - Patch applied. DN 2.2.2 (not yet released, but downloadable as a nightly build), and 3.x abide by this JDO spec change
        Hide
        Michael Bouschen added a comment -

        Here is the proposed spec change on page 177 of the 3.0 version of the spec. Replace the second and third paragraph under the headline "Aggregate Types" by:

        Sum returns Long for integral types and the field’s type for other Number types (BigDecimal, Big- Integer, Float, and Double). Avg returns Double. Sum and avg are invalid if applied to non-Number types.

        Min, and max return the type of the expression.

        Show
        Michael Bouschen added a comment - Here is the proposed spec change on page 177 of the 3.0 version of the spec. Replace the second and third paragraph under the headline "Aggregate Types" by: Sum returns Long for integral types and the field’s type for other Number types (BigDecimal, Big- Integer, Float, and Double). Avg returns Double. Sum and avg are invalid if applied to non-Number types. Min, and max return the type of the expression.
        Hide
        Michael Bouschen added a comment -

        Changed the spec. Committed revision 1379842.

        Show
        Michael Bouschen added a comment - Changed the spec. Committed revision 1379842.

          People

          • Assignee:
            Michael Bouschen
            Reporter:
            Guido Anzuoni
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development