You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by Daniel Dai <da...@gmail.com> on 2011/03/07 20:04:27 UTC

Review Request: Typed map for Pig

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

Review request for pig.


Summary
-------

Currently Pig map type is untyped, which means map value is always of bytearray(ie. unknown) type. In PIG-1277, we allow unknown type to be a shuffle key, which somewhat relieve the problem. However, typed map is still beneficial in that:

1. User can make semantic use of the map value type. Currently, user need to explicitly cast map value, which is ugly
2. Though PIG-1277 allow unknown type be a shuffle key, the performance suffers. We don't have a raw comparator for the unknown type, instead, we need to instantiate the value object and invoke its comparator


This addresses bug PIG-1876.
    https://issues.apache.org/jira/browse/PIG-1876


Diffs
-----

  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/LoadCaster.java 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/ResourceSchema.java 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/executionengine/physicalLayer/expressionOperators/POCast.java 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/backend/hadoop/hbase/HBaseBinaryConverter.java 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/builtin/BinStorage.java 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/builtin/TextLoader.java 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/builtin/Utf8StorageConverter.java 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/data/DataType.java 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/impl/io/ReadToEndLoader.java 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/impl/logicalLayer/schema/Schema.java 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/LogicalExpPlanMigrationVistor.java 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/expression/MapLookupExpression.java 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/relational/LogicalSchema.java 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/newplan/logical/visitor/ColumnAliasConversionVisitor.java 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AliasMasker.g 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/AstValidator.g 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/LogicalPlanGenerator.g 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/src/org/apache/pig/parser/QueryParser.g 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestPOCast.java 1078206 
  http://svn.apache.org/repos/asf/pig/trunk/test/org/apache/pig/test/TestTypedMap.java PRE-CREATION 

Diff: https://reviews.apache.org/r/472/diff


Testing
-------

test-patch:
     [exec] -1 overall.
     [exec]
     [exec]     +1 @author.  The patch does not contain any @author tags.
     [exec]
     [exec]     +1 tests included.  The patch appears to include 6 new or modified tests.
     [exec]
     [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
     [exec]
     [exec]     -1 javac.  The applied patch generated 866 javac compiler warnings (more than the trunk's current 860 warnings).
     [exec]
     [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
     [exec]
     [exec]     -1 release audit.  The applied patch generated 536 release audit warnings (more than the trunk's current 531 warning

All javac warnings are related to diff in parser. Only one new file TestTypedMap.java is added, and it contains proper header, so ignore release audit warning.

Unit test:
    all pass

End-to-end test:
    all pass


Thanks,

Daniel