----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9012/#review15496 -----------------------------------------------------------
src/org/apache/pig/backend/hadoop/hbase/HBaseBinaryConverter.java <https://reviews.apache.org/r/9012/#comment33457> I'm not sure what the contract of HBaseBinaryConverter is, which is why I didn't implement it. Theoretically, any Pig type can be serialized to Bytes since we do this in BinInterSedes, but here they were very particular to only use HBase's serialization, leading me to believe we should only convert bytes in the case where they can be transformed into the HBase analogue. We should find someone who knows more about this code, or dig around to see how it is used. src/org/apache/pig/builtin/Utf8StorageConverter.java <https://reviews.apache.org/r/9012/#comment33458> None of the other calls in this class specified the charset, so I did not either. Is there a reason why in this case specifically we need to, given we did bi => string => bytes? src/org/apache/pig/data/DataType.java <https://reviews.apache.org/r/9012/#comment33459> Odd. I think this is because of the preexisting presence of tabs in the file, which I will remove. src/org/apache/pig/data/DefaultTuple.java <https://reviews.apache.org/r/9012/#comment33460> This is janky, but to understand what is going on you need to look at DataReaderWriter to see how BigInteger and BigDecimal are being serialized in this specific case (which is not BinInterSedes, and in fact, I don't think this code is called anywhere...but still, it should be correct. We should probably unify all the serialization under BinInterSedes if possible, but that's a JIRA for another day). So basically, in this case we serialize as a String, and then decode. I'm going to update it to serialize the bytes directly...no reason not to. src/org/apache/pig/data/SizeUtil.java <https://reviews.apache.org/r/9012/#comment33464> Thus the TODO :S I completely punted on figuring out how big they are. I will try and tackle that now...thanks for the link, it will definitely help a lot src/org/apache/pig/impl/util/StorageUtil.java <https://reviews.apache.org/r/9012/#comment33469> Same comment here as earlier. It may need to be, but I'm unsure, and if it does, why doesn't it need to be in the other cases? - Jonathan Coveney 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. > https://issues.apache.org/jira/browse/PIG-2764 > > > Diffs > ----- > > .gitignore cc62d7d > src/org/apache/pig/LoadCaster.java 574769b > src/org/apache/pig/PigWarning.java 5de075f > src/org/apache/pig/StoreCaster.java 5fe48de > src/org/apache/pig/backend/hadoop/BigDecimalWritable.java PRE-CREATION > src/org/apache/pig/backend/hadoop/BigIntegerWritable.java PRE-CREATION > src/org/apache/pig/backend/hadoop/HDataType.java 84a56b8 > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/JobControlCompiler.java > 96fba6b > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigBigDecimalRawComparator.java > PRE-CREATION > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigBigIntegerRawComparator.java > PRE-CREATION > > src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/partitioners/WeightedRangePartitioner.java > 9749339 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/PhysicalOperator.java > f40eb43 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/Add.java > c84b767 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/ConstantExpression.java > db3840f > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/Divide.java > 4656c28 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/EqualToExpr.java > 6683beb > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/ExpressionOperator.java > 2806336 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/GTOrEqualToExpr.java > d64a080 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/GreaterThanExpr.java > 704d0b8 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/LTOrEqualToExpr.java > 9dc929e > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/LessThanExpr.java > 0320698 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/Mod.java > 6819185 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/Multiply.java > 7b57bed > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/NotEqualToExpr.java > 79a4461 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POBinCond.java > 08544d5 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POCast.java > e8c2f2c > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POIsNull.java > f20b839 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/PONegative.java > c076ae7 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POProject.java > 8887133 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POUserComparisonFunc.java > 479eb83 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POUserFunc.java > 3c7e741 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/Subtract.java > 79d4c73 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POForEach.java > bf2ba08 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POLocalRearrange.java > ddb25f1 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POPartialAgg.java > aa11409 > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POPreCombinerLocalRearrange.java > 52401eb > > src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/relationalOperators/POSort.java > ad33e7b > src/org/apache/pig/backend/hadoop/hbase/HBaseBinaryConverter.java 60a5899 > src/org/apache/pig/backend/hadoop/hbase/HBaseStorage.java a6f4ea6 > src/org/apache/pig/builtin/ABS.java 8a7c631 > src/org/apache/pig/builtin/BigDecimalAbs.java PRE-CREATION > src/org/apache/pig/builtin/BigIntegerAbs.java PRE-CREATION > src/org/apache/pig/builtin/BinStorage.java 38b4492 > src/org/apache/pig/builtin/TextLoader.java d5bcf02 > src/org/apache/pig/builtin/Utf8StorageConverter.java da12ed6 > src/org/apache/pig/data/BinInterSedes.java e851d8b > src/org/apache/pig/data/DataReaderWriter.java 37a162a > src/org/apache/pig/data/DataType.java e4c7b98 > src/org/apache/pig/data/DefaultTuple.java e182b06 > src/org/apache/pig/data/SizeUtil.java 90e5d94 > src/org/apache/pig/data/TypeAwareTuple.java cf78d0a > src/org/apache/pig/impl/io/NullableBigDecimalWritable.java PRE-CREATION > src/org/apache/pig/impl/io/NullableBigIntegerWritable.java PRE-CREATION > src/org/apache/pig/impl/logicalLayer/schema/SchemaUtil.java c257ada > src/org/apache/pig/impl/util/CastUtils.java 309130a > src/org/apache/pig/impl/util/NumValCarrier.java af519fd > src/org/apache/pig/impl/util/StorageUtil.java 087651a > src/org/apache/pig/newplan/logical/rules/ConstExpEvaluator.java f968294 > src/org/apache/pig/newplan/logical/visitor/TypeCheckingExpVisitor.java > d7a1370 > src/org/apache/pig/parser/AliasMasker.g 8cb1b30 > src/org/apache/pig/parser/AstPrinter.g c6f7ff8 > src/org/apache/pig/parser/AstValidator.g 8646078 > src/org/apache/pig/parser/LogicalPlanBuilder.java 699f7a6 > src/org/apache/pig/parser/LogicalPlanGenerator.g 9b9c099 > src/org/apache/pig/parser/QueryLexer.g f201916 > src/org/apache/pig/parser/QueryParser.g 642884e > src/org/apache/pig/pen/AugmentBaseDataVisitor.java 57dcb14 > > test/e2e/pig/udfs/java/org/apache/pig/test/udf/storefunc/PigPerformanceLoader.java > 94c28be > test/org/apache/pig/test/TestAdd.java de9e90d > test/org/apache/pig/test/TestBestFitCast.java e3172d2 > test/org/apache/pig/test/TestPOCast.java 72787b3 > > Diff: https://reviews.apache.org/r/9012/diff/ > > > Testing > ------- > > > Thanks, > > Jonathan Coveney > >
