giraph-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alessandro Presta" <alessan...@fb.com>
Subject Re: Review Request 12253: Type converters and giraph-hive cleanup
Date Sat, 06 Jul 2013 20:41:21 GMT

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


I'm ok with this approach (although I suggested an alternative to you offline, which is to
use a Python wrapper class).
I'm concerned with conversions that have loss of precision or possibility of overflow, like
double->float, long->int, int->byte, etc.
What's the reason to have them? Maybe you should have separate interfaces for converting from
Java to Writable and back, so that we only allow the correct directions.
E.g. int to LongWritable is ok, IntWritable to long is ok, but the other two combinations
are not.


giraph-core/src/main/java/org/apache/giraph/types/JavaWritableConverter.java
<https://reviews.apache.org/r/12253/#comment46493>

    Just a suggestion: how about we call this something like WritableWrapper, with methods
wrap() and unwrap()?



giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/TypedHiveToEdge.java
<https://reviews.apache.org/r/12253/#comment46496>

    Default value is the empty string?



giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/TypedHiveToEdge.java
<https://reviews.apache.org/r/12253/#comment46495>

    Newline after @Override. This will cause an AbstractMethodError otherwise.



giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/TypedHiveToVertex.java
<https://reviews.apache.org/r/12253/#comment46498>

    See above.



giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/TypedHiveToVertex.java
<https://reviews.apache.org/r/12253/#comment46497>

    See above.



giraph-hive/src/main/java/org/apache/giraph/hive/output/TypedVertexToHive.java
<https://reviews.apache.org/r/12253/#comment46499>

    See above.



giraph-hive/src/main/java/org/apache/giraph/hive/output/TypedVertexToHive.java
<https://reviews.apache.org/r/12253/#comment46500>

    See above.



giraph-hive/src/main/java/org/apache/giraph/hive/output/TypedVertexToHive.java
<https://reviews.apache.org/r/12253/#comment46501>

    See above.


- Alessandro Presta


On July 3, 2013, 4:43 p.m., Nitay Joffe wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12253/
> -----------------------------------------------------------
> 
> (Updated July 3, 2013, 4:43 p.m.)
> 
> 
> Review request for giraph.
> 
> 
> Bugs: GIRAPH-705
>     https://issues.apache.org/jira/browse/GIRAPH-705
> 
> 
> Repository: giraph-git
> 
> 
> Description
> -------
> 
> Type converters and giraph-hive cleanup
> 
> 
> Diffs
> -----
> 
>   giraph-core/src/main/java/org/apache/giraph/benchmark/PageRankBenchmark.java d88ff0d8376357035c6944fb2dbdcb9c4614d3d3

>   giraph-core/src/main/java/org/apache/giraph/jython/JythonUtils.java f456aa8b6a665925ceabe8983df07239dfc171ad

>   giraph-core/src/main/java/org/apache/giraph/types/BooleanToBooleanWritableConverter.java
PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/types/ByteToByteWritableConverter.java
PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/types/ByteToIntWritableConverter.java PRE-CREATION

>   giraph-core/src/main/java/org/apache/giraph/types/ByteToLongWritableConverter.java
PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/types/DoubleToDoubleWritableConverter.java
PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/types/DoubleToFloatWritableConverter.java
PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/types/FloatToDoubleWritableConverter.java
PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/types/FloatToFloatWritableConverter.java
PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/types/IntToByteWritableConverter.java PRE-CREATION

>   giraph-core/src/main/java/org/apache/giraph/types/IntToIntWritableConverter.java PRE-CREATION

>   giraph-core/src/main/java/org/apache/giraph/types/IntToLongWritableConverter.java PRE-CREATION

>   giraph-core/src/main/java/org/apache/giraph/types/JavaAndWritableClasses.java PRE-CREATION

>   giraph-core/src/main/java/org/apache/giraph/types/JavaWritableConverter.java PRE-CREATION

>   giraph-core/src/main/java/org/apache/giraph/types/LongToByteWritableConverter.java
PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/types/LongToIntWritableConverter.java PRE-CREATION

>   giraph-core/src/main/java/org/apache/giraph/types/LongToLongWritableConverter.java
PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/types/ShortToByteWritableConverter.java
PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/types/ShortToIntWritableConverter.java
PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/types/ShortToLongWritableConverter.java
PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/types/TypeConverters.java PRE-CREATION

>   giraph-core/src/main/java/org/apache/giraph/types/package-info.java PRE-CREATION 
>   giraph-core/src/main/java/org/apache/giraph/utils/ConfigurationUtils.java f0a7e4f784f970250f6f515a261b19f15e18fbc7

>   giraph-core/src/main/java/org/apache/giraph/utils/DistributedCacheUtils.java 6abe89b068e17823a3083061cb213c53c5b72f0d

>   giraph-core/src/test/java/org/apache/giraph/jython/TestJython.java 245d342ddf903c83130e299a33d0bee74cfc6949

>   giraph-hive/src/main/java/org/apache/giraph/hive/HiveGiraphRunner.java 589fed6c746c8f988f36cc439e78bc389b4b6e40

>   giraph-hive/src/main/java/org/apache/giraph/hive/common/HiveUtils.java 11b060f603b32ead84799a818d4604b0b1ba14af

>   giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/SimpleHiveToEdge.java 56b38f9d740aacdf72f1d710fbfa0c2de833936a

>   giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/TypedHiveToEdge.java PRE-CREATION

>   giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/TypedHiveToVertex.java
PRE-CREATION 
>   giraph-hive/src/main/java/org/apache/giraph/hive/output/AbstractVertexToHive.java 477ce6e3da499d96e3dc50fd35edeae649c85192

>   giraph-hive/src/main/java/org/apache/giraph/hive/output/HiveVertexWriter.java bb27f25c627f50c3b1bb48d027e42631f3b0855c

>   giraph-hive/src/main/java/org/apache/giraph/hive/output/TypedVertexToHive.java PRE-CREATION

>   giraph-hive/src/main/java/org/apache/giraph/hive/output/VertexToHive.java f9537a779a47d7eff374dcf4db2557f3495f6969

>   giraph-hive/src/main/java/org/apache/giraph/hive/types/HiveValueReader.java PRE-CREATION

>   giraph-hive/src/main/java/org/apache/giraph/hive/types/HiveValueWriter.java PRE-CREATION

>   giraph-hive/src/main/java/org/apache/giraph/hive/types/HiveVertexIdReader.java PRE-CREATION

>   giraph-hive/src/main/java/org/apache/giraph/hive/types/HiveVertexIdWriter.java PRE-CREATION

>   giraph-hive/src/main/java/org/apache/giraph/hive/types/TypedValueReader.java PRE-CREATION

>   giraph-hive/src/main/java/org/apache/giraph/hive/types/TypedValueWriter.java PRE-CREATION

>   giraph-hive/src/main/java/org/apache/giraph/hive/types/TypedVertexIdReader.java PRE-CREATION

>   giraph-hive/src/main/java/org/apache/giraph/hive/types/TypedVertexIdWriter.java PRE-CREATION

>   giraph-hive/src/main/java/org/apache/giraph/hive/types/package-info.java PRE-CREATION

> 
> Diff: https://reviews.apache.org/r/12253/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Nitay Joffe
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message