Pig
  1. Pig
  2. PIG-3569

SUM function for BigDecimal and BigInteger

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 0.12.0
    • Fix Version/s: 0.13.0
    • Component/s: internal-udfs
    • Labels:
      None
    • Patch Info:
      Patch Available
    • Hadoop Flags:
      Reviewed
    • Release Note:
      Attached PIG_3569.patch was tested with latest from trunk.

      Description

      SUM function doesn't support newly added BigDecimal data type. I've a patch to make SUM function work for BigDecimal

      1. PIG_3569_v2.patch
        25 kB
        Hariharasudhan Chinnan

        Activity

        Daniel Dai made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Hariharasudhan Chinnan added a comment -

        Thank you Rohini for your feedback and for accepting my patch. I'm all exited on my first ever patch to any open source project getting accepted. You folks are a welcoming community. I look forward to contributing more in the future.

        Show
        Hariharasudhan Chinnan added a comment - Thank you Rohini for your feedback and for accepting my patch. I'm all exited on my first ever patch to any open source project getting accepted. You folks are a welcoming community. I look forward to contributing more in the future.
        Rohini Palaniswamy made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags Reviewed [ 10343 ]
        Resolution Fixed [ 1 ]
        Hide
        Rohini Palaniswamy added a comment -

        +1. Committed to trunk (0.13). Thank you Hari.

        Show
        Rohini Palaniswamy added a comment - +1. Committed to trunk (0.13). Thank you Hari.
        Rohini Palaniswamy made changes -
        Summary SUM function for BigDecimal SUM function for BigDecimal and BigInteger
        Hariharasudhan Chinnan made changes -
        Attachment PIG_3569_v2.patch [ 12613765 ]
        Hide
        Hariharasudhan Chinnan added a comment -

        Incorporated feedback from Rohini. Added BigInteger files for SUM function. Added test cases to TestBuiltin.java. All tests execute successfully.

        Show
        Hariharasudhan Chinnan added a comment - Incorporated feedback from Rohini. Added BigInteger files for SUM function. Added test cases to TestBuiltin.java. All tests execute successfully.
        Hariharasudhan Chinnan made changes -
        Attachment PIG_3569.patch [ 12613646 ]
        Hide
        Rohini Palaniswamy added a comment -

        Thanks for the clarification. We can address that in a different jira.

        Few comments:
        1) //Memory Management? - That is not required. You can remove setting values to null as it is not a big object and java GC should handle that easily.
        2) BigDecimal d = ((BigDecimal) n).add(BigDecimal.ZERO);
        Why do you have to add zero? Isn't typecast just enough?
        3) AlgebraicBigDecimalSumBase - Can you please rename to AlgebraicBigDecimalMathBase and also correct the javadoc to reflect that, so that we can use this class to add MIN and MAX in a different jira in future.
        4) Please make a copy of that code for BigInteger as well.
        5) Please update testSUM* testcases in TestBuiltin.java for BigInteger and BigDecimal.

        Show
        Rohini Palaniswamy added a comment - Thanks for the clarification. We can address that in a different jira. Few comments: 1) //Memory Management? - That is not required. You can remove setting values to null as it is not a big object and java GC should handle that easily. 2) BigDecimal d = ((BigDecimal) n).add(BigDecimal.ZERO); Why do you have to add zero? Isn't typecast just enough? 3) AlgebraicBigDecimalSumBase - Can you please rename to AlgebraicBigDecimalMathBase and also correct the javadoc to reflect that, so that we can use this class to add MIN and MAX in a different jira in future. 4) Please make a copy of that code for BigInteger as well. 5) Please update testSUM* testcases in TestBuiltin.java for BigInteger and BigDecimal.
        Hide
        Hariharasudhan Chinnan added a comment -

        For MIN and MAX works because the seed for MIN Double and MAX Double could be clearly defined as double.positive_infinity and double.negative_infinity. For BigDecimal, no such boundaries could be defined. More framework level changes have to be implemented first to get MAX and MIN working.

        Show
        Hariharasudhan Chinnan added a comment - For MIN and MAX works because the seed for MIN Double and MAX Double could be clearly defined as double.positive_infinity and double.negative_infinity. For BigDecimal, no such boundaries could be defined. More framework level changes have to be implemented first to get MAX and MIN working.
        Hide
        Rohini Palaniswamy added a comment -

        Instead of AlgebraicBigDecimalSumBase, can you create a AlgebraicBigDecimalMathBase similar to AlgebraicFloatMathBase, so that it supports MIN and MAX as well. Also can you add code for BigInteger as well?

        Show
        Rohini Palaniswamy added a comment - Instead of AlgebraicBigDecimalSumBase, can you create a AlgebraicBigDecimalMathBase similar to AlgebraicFloatMathBase, so that it supports MIN and MAX as well. Also can you add code for BigInteger as well?
        Rohini Palaniswamy made changes -
        Status Reopened [ 4 ] Patch Available [ 10002 ]
        Rohini Palaniswamy made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Assignee Hariharasudhan Chinnan [ harichinnan ]
        Hide
        Rohini Palaniswamy added a comment -

        Added Hari to the contributors list and assigning the jira to him. Reopening jira for reviewing.

        Hariharasudhan Chinnan,
        Welcome to the pig community. While contributing, you need to make the status as Patch Available after you put up a patch. One of the committers will review the patch and after all review comments are addressed will mark it Resolved after committing the patch to svn. Please take a look at https://cwiki.apache.org/confluence/display/PIG/HowToContribute which has all the relevant information and process documented. Feel free to mail the dev list if you have any questions.

        Show
        Rohini Palaniswamy added a comment - Added Hari to the contributors list and assigning the jira to him. Reopening jira for reviewing. Hariharasudhan Chinnan , Welcome to the pig community. While contributing, you need to make the status as Patch Available after you put up a patch. One of the committers will review the patch and after all review comments are addressed will mark it Resolved after committing the patch to svn. Please take a look at https://cwiki.apache.org/confluence/display/PIG/HowToContribute which has all the relevant information and process documented. Feel free to mail the dev list if you have any questions.
        Hariharasudhan Chinnan made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hariharasudhan Chinnan made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Release Note Attached PIG_3569.patch was tested with latest from trunk.
        Hariharasudhan Chinnan made changes -
        Attachment PIG_3569.patch [ 12613646 ]
        Hide
        Hariharasudhan Chinnan added a comment -

        Patch for getting sum function to work with BigDecimal

        Show
        Hariharasudhan Chinnan added a comment - Patch for getting sum function to work with BigDecimal
        Hariharasudhan Chinnan made changes -
        Status Patch Available [ 10002 ] Open [ 1 ]
        Hariharasudhan Chinnan made changes -
        Field Original Value New Value
        Status Open [ 1 ] Patch Available [ 10002 ]
        Fix Version/s 0.13.0 [ 12324971 ]
        Hariharasudhan Chinnan created issue -

          People

          • Assignee:
            Hariharasudhan Chinnan
            Reporter:
            Hariharasudhan Chinnan
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development