Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.7.0
    • Fix Version/s: 0.7.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Incompatible change, Reviewed

      Description

      There have been some changes (as recorded in the Changes Section, Nov 2 2009 sub section of http://wiki.apache.org/pig/LoadStoreRedesignProposal) in the load/store interfaces - this jira is to track the task of making those changes under src. Changes under test will be addresses in a different jira.

      1. PIG-1090.patch
        39 kB
        Pradeep Kamath
      2. PIG-1090-10.patch
        0.9 kB
        Richard Ding
      3. PIG-1090-11.patch
        0.9 kB
        Pradeep Kamath
      4. PIG-1090-12.patch
        21 kB
        Daniel Dai
      5. PIG-1090-13.patch
        155 kB
        Richard Ding
      6. PIG-1090-14.patch
        8 kB
        Richard Ding
      7. PIG-1090-15.patch
        10 kB
        Daniel Dai
      8. PIG-1090-16.patch
        0.9 kB
        Daniel Dai
      9. PIG-1090-17.patch
        7 kB
        Daniel Dai
      10. PIG-1090-18.patch
        20 kB
        Pradeep Kamath
      11. PIG-1090-19.patch
        2 kB
        Richard Ding
      12. PIG-1090-2.patch
        19 kB
        Pradeep Kamath
      13. PIG-1090-20.patch
        3 kB
        Daniel Dai
      14. PIG-1090-21.patch
        12 kB
        Pradeep Kamath
      15. PIG-1090-22.patch
        16 kB
        Daniel Dai
      16. PIG-1090-3.patch
        17 kB
        Richard Ding
      17. PIG-1090-4.patch
        69 kB
        Pradeep Kamath
      18. PIG-1090-6.patch
        25 kB
        Richard Ding
      19. PIG-1090-7.patch
        85 kB
        Pradeep Kamath
      20. PIG-1090-8.patch
        84 kB
        Pradeep Kamath
      21. PIG-1090-9.patch
        22 kB
        Richard Ding
      22. PIG-1190-5.patch
        72 kB
        Richard Ding

        Activity

        Hide
        Pradeep Kamath added a comment -

        Attached patch to make changes under src directory of the code base to comply with interface changes. Changes have not been made under test since cleaning up tests will be undertaken in a separate jira.

        Here are results from a local 'test-patch' run:

             [exec] -1 overall.
             [exec]
             [exec]     +1 @author.  The patch does not contain any @author tags.
             [exec]
             [exec]     -1 tests included.  The patch doesn't appear to include any new or modified tests.
             [exec]                         Please justify why no tests are needed for this patch.
             [exec]
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec]
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec]
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec]
             [exec]     -1 release audit.  The applied patch generated 331 release audit warnings (more than the trunk's current 330 warnings).
             [exec]
             [exec]
             [exec]
        
        • Since this patch only makes changes to comply with recent interface changes there are no new tests added.
        • The release audit warning is due to a difference in a html file:
          cat /homes/pradeepk/tmp/releaseAuditDiffWarnings.txt
          
          73d72
          <      [java]  !????? /tmp/PIG-1090/load-store-redesign/build/pig-0.6.0-dev/docs/jdiff/changes/org.apache.pig.backend.hadoop.executionengine.physicalLayer.expressionOperators.POCast.html
          
        Show
        Pradeep Kamath added a comment - Attached patch to make changes under src directory of the code base to comply with interface changes. Changes have not been made under test since cleaning up tests will be undertaken in a separate jira. Here are results from a local 'test-patch' run: [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] -1 tests included. The patch doesn't appear to include any new or modified tests. [exec] Please justify why no tests are needed for this patch. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] -1 release audit. The applied patch generated 331 release audit warnings (more than the trunk's current 330 warnings). [exec] [exec] [exec] Since this patch only makes changes to comply with recent interface changes there are no new tests added. The release audit warning is due to a difference in a html file: cat /homes/pradeepk/tmp/releaseAuditDiffWarnings.txt 73d72 < [java] !????? /tmp/PIG-1090/load-store-redesign/build/pig-0.6.0-dev/docs/jdiff/changes/org.apache.pig.backend.hadoop.executionengine.physicalLayer.expressionOperators.POCast.html
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12424793/PIG-1090.patch
        against trunk revision 835499.

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no tests are needed for this patch.

        -1 patch. The patch command could not apply the patch.

        Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h8.grid.sp2.yahoo.net/47/console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12424793/PIG-1090.patch against trunk revision 835499. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no tests are needed for this patch. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h8.grid.sp2.yahoo.net/47/console This message is automatically generated.
        Hide
        Pradeep Kamath added a comment -

        The above issue with Hadoop QA is because this patch is for load-store-redesign branch and not for trunk

        Show
        Pradeep Kamath added a comment - The above issue with Hadoop QA is because this patch is for load-store-redesign branch and not for trunk
        Hide
        Alan Gates added a comment -

        Patch looks good.

        What's the ReadToEndLoader?

        What's the plan for BinStorage? Are we going to write Input and Output Formats for it? If we have to do that is there an existing binary storage format with existing input and output formats that we can use (like Avro or something)?

        Show
        Alan Gates added a comment - Patch looks good. What's the ReadToEndLoader? What's the plan for BinStorage? Are we going to write Input and Output Formats for it? If we have to do that is there an existing binary storage format with existing input and output formats that we can use (like Avro or something)?
        Hide
        Pradeep Kamath added a comment -

        What's the ReadToEndLoader?

        This is a internal utility LoadFunc I wrote to make it easy to read side files. It encapsulates the real Loader. Though this has been implemented as a LoadFunc, the only LoadFunc method which is truly implemented is getNext(). The usage pattern is to construct an instance using the constructor which would take a reference to the true LoadFunc (which can read the side file data) and then repeatedly call getNext() till null is encountered in the return value. The implementation of ReadToEndLoader hides the actions of getting InputSplits from the underlying InputFormat and then processing each split by getting the RecordReader and processing data in the split before moving to the next.

        What's the plan for BinStorage?

        An input and output format has already been created and checked in in this branch for Binstorage

        Show
        Pradeep Kamath added a comment - What's the ReadToEndLoader? This is a internal utility LoadFunc I wrote to make it easy to read side files. It encapsulates the real Loader. Though this has been implemented as a LoadFunc, the only LoadFunc method which is truly implemented is getNext(). The usage pattern is to construct an instance using the constructor which would take a reference to the true LoadFunc (which can read the side file data) and then repeatedly call getNext() till null is encountered in the return value. The implementation of ReadToEndLoader hides the actions of getting InputSplits from the underlying InputFormat and then processing each split by getting the RecordReader and processing data in the split before moving to the next. What's the plan for BinStorage? An input and output format has already been created and checked in in this branch for Binstorage
        Hide
        Pradeep Kamath added a comment -

        Patch committed to load-store-redesign branch

        Show
        Pradeep Kamath added a comment - Patch committed to load-store-redesign branch
        Hide
        Pradeep Kamath added a comment -

        Reopening since we need to implement LoadMetadata interface in BinStorage so as to implement the getSchema() method in that interface - this will depend on the decision for the comment - http://issues.apache.org/jira/browse/PIG-966?focusedCommentId=12780873&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#action_12780873

        Show
        Pradeep Kamath added a comment - Reopening since we need to implement LoadMetadata interface in BinStorage so as to implement the getSchema() method in that interface - this will depend on the decision for the comment - http://issues.apache.org/jira/browse/PIG-966?focusedCommentId=12780873&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#action_12780873
        Hide
        Pradeep Kamath added a comment -

        Attached patch for adding getSchema() functionality in BinStorage

        The output from running ant "test-patch" target locally:

           [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 7 new or modified tests.
             [exec]
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec]
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec]
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec]
             [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
             [exec]
             [exec]
             [exec]
        
        
        Show
        Pradeep Kamath added a comment - Attached patch for adding getSchema() functionality in BinStorage The output from running ant "test-patch" target locally: [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 7 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] [exec]
        Hide
        Olga Natkovich added a comment -

        +1

        Show
        Olga Natkovich added a comment - +1
        Hide
        Richard Ding added a comment -

        This patch deals with conversions between Pig Schema and the new Resource Schema.

        Given a resource schema, one calls the folllowing static method of Schema to convert it to a Pig schema:

        public static Schema getPigSchema(ResourceSchema rSchema) throws FrontendException;
        

        One the other hand, a Pig schema is converted to a Resource Schema by passing it to a resource schema constructor:

        public ResourceSchema(Schema pigSchema) ;
        

        One restriction for Resource Schema is that a Bag defined by Resource Schema must contain exactly one field of type Tuple. Ohterwise an exception will thrown during the conversion.

        Here are results from a local 'test-patch' run:

             [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 3 new or modified tests.
             [exec] 
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec] 
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec] 
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec] 
             [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
             [exec] 
        
        
        Show
        Richard Ding added a comment - This patch deals with conversions between Pig Schema and the new Resource Schema. Given a resource schema, one calls the folllowing static method of Schema to convert it to a Pig schema: public static Schema getPigSchema(ResourceSchema rSchema) throws FrontendException; One the other hand, a Pig schema is converted to a Resource Schema by passing it to a resource schema constructor: public ResourceSchema(Schema pigSchema) ; One restriction for Resource Schema is that a Bag defined by Resource Schema must contain exactly one field of type Tuple. Ohterwise an exception will thrown during the conversion. Here are results from a local 'test-patch' run: [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 3 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec]
        Hide
        Dmitriy V. Ryaboy added a comment -

        I thought Alan wanted to keep the internal state of ResourceSchema public?
        This patch changes the visibility of internal members to private.

        Show
        Dmitriy V. Ryaboy added a comment - I thought Alan wanted to keep the internal state of ResourceSchema public? This patch changes the visibility of internal members to private.
        Hide
        Richard Ding added a comment -

        The problem is that the getters/setters for the internal members are also defined for ResourceSchema. I felt that we should choose one way to access the internal members.

        Show
        Richard Ding added a comment - The problem is that the getters/setters for the internal members are also defined for ResourceSchema. I felt that we should choose one way to access the internal members.
        Hide
        Dmitriy V. Ryaboy added a comment -

        Richard,
        I added the getters/setters so that ResourceSchema can be treated as a POJO, and standard serialization tools can easily interact with it, in PIG-760.
        Alan said in PIG-760 that he is fine with adding getters and setters, but feels strongly that direct access to these members should still be allowed, for simplicity's sake.
        I'm fine with the visibility being either way, as long as the getters/setters stay in (although perhaps protected would be a better choice than private).

        Show
        Dmitriy V. Ryaboy added a comment - Richard, I added the getters/setters so that ResourceSchema can be treated as a POJO, and standard serialization tools can easily interact with it, in PIG-760 . Alan said in PIG-760 that he is fine with adding getters and setters, but feels strongly that direct access to these members should still be allowed, for simplicity's sake. I'm fine with the visibility being either way, as long as the getters/setters stay in (although perhaps protected would be a better choice than private).
        Hide
        Pradeep Kamath added a comment -

        I committed the latest patch - thanks Richard! Looks like I committed it while the above discussion was still on - if things need to be changed, please attach a small patch for the same and I can commit it - if we decide to keep things the way they are now that's fine.

        Show
        Pradeep Kamath added a comment - I committed the latest patch - thanks Richard! Looks like I committed it while the above discussion was still on - if things need to be changed, please attach a small patch for the same and I can commit it - if we decide to keep things the way they are now that's fine.
        Hide
        Pradeep Kamath added a comment -

        Attached patch for changes to enable LoadMetadata interface. A main piece of code which has been introduced in this patch is the PartitionFitlerOptimizer. Load statements in pig which work with metadata wherein the data has partition columns can be followed by a filter statement which can have conditions on the partition columns. The filter can also have conditions on non partition columns. The PartitionFitlerOptimizer looks for conditions on partition columns in the filter following the load and extracts them out and passes it to the LoadMetadata implementation in the loader. Other changes in the code are to enable interacting with the LoadMetadata implementation in the loader.

        Show
        Pradeep Kamath added a comment - Attached patch for changes to enable LoadMetadata interface. A main piece of code which has been introduced in this patch is the PartitionFitlerOptimizer. Load statements in pig which work with metadata wherein the data has partition columns can be followed by a filter statement which can have conditions on the partition columns. The filter can also have conditions on non partition columns. The PartitionFitlerOptimizer looks for conditions on partition columns in the filter following the load and extracts them out and passes it to the LoadMetadata implementation in the loader. Other changes in the code are to enable interacting with the LoadMetadata implementation in the loader.
        Hide
        Pradeep Kamath added a comment -

        The result of running "test-patch" ant target locally for the latest patch (PIG-1090-4.patch):

            [exec]
             [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 3 new or modified tests.
             [exec]
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec]
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec]
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec]
             [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
             [exec]
             [exec]
             [exec]
        
        
        Show
        Pradeep Kamath added a comment - The result of running "test-patch" ant target locally for the latest patch ( PIG-1090 -4.patch): [exec] [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 3 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] [exec] [exec]
        Hide
        Richard Ding added a comment -

        This patch removed the following files from repository:

        D      test/org/apache/pig/test/TestPigLineRecordReader.java
        D      src/org/apache/pig/backend/executionengine/PigSlice.java
        D      src/org/apache/pig/backend/executionengine/PigSlicer.java
        D      src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/SliceWrapper.java
        D      src/org/apache/pig/impl/io/PigLineRecordReader.java
        D      src/org/apache/pig/impl/io/ValidatingInputFileSpec.java
        D      src/org/apache/pig/builtin/PigDump.java
        

        Here is the output of '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 11 new or modified tests.
             [exec] 
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec] 
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec] 
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec] 
             [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
        
        Show
        Richard Ding added a comment - This patch removed the following files from repository: D test/org/apache/pig/test/TestPigLineRecordReader.java D src/org/apache/pig/backend/executionengine/PigSlice.java D src/org/apache/pig/backend/executionengine/PigSlicer.java D src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/SliceWrapper.java D src/org/apache/pig/impl/io/PigLineRecordReader.java D src/org/apache/pig/impl/io/ValidatingInputFileSpec.java D src/org/apache/pig/builtin/PigDump.java Here is the output of '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 11 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
        Hide
        Dmitriy V. Ryaboy added a comment -

        I think LoadMetadata should not be responsible for pushing down filters. This is something LoadPushdown is for. LoadMetadata is, according to the redesign proposal, an interface for loading metadata, not using it to guide processing.

        The filter pushdown optimization seems like a fairly involved piece of code. Can we break this out into a separate ticket, to separate the issue of fulfilling the redesign from the issue of implementing this optimization?

        Show
        Dmitriy V. Ryaboy added a comment - I think LoadMetadata should not be responsible for pushing down filters. This is something LoadPushdown is for. LoadMetadata is, according to the redesign proposal, an interface for loading metadata, not using it to guide processing. The filter pushdown optimization seems like a fairly involved piece of code. Can we break this out into a separate ticket, to separate the issue of fulfilling the redesign from the issue of implementing this optimization?
        Hide
        Pradeep Kamath added a comment -

        Dmitriy,
        The method in LoadMetadata that is implemented in PIG-1090-4.patch is to set partition filter and not to implement filter pushdown in general. Only partition filter conditions are pushed down through LoadMetadata as per the redesign proposal. As you rightly pointed pushing down filters in general will be done through the LoadPushDown interface which currently only has a pushProjection method - at a later point when Pig is able to push down filters, a pushFilter method can be introduced. It is not currently present because we don't know what the argument would look like eventually when we do push down filters. The optimization in the patch attached to this jira is only to extract conditions on partition columns which is needed to be able to call LoadMetadata.setPartitionFitler() method and hence was added in this patch.

        Show
        Pradeep Kamath added a comment - Dmitriy, The method in LoadMetadata that is implemented in PIG-1090 -4.patch is to set partition filter and not to implement filter pushdown in general. Only partition filter conditions are pushed down through LoadMetadata as per the redesign proposal. As you rightly pointed pushing down filters in general will be done through the LoadPushDown interface which currently only has a pushProjection method - at a later point when Pig is able to push down filters, a pushFilter method can be introduced. It is not currently present because we don't know what the argument would look like eventually when we do push down filters. The optimization in the patch attached to this jira is only to extract conditions on partition columns which is needed to be able to call LoadMetadata.setPartitionFitler() method and hence was added in this patch.
        Hide
        Pradeep Kamath added a comment -

        +1 for PIG-1090-5.patch, patch committed to load-store-redesign branch.

        Show
        Pradeep Kamath added a comment - +1 for PIG-1090 -5.patch, patch committed to load-store-redesign branch.
        Hide
        Thejas M Nair added a comment -

        I have reviewed the changes related to partition filter extraction.

        • The case where load statement has a user defined schema with different column names for partition column needs to be handled.
        • src/org/apache/pig/LoadMetadata.java - I think we should document in the comments that the load function does not have to implement setParitionFilter even if it implements other parts of LoadMetadata interface. And that it can communicate this to pig by returning null in getPartitionKeys.
        • src/org/apache/pig/Expression.java - in BinaryExpression.toString() , need to add parenthesis around the arguments , if they are binary expressions so that the string represents the correct operator precedence as specified in the filter condition. eg (a = 1 or b = 1) and c = 1 now gets converted to "a = 1 or b = 1 and c = 1" .
        • src/org/apache/pig/Expression.java - in Const.toString() - It will be better to use single quotes instead of double quotes around string constants, as string literals in SQL (standard) and pig-latin are single-quoted .
        Show
        Thejas M Nair added a comment - I have reviewed the changes related to partition filter extraction. The case where load statement has a user defined schema with different column names for partition column needs to be handled. src/org/apache/pig/LoadMetadata.java - I think we should document in the comments that the load function does not have to implement setParitionFilter even if it implements other parts of LoadMetadata interface. And that it can communicate this to pig by returning null in getPartitionKeys. src/org/apache/pig/Expression.java - in BinaryExpression.toString() , need to add parenthesis around the arguments , if they are binary expressions so that the string represents the correct operator precedence as specified in the filter condition. eg (a = 1 or b = 1) and c = 1 now gets converted to "a = 1 or b = 1 and c = 1" . src/org/apache/pig/Expression.java - in Const.toString() - It will be better to use single quotes instead of double quotes around string constants, as string literals in SQL (standard) and pig-latin are single-quoted .
        Hide
        Thejas M Nair added a comment -

        My previous comment is regarding PIG-1090-4.patch .

        Show
        Thejas M Nair added a comment - My previous comment is regarding PIG-1090 -4.patch .
        Hide
        Daniel Dai added a comment -

        Regarding to PIG-1090-4.patch, In LOLoad.getSchema, we shall remove the lines to setup "pig.loader.signature". In the new design, UDF writers should use signature inside the LoadFun to keep track of signature rather than the Configuration.

        Other part relate to signature and push projection looks good to me.

        Show
        Daniel Dai added a comment - Regarding to PIG-1090 -4.patch, In LOLoad.getSchema, we shall remove the lines to setup "pig.loader.signature". In the new design, UDF writers should use signature inside the LoadFun to keep track of signature rather than the Configuration. Other part relate to signature and push projection looks good to me.
        Hide
        Richard Ding added a comment -

        This patch removed one deprecated file - WrappedIOException.java.

        Here are the results of local 'test-patch' run:

             [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 3 new or modified tests.
             [exec] 
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec] 
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec] 
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec] 
             [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
        
        Show
        Richard Ding added a comment - This patch removed one deprecated file - WrappedIOException.java. Here are the results of local 'test-patch' run: [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 3 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
        Hide
        Pradeep Kamath added a comment -

        Attached new patch regarding support for LoadMetadata (second version of PIG-1090-4.patch) addressing review comments from Thejas and Daniel.

        Thejas/Daniel - can you review and let me know if the changes are fine.

        Results from running "test-patch" ant target locally:

             [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 3 new or modified tests.
             [exec]
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec]
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec]
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec]
             [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
        
        
        Show
        Pradeep Kamath added a comment - Attached new patch regarding support for LoadMetadata (second version of PIG-1090 -4.patch) addressing review comments from Thejas and Daniel. Thejas/Daniel - can you review and let me know if the changes are fine. Results from running "test-patch" ant target locally: [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 3 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
        Hide
        Pradeep Kamath added a comment -

        +1 to PIG-1090-6.patch, patch committed, thanks Richard!

        Show
        Pradeep Kamath added a comment - +1 to PIG-1090 -6.patch, patch committed, thanks Richard!
        Hide
        Pradeep Kamath added a comment -

        Attached PIG-1090-8.patch which is a minor update on PIG-1090-7.patch (both are follow up patches on PIG-1090-4.patch to support LoadMetadata interface). The updates are to remove an extra check in PartitionFitlerOptimizer which was already performed earlier in the code and to regenerate the patch against latest svn.

        Show
        Pradeep Kamath added a comment - Attached PIG-1090 -8.patch which is a minor update on PIG-1090 -7.patch (both are follow up patches on PIG-1090 -4.patch to support LoadMetadata interface). The updates are to remove an extra check in PartitionFitlerOptimizer which was already performed earlier in the code and to regenerate the patch against latest svn.
        Hide
        Thejas M Nair added a comment -

        +1 to the partition filter extraction changes in PIG-1090-8.patch.
        I have reviewed the following files
        test/org/apache/pig/test/TestPartitionFilterExtractor.java
        src/org/apache/pig/Expression.java
        src/org/apache/pig/impl/logicalLayer/optimizer/PartitionFilterOptimizer.java
        src/org/apache/pig/LoadMetadata.java
        src/org/apache/pig/impl/logicalLayer/optimizer/LogicalOptimizer.java
        src/org/apache/pig/builtin/BinStorage.java
        src/org/apache/pig/builtin/PigStorage.java
        src/org/apache/pig/builtin/TextLoader.java

        Show
        Thejas M Nair added a comment - +1 to the partition filter extraction changes in PIG-1090 -8.patch. I have reviewed the following files test/org/apache/pig/test/TestPartitionFilterExtractor.java src/org/apache/pig/Expression.java src/org/apache/pig/impl/logicalLayer/optimizer/PartitionFilterOptimizer.java src/org/apache/pig/LoadMetadata.java src/org/apache/pig/impl/logicalLayer/optimizer/LogicalOptimizer.java src/org/apache/pig/builtin/BinStorage.java src/org/apache/pig/builtin/PigStorage.java src/org/apache/pig/builtin/TextLoader.java
        Hide
        Richard Ding added a comment -

        This patch replaced msStorage with a Configuration object in LOLoad and fixed corresponding test cases.

        The results of "test-patch" run:

             [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 15 new or modified tests.
             [exec] 
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec] 
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec] 
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec] 
             [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
        
        
        Show
        Richard Ding added a comment - This patch replaced msStorage with a Configuration object in LOLoad and fixed corresponding test cases. The results of "test-patch" run: [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 15 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
        Hide
        Daniel Dai added a comment -

        +1 for PIG-1090-8.patch

        Show
        Daniel Dai added a comment - +1 for PIG-1090 -8.patch
        Hide
        Pradeep Kamath added a comment -

        Committed PIG-1090-8.patch and PIG-1090-9.patch to branch.

        Show
        Pradeep Kamath added a comment - Committed PIG-1090 -8.patch and PIG-1090 -9.patch to branch.
        Hide
        Richard Ding added a comment -

        Remove 'FIXME" comments from JobControlCompiler class and open JIRA Pig-1181 to track the issue.

        Show
        Richard Ding added a comment - Remove 'FIXME" comments from JobControlCompiler class and open JIRA Pig-1181 to track the issue.
        Hide
        Pradeep Kamath added a comment -

        Patch to fix issue in TextLoader() wherein the output would be scrambled if different lines in the input are or different sizes. The issue was that TextInputFormat which is used by TextLoader reuses the memory buffer to provide each line of the input. So in TextLoader we need to make a copy for use.

        Show
        Pradeep Kamath added a comment - Patch to fix issue in TextLoader() wherein the output would be scrambled if different lines in the input are or different sizes. The issue was that TextInputFormat which is used by TextLoader reuses the memory buffer to provide each line of the input. So in TextLoader we need to make a copy for use.
        Hide
        Richard Ding added a comment -

        +1 for PIG-1090-11 patch.

        Show
        Richard Ding added a comment - +1 for PIG-1090 -11 patch.
        Hide
        Pradeep Kamath added a comment -

        PIG-1090-10.patch and PIG-1090-11.patch have been committed.

        Show
        Pradeep Kamath added a comment - PIG-1090 -10.patch and PIG-1090 -11.patch have been committed.
        Hide
        Richard Ding added a comment -

        This patch (13) ports piggybank code base to the load-store-redesign branch. Now piggybank compiles and passes all the unit tests on the branch.

        Among the ported storage UDFs are MultiStorage (PIG-958) and PigStorageSchema (PIG-760).

        Show
        Richard Ding added a comment - This patch (13) ports piggybank code base to the load-store-redesign branch. Now piggybank compiles and passes all the unit tests on the branch. Among the ported storage UDFs are MultiStorage ( PIG-958 ) and PigStorageSchema ( PIG-760 ).
        Hide
        Daniel Dai added a comment -

        PIG-1090-12.patch is for the StoreFunc changes.

        Show
        Daniel Dai added a comment - PIG-1090 -12.patch is for the StoreFunc changes.
        Hide
        Pradeep Kamath added a comment -

        Review comments for PIG-1090-12.patch:
        1) I noticed that in JobControlCompiler, there is a call to setStoreLocation besides a call to checkSchema(). If the setStoreLocation is not needed, maybe it can removed since this method is called in PigOutputFormat. Also, please update the comments in StoreFunc.checkschema to mention that the method is called only if schema of the data is not null.
        2) Comments in PigOutputFormat in places where setUpUdfContext() is called would be useful

        Show
        Pradeep Kamath added a comment - Review comments for PIG-1090 -12.patch: 1) I noticed that in JobControlCompiler, there is a call to setStoreLocation besides a call to checkSchema(). If the setStoreLocation is not needed, maybe it can removed since this method is called in PigOutputFormat. Also, please update the comments in StoreFunc.checkschema to mention that the method is called only if schema of the data is not null. 2) Comments in PigOutputFormat in places where setUpUdfContext() is called would be useful
        Hide
        Daniel Dai added a comment -

        PIG-1090-12.patch committed.

        Show
        Daniel Dai added a comment - PIG-1090 -12.patch committed.
        Hide
        Richard Ding added a comment -

        The main updates of the latest patch 13 is the following:

        • Remove the these files:
        D      src/org/apache/pig/experimental/LoadMetadata.java
        D      src/org/apache/pig/experimental/ResourceStatistics.java
        D      src/org/apache/pig/experimental/ResourceSchema.java
        D      src/org/apache/pig/experimental/JsonMetadata.java
        D      src/org/apache/pig/experimental/StoreMetadata.java
        
        • Move JsonMetadata.java to the package org.apache.pig.piggybank.storage
        • Move StoreMetadata.java to the package org.apache.pig
        • Modify PigStorageSchema class to use PigOutputCommitter to store the metadata with the output file (PIG-760).

        Dmitriy,

        Can you review PIG-760 related changes?

        Thanks.

        Show
        Richard Ding added a comment - The main updates of the latest patch 13 is the following: Remove the these files: D src/org/apache/pig/experimental/LoadMetadata.java D src/org/apache/pig/experimental/ResourceStatistics.java D src/org/apache/pig/experimental/ResourceSchema.java D src/org/apache/pig/experimental/JsonMetadata.java D src/org/apache/pig/experimental/StoreMetadata.java Move JsonMetadata.java to the package org.apache.pig.piggybank.storage Move StoreMetadata.java to the package org.apache.pig Modify PigStorageSchema class to use PigOutputCommitter to store the metadata with the output file ( PIG-760 ). Dmitriy, Can you review PIG-760 related changes? Thanks.
        Hide
        Richard Ding added a comment -

        Sync patch-13 with patch-12.

        Show
        Richard Ding added a comment - Sync patch-13 with patch-12.
        Hide
        Dmitriy V. Ryaboy added a comment -

        Richard,
        I'll check it out, thanks.

        Show
        Dmitriy V. Ryaboy added a comment - Richard, I'll check it out, thanks.
        Hide
        Dmitriy V. Ryaboy added a comment -

        Richard, patch doesn't apply when it encounters src/org/apache/pig/StoreMetadata.java:

        dvryaboy@abacus:~/src/pig-lsr2$ patch -p0 < PIG-1090-13.patch
        patching file src/org/apache/pig/ResourceStatistics.java
        patching file src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigOutputCommitter.java
        patching file src/org/apache/pig/data/DataType.java
        can't find file to patch at input line 348
        Perhaps you used the wrong -p or --strip option?
        The text leading up to this was:
        --------------------------

        Index: src/org/apache/pig/StoreMetadata.java
        ===================================================================
        — src/org/apache/pig/StoreMetadata.java (revision 899344)
        +++ src/org/apache/pig/StoreMetadata.java (working copy)
        --------------------------
        Show
        Dmitriy V. Ryaboy added a comment - Richard, patch doesn't apply when it encounters src/org/apache/pig/StoreMetadata.java: dvryaboy@abacus:~/src/pig-lsr2$ patch -p0 < PIG-1090 -13.patch patching file src/org/apache/pig/ResourceStatistics.java patching file src/org/apache/pig/backend/hadoop/executionengine/mapReduceLayer/PigOutputCommitter.java patching file src/org/apache/pig/data/DataType.java can't find file to patch at input line 348 Perhaps you used the wrong -p or --strip option? The text leading up to this was: -------------------------- Index: src/org/apache/pig/StoreMetadata.java =================================================================== — src/org/apache/pig/StoreMetadata.java (revision 899344) +++ src/org/apache/pig/StoreMetadata.java (working copy) --------------------------
        Hide
        Richard Ding added a comment -

        Dmitriy,

        My apologies. I was using "svn move" command to change the package of StoreMetadata and didn't realize that this command doesn't work with patch command.

        Please get the latest patch (13). Here are the results of "test-patch" run:

             [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 12 new or modified tests.
             [exec] 
             [exec]     +1 javadoc.  The javadoc tool did not generate any warning messages.
             [exec] 
             [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
             [exec] 
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec] 
             [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
        
        
        Show
        Richard Ding added a comment - Dmitriy, My apologies. I was using "svn move" command to change the package of StoreMetadata and didn't realize that this command doesn't work with patch command. Please get the latest patch (13). Here are the results of "test-patch" run: [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 12 new or modified tests. [exec] [exec] +1 javadoc. The javadoc tool did not generate any warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
        Hide
        Dmitriy V. Ryaboy added a comment -

        Looks good.
        In piggybank/storage/JsonMetadata lin 188, the javadoc needs to be adjusted (LoadMetadata no longer in experimental)

        Show
        Dmitriy V. Ryaboy added a comment - Looks good. In piggybank/storage/JsonMetadata lin 188, the javadoc needs to be adjusted (LoadMetadata no longer in experimental)
        Hide
        Pradeep Kamath added a comment -

        Couple of comments on PIG-1090-13.patch.

        • The call to storeCleanup() should happen after the call to setUpContext() since the setUpContext() call changes the Configuration inside the Context and we should use this updated Configuration in storeCleanup()
        • In storeCleanup(), we could get StoreFunc instance once by calling store.getStoreFunc() and then use that instance later in the method. Also that instance can be used to check:
          if(storeFunc instanceof StoreMetadata) {
          }
          
        Show
        Pradeep Kamath added a comment - Couple of comments on PIG-1090 -13.patch. The call to storeCleanup() should happen after the call to setUpContext() since the setUpContext() call changes the Configuration inside the Context and we should use this updated Configuration in storeCleanup() In storeCleanup(), we could get StoreFunc instance once by calling store.getStoreFunc() and then use that instance later in the method. Also that instance can be used to check: if (storeFunc instanceof StoreMetadata) { }
        Hide
        Richard Ding added a comment -

        New patch (13) that addresses the comments by Dmitriy and Pradeep.

        Show
        Richard Ding added a comment - New patch (13) that addresses the comments by Dmitriy and Pradeep.
        Hide
        Richard Ding added a comment -

        Committed PIG-1090-13 patch.

        Show
        Richard Ding added a comment - Committed PIG-1090 -13 patch.
        Hide
        Richard Ding added a comment -

        Patch 14 reaffirms the contract between Pig and ResourceSchema: A bag schema in ResourceSchema has either an empty inner schema, or the inner schema is a tuple schema.

        The results of "test-patch" run:

             [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 does not increase the total number of javac compiler warnings.
             [exec] 
             [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
             [exec] 
             [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
        
        
        Show
        Richard Ding added a comment - Patch 14 reaffirms the contract between Pig and ResourceSchema: A bag schema in ResourceSchema has either an empty inner schema, or the inner schema is a tuple schema. The results of "test-patch" run: [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 does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
        Hide
        Pradeep Kamath added a comment -

        +1 for PIG-1090-14.patch

        Show
        Pradeep Kamath added a comment - +1 for PIG-1090 -14.patch
        Hide
        Richard Ding added a comment -

        Committed PIG-1090-14.patch.

        Show
        Richard Ding added a comment - Committed PIG-1090 -14.patch.
        Hide
        Daniel Dai added a comment -

        PIG-1090-15.patch is store side changes in regard to StoreMetadata.storeSchema

        Show
        Daniel Dai added a comment - PIG-1090 -15.patch is store side changes in regard to StoreMetadata.storeSchema
        Hide
        Pradeep Kamath added a comment -

        +1 for PIG-1090-15.patch, patch committed.

        Here are the results of running ant 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 does not increase the total number of javac compiler warnings.
        [exec]
        [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
        [exec]
        [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
        [exec]

        Show
        Pradeep Kamath added a comment - +1 for PIG-1090 -15.patch, patch committed. Here are the results of running ant 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 does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec]
        Hide
        Daniel Dai added a comment -

        Revert a change to LogUtil which does not suppose to.

        Show
        Daniel Dai added a comment - Revert a change to LogUtil which does not suppose to.
        Hide
        Pradeep Kamath added a comment -

        +1 for PIG-1090-16.patch, patch committed.

        Show
        Pradeep Kamath added a comment - +1 for PIG-1090 -16.patch, patch committed.
        Hide
        Pradeep Kamath added a comment -

        Attached patch to remove StoreConfig and CommittableStoreFunc and all references to them. StoreConfig was previously used to communicate location, schema and sort information for the POStore to the storeFunc through JobConf. This is no longer needed since storeFunc.setStoreLocation() provides the location and storeFunc.checkSchema() provides a ResourceSchema which has both the schema and sort information. CommittableStoreFunc was an additional interface needed to call a "commit" method on storeFunc. This is no longer needed since StoreFunc is now based on OutputFormat which will have an associated OutputCommitter.

        Show
        Pradeep Kamath added a comment - Attached patch to remove StoreConfig and CommittableStoreFunc and all references to them. StoreConfig was previously used to communicate location, schema and sort information for the POStore to the storeFunc through JobConf. This is no longer needed since storeFunc.setStoreLocation() provides the location and storeFunc.checkSchema() provides a ResourceSchema which has both the schema and sort information. CommittableStoreFunc was an additional interface needed to call a "commit" method on storeFunc. This is no longer needed since StoreFunc is now based on OutputFormat which will have an associated OutputCommitter.
        Hide
        Daniel Dai added a comment -

        +1 for PIG-1090-18.patch

        Show
        Daniel Dai added a comment - +1 for PIG-1090 -18.patch
        Hide
        Pradeep Kamath added a comment -

        Comment on PIG-1090-17.patch - instead of using funcSpec.getCtorArgs(), it would be better to use funcSpec.ToString() since that will include the funcspec class name and will be more complete. Otherwise changes are good.

        Show
        Pradeep Kamath added a comment - Comment on PIG-1090 -17.patch - instead of using funcSpec.getCtorArgs(), it would be better to use funcSpec.ToString() since that will include the funcspec class name and will be more complete. Otherwise changes are good.
        Hide
        Daniel Dai added a comment -

        Resubmit PIG-1090-17.patch to address Pradeep's comments.

        Show
        Daniel Dai added a comment - Resubmit PIG-1090 -17.patch to address Pradeep's comments.
        Hide
        Richard Ding added a comment -

        Patch 19 fixes a unit test in Piggybank.

        Show
        Richard Ding added a comment - Patch 19 fixes a unit test in Piggybank.
        Hide
        Pradeep Kamath added a comment -

        PIG-1090-17.patch committed

        Show
        Pradeep Kamath added a comment - PIG-1090 -17.patch committed
        Hide
        Daniel Dai added a comment -

        PIG-1090-19.patch committed.

        Show
        Daniel Dai added a comment - PIG-1090 -19.patch committed.
        Hide
        Pradeep Kamath added a comment -

        PIG-1090-18.patch committed.

        Show
        Pradeep Kamath added a comment - PIG-1090 -18.patch committed.
        Hide
        Daniel Dai added a comment -

        Fix one bug in MergeJoin when index has only one entry.

        Show
        Daniel Dai added a comment - Fix one bug in MergeJoin when index has only one entry.
        Hide
        Pradeep Kamath added a comment -

        Attached patch to handle calling storeSchema in StoreMetadata interface in local mode (currently there is a hadoop bug : https://issues.apache.org/jira/browse/MAPREDUCE-1447 which prevents the current code from making this call in local mode). The patch is a workaround till hadoop fixes the bug - in MapReduceLauncer, we explictly call this method for successful stores.

        Show
        Pradeep Kamath added a comment - Attached patch to handle calling storeSchema in StoreMetadata interface in local mode (currently there is a hadoop bug : https://issues.apache.org/jira/browse/MAPREDUCE-1447 which prevents the current code from making this call in local mode). The patch is a workaround till hadoop fixes the bug - in MapReduceLauncer, we explictly call this method for successful stores.
        Hide
        Pradeep Kamath added a comment -

        test-patch results for PIG-1090-21.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 does not increase the total number of javac compiler warnings.
        [exec]
        [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
        [exec]
        [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.

        Show
        Pradeep Kamath added a comment - test-patch results for PIG-1090 -21.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 does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
        Hide
        Richard Ding added a comment -

        +1 for PIG-1090-21.patch.

        Show
        Richard Ding added a comment - +1 for PIG-1090 -21.patch.
        Hide
        Daniel Dai added a comment -

        Two bug fix:
        1. Cycle in plan if load and store location are the same
        2. relToAbsPathForStoreLocation is not called using pig API directly not using Grunt.

        Show
        Daniel Dai added a comment - Two bug fix: 1. Cycle in plan if load and store location are the same 2. relToAbsPathForStoreLocation is not called using pig API directly not using Grunt.
        Hide
        Pradeep Kamath added a comment -

        +1 for PIG-1090-22.patch, patch committed.
        Closing this jira as resolved since all changes to accommodate the new load-store interfaces have now been checked in.

        Show
        Pradeep Kamath added a comment - +1 for PIG-1090 -22.patch, patch committed. Closing this jira as resolved since all changes to accommodate the new load-store interfaces have now been checked in.

          People

          • Assignee:
            Pradeep Kamath
            Reporter:
            Pradeep Kamath
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development