> On July 6, 2013, 8:41 p.m., Alessandro Presta wrote: > > 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.
The Python wrapper is for a different use case. The point of this diff is to allow reading/writing primitive types with Hive automatically, that is without having the user write any I/O code whatsoever. For example a PageRank-style job using a snippet like: https://gist.github.com/nitay/a01335d0d39bf1acad53 just works without additional Hive I/O code. This makes algorithms using only primitive Hive columns really easy. For your other point - sure I'll split the interface and remove the loss of precision ones. > On July 6, 2013, 8:41 p.m., Alessandro Presta wrote: > > giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/TypedHiveToEdge.java, > > line 64 > > <https://reviews.apache.org/r/12253/diff/1/?file=318056#file318056line64> > > > > Newline after @Override. This will cause an AbstractMethodError > > otherwise. Huh? You get an AbstractMethodError if you don't put a newline? I'm confused, since when does whitespace cause errors in Java? > On July 6, 2013, 8:41 p.m., Alessandro Presta wrote: > > giraph-core/src/main/java/org/apache/giraph/types/JavaWritableConverter.java, > > line 28 > > <https://reviews.apache.org/r/12253/diff/1/?file=318041#file318041line28> > > > > Just a suggestion: how about we call this something like > > WritableWrapper, with methods wrap() and unwrap()? sounds good I'll change it. > On July 6, 2013, 8:41 p.m., Alessandro Presta wrote: > > giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/TypedHiveToEdge.java, > > line 46 > > <https://reviews.apache.org/r/12253/diff/1/?file=318056#file318056line46> > > > > Default value is the empty string? I'll change it to null. - Nitay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12253/#review22798 ----------------------------------------------------------- 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 > >
