Pig
  1. Pig
  2. PIG-936

making dump and PigDump independent from Tuple.toString

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.3.0
    • Fix Version/s: 0.4.0
    • Component/s: None
    • Labels:
      None

      Description

      Since Tuple is an interface, a toString implementation can change from one tuple implementation to the next. This means that format of dump and PigDump will be different depending on the tuples processed. This could be quite confusing to the users.

      1. Pig_936.Patch
        14 kB
        Jeff Zhang
      2. Pig_936_3.Patch
        15 kB
        Daniel Dai
      3. Pig_936_2.Patch
        5 kB
        Daniel Dai

        Activity

        Hide
        Jeff Zhang added a comment -

        Extract the output format of Dump and PigDump into class TupleFormat

        Show
        Jeff Zhang added a comment - Extract the output format of Dump and PigDump into class TupleFormat
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12418087/Pig_936.Patch
        against trunk revision 806668.

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

        +1 tests included. The patch appears to include 3 new or modified tests.

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

        Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/1/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/12418087/Pig_936.Patch against trunk revision 806668. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/1/console This message is automatically generated.
        Hide
        Dmitriy V. Ryaboy added a comment -

        Patch makes sense.
        pig.data doesn't seem like the right package for this class – perhaps pig.tools ?

        Also please make sure to format your code in accordance with the style guidelines (http://java.sun.com/docs/codeconv/ ), and use 4 spaces – not tabs – for indentation.

        Show
        Dmitriy V. Ryaboy added a comment - Patch makes sense. pig.data doesn't seem like the right package for this class – perhaps pig.tools ? Also please make sure to format your code in accordance with the style guidelines ( http://java.sun.com/docs/codeconv/ ), and use 4 spaces – not tabs – for indentation.
        Hide
        Jeff Zhang added a comment -

        Dmitriy,
        Thank you for your review and suggestion.
        1. I add one class BagFormat, because Tuple maybe have DataBag as its field, And like Tuple, DataBag also has different implementation,so call toString() on DataBag is not enough.

        2. And I put the BagFormat and TupleFormat in package org.apache.pig.impl.util. I am not sure whether this is the right package I should put.

        Show
        Jeff Zhang added a comment - Dmitriy, Thank you for your review and suggestion. 1. I add one class BagFormat, because Tuple maybe have DataBag as its field, And like Tuple, DataBag also has different implementation,so call toString() on DataBag is not enough. 2. And I put the BagFormat and TupleFormat in package org.apache.pig.impl.util. I am not sure whether this is the right package I should put.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12418090/Pig_936.Patch
        against trunk revision 806668.

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

        +1 tests included. The patch appears to include 6 new or modified tests.

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

        Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/2/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/12418090/Pig_936.Patch against trunk revision 806668. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/2/console This message is automatically generated.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12418090/Pig_936.Patch
        against trunk revision 806668.

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

        +1 tests included. The patch appears to include 6 new or modified tests.

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

        Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/3/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/12418090/Pig_936.Patch against trunk revision 806668. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/3/console This message is automatically generated.
        Hide
        Jeff Zhang added a comment -

        Does anyone know why my patch failed ?
        The error message in build log is : (Stripping trailing CRs from patch.)
        I do not quite understand it. I developed this patch on windows, does it necessary for me to code in linux platform ?

        Show
        Jeff Zhang added a comment - Does anyone know why my patch failed ? The error message in build log is : (Stripping trailing CRs from patch.) I do not quite understand it. I developed this patch on windows, does it necessary for me to code in linux platform ?
        Hide
        Daniel Dai added a comment -

        Hi, Jeff,
        It is fine to make patch on Windows. "Stripping trailing CRs from patch" does fail the patch. The problem is PigDump.java, which is now surprisingly in Windows format (You can see the tailing ^M if you open in vi). If you convert PigDump.java into Unix format (by using dos2unix), then your patch can be applied. I attached the patch again. The only change is that it will convert PigDump.java into Unix as well, so it can be applied to trunk.

        I also reviewed the patch. It looks good to me. I am fine with the putting TupleFormat and BagFormat into package "org.apache.pig.impl.util". I will commit it shortly if no other comments. Thanks!

        Show
        Daniel Dai added a comment - Hi, Jeff, It is fine to make patch on Windows. "Stripping trailing CRs from patch" does fail the patch. The problem is PigDump.java, which is now surprisingly in Windows format (You can see the tailing ^M if you open in vi). If you convert PigDump.java into Unix format (by using dos2unix), then your patch can be applied. I attached the patch again. The only change is that it will convert PigDump.java into Unix as well, so it can be applied to trunk. I also reviewed the patch. It looks good to me. I am fine with the putting TupleFormat and BagFormat into package "org.apache.pig.impl.util". I will commit it shortly if no other comments. Thanks!
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12418420/Pig_936_2.Patch
        against trunk revision 810327.

        +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 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        -1 findbugs. The patch appears to cause Findbugs to fail.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/8/testReport/
        Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/8/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/12418420/Pig_936_2.Patch against trunk revision 810327. +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 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to cause Findbugs to fail. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/8/testReport/ Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/8/console This message is automatically generated.
        Hide
        Daniel Dai added a comment -

        Missing several files in the last patch. Resubmitting

        Show
        Daniel Dai added a comment - Missing several files in the last patch. Resubmitting
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12418422/Pig_936_3.Patch
        against trunk revision 810327.

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

        +1 tests included. The patch appears to include 6 new or modified tests.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 findbugs. The patch does not introduce any new Findbugs warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        -1 core tests. The patch failed core unit tests.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/9/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/9/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/9/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/12418422/Pig_936_3.Patch against trunk revision 810327. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/9/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/9/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: http://hudson.zones.apache.org/hudson/job/Pig-Patch-h7.grid.sp2.yahoo.net/9/console This message is automatically generated.
        Hide
        Daniel Dai added a comment -

        Unit test failure is not related to this patch.

        Show
        Daniel Dai added a comment - Unit test failure is not related to this patch.
        Hide
        Jeff Zhang added a comment -

        Daniel,

        Thank you for your help.

        Show
        Jeff Zhang added a comment - Daniel, Thank you for your help.
        Hide
        Daniel Dai added a comment -

        Patch committed. Thanks Jeff for contributing!

        Show
        Daniel Dai added a comment - Patch committed. Thanks Jeff for contributing!

          People

          • Assignee:
            Jeff Zhang
            Reporter:
            Olga Natkovich
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development