Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Pig >> mail # dev >> Review Request: Add BigInteger and BigDecimal to Pig


Copy link to this message
-
Re: Review Request: Add BigInteger and BigDecimal to Pig

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9012/#review15469
-----------------------------------------------------------

src/org/apache/pig/backend/hadoop/HDataType.java
<https://reviews.apache.org/r/9012/#comment33407>

    Missing 'break;'

src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POCast.java
<https://reviews.apache.org/r/9012/#comment33400>

    Please use braces after if for clarity.

src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POCast.java
<https://reviews.apache.org/r/9012/#comment33401>

    Please use braces after if for clarity.

src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POCast.java
<https://reviews.apache.org/r/9012/#comment33402>

    Please use braces after if for clarity.

src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POCast.java
<https://reviews.apache.org/r/9012/#comment33403>

    Please use braces after if for clarity.

src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POCast.java
<https://reviews.apache.org/r/9012/#comment33404>

    Please use braces after if for clarity.

src/org/apache/pig/backend/hadoop/hbase/HBaseBinaryConverter.java
<https://reviews.apache.org/r/9012/#comment33406>

    Why not use BigInteger(byte[]) or the String rep? Same comment for BigDecimal and the String rep.

src/org/apache/pig/builtin/TextLoader.java
<https://reviews.apache.org/r/9012/#comment33379>

    Message should be 'conversion to BigInteger'

src/org/apache/pig/builtin/TextLoader.java
<https://reviews.apache.org/r/9012/#comment33381>

    Message should be 'conversion to BigDecimal'

src/org/apache/pig/builtin/Utf8StorageConverter.java
<https://reviews.apache.org/r/9012/#comment33384>

    Shouldn't the charset be specified in the calls to getBytes for both BigInteger and BigDecimal?

src/org/apache/pig/data/DataType.java
<https://reviews.apache.org/r/9012/#comment33386>

    Indentation is messy here.

src/org/apache/pig/data/DataType.java
<https://reviews.apache.org/r/9012/#comment33387>

    Messy indent.

src/org/apache/pig/data/DataType.java
<https://reviews.apache.org/r/9012/#comment33388>

    Messy indent.

src/org/apache/pig/data/DataType.java
<https://reviews.apache.org/r/9012/#comment33391>

    Should  read 'BigDecimal' here and in next message.

src/org/apache/pig/data/DataType.java
<https://reviews.apache.org/r/9012/#comment33392>

    Messy indent

src/org/apache/pig/data/DefaultTuple.java
<https://reviews.apache.org/r/9012/#comment33394>

    I don't understand those two lines!

src/org/apache/pig/data/SizeUtil.java
<https://reviews.apache.org/r/9012/#comment33395>

    I thought BigDecimal and BigInteger did not have the same size, cf http://javamoods.blogspot.fr/2009/03/how-big-is-bigdecimal.html

src/org/apache/pig/impl/util/StorageUtil.java
<https://reviews.apache.org/r/9012/#comment33397>

    Shouldn't the charset be specified?

src/org/apache/pig/parser/LogicalPlanBuilder.java
<https://reviews.apache.org/r/9012/#comment33408>

    Since suffixes are 'BI' and 'BD', we should strip the last two characters of the string, not only the last one.
- Mathias Herberts
On Jan. 17, 2013, 9:02 p.m., Jonathan Coveney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9012/
> -----------------------------------------------------------
>
> (Updated Jan. 17, 2013, 9:02 p.m.)
>
>
> Review request for pig, Alan Gates and Mathias Herberts.
>
>
> Description
> -------
>
> This patch adds big integer and big decimal support to Pig. It could use more tests, something I'd appreciate feedback on (but I wanted to make sure the core implementation is good)
>
>
> This addresses bug PIG-2764.