Pig
  1. Pig
  2. PIG-1618

Switch to new parser generator technology

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.8.0
    • Fix Version/s: 0.9.0
    • Component/s: None
    • Labels:
      None

      Description

      There are many bugs in Pig related to the parser, particularly to bad error messages. After review of Java CC we feel these will be difficult to address using that tool. Also, the .jjt files used by JavaCC are hard to understand and maintain.

      ANTLR is being reviewed as the most likely choice to move to, but other parsers will be reviewed as well.

      This JIRA will act as an umbrella issue for other parser issues.

      1. antlr-3.2.jar
        1.84 MB
        Xuefu Zhang
      2. javadoc.patch
        2 kB
        Thejas M Nair
      3. NewParser-1.patch
        37 kB
        Xuefu Zhang
      4. NewParser-10.patch
        50 kB
        Xuefu Zhang
      5. NewParser-11.patch
        61 kB
        Xuefu Zhang
      6. NewParser-12.patch
        12 kB
        Xuefu Zhang
      7. NewParser-13.2.patch
        35 kB
        Thejas M Nair
      8. NewParser-13.patch
        35 kB
        Xuefu Zhang
      9. NewParser-14.patch
        16 kB
        Xuefu Zhang
      10. NewParser-15.patch
        44 kB
        Xuefu Zhang
      11. NewParser-18.patch
        886 kB
        Xuefu Zhang
      12. NewParser-19.3.patch
        965 kB
        Thejas M Nair
      13. NewParser-19.patch
        881 kB
        Xuefu Zhang
      14. NewParser-2.patch
        38 kB
        Xuefu Zhang
      15. NewParser-21.patch
        32 kB
        Xuefu Zhang
      16. NewParser-22.patch
        29 kB
        Xuefu Zhang
      17. NewParser-23.2.patch
        194 kB
        Thejas M Nair
      18. NewParser-23.patch
        185 kB
        Thejas M Nair
      19. NewParser-24.patch
        20 kB
        Xuefu Zhang
      20. NewParser-25.patch
        0.8 kB
        Xuefu Zhang
      21. NewParser-3.patch
        16 kB
        Xuefu Zhang
      22. NewParser-3.patch
        15 kB
        Xuefu Zhang
      23. NewParser-4.patch
        17 kB
        Xuefu Zhang
      24. NewParser-5.patch
        21 kB
        Xuefu Zhang
      25. NewParser-6.patch
        35 kB
        Xuefu Zhang
      26. NewParser-7.patch
        34 kB
        Xuefu Zhang
      27. NewParser-8.patches
        42 kB
        Xuefu Zhang
      28. NewParser-9.patch
        35 kB
        Xuefu Zhang

        Issue Links

          Activity

          Hide
          Xuefu Zhang added a comment -

          First patch to address the issue. With this patch, the generated parse can parse pig script and generate a flat AST.

          The patch contains antlr lib, but the lib may not be applied when it's patched. (for me, it didn't.) In that case, so the attached lib, antlr-3.2.jar can be put under pig lib directory.

          Show
          Xuefu Zhang added a comment - First patch to address the issue. With this patch, the generated parse can parse pig script and generate a flat AST. The patch contains antlr lib, but the lib may not be applied when it's patched. (for me, it didn't.) In that case, so the attached lib, antlr-3.2.jar can be put under pig lib directory.
          Hide
          Xuefu Zhang added a comment -

          Updated to remove some code duplication.

          Show
          Xuefu Zhang added a comment - Updated to remove some code duplication.
          Hide
          Giridharan Kesavan added a comment -

          could we use ivy for resolving/retrieving antlr-3.2.jar ?

          Show
          Giridharan Kesavan added a comment - could we use ivy for resolving/retrieving antlr-3.2.jar ?
          Hide
          Xuefu Zhang added a comment -

          Tried but the jar available in the public repos seems having some issue. However, this will be re-addressed at a later time.

          Show
          Xuefu Zhang added a comment - Tried but the jar available in the public repos seems having some issue. However, this will be re-addressed at a later time.
          Hide
          Thejas M Nair added a comment -

          +1
          Looks great, now one can understand the grammar from the parser rules !

          Show
          Thejas M Nair added a comment - +1 Looks great, now one can understand the grammar from the parser rules !
          Hide
          Thejas M Nair added a comment -

          NewParser-2.patch and antlr-3.2.jar committed to trunk .

          Show
          Thejas M Nair added a comment - NewParser-2.patch and antlr-3.2.jar committed to trunk .
          Hide
          Thejas M Nair added a comment -

          Earlier patch introduced javadoc warnings. This patch (javadoc.patch) fixes it. It passes test-patch. Running unit tests.

          Show
          Thejas M Nair added a comment - Earlier patch introduced javadoc warnings. This patch (javadoc.patch) fixes it. It passes test-patch. Running unit tests.
          Hide
          Thejas M Nair added a comment -

          Unit tests have passed with javadoc.patch . The javadoc warnings were caused by absence of antlr jar in javadoc classpath -

               [exec]   [javadoc] Constructing Javadoc information...
               [exec]   [javadoc] /tmp/patchtestthejas/trunk/src/org/apache/pig/parser/QueryParserFileStream.java:23: package org.antlr.runtime does not exist
               [exec]   [javadoc] import org.antlr.runtime.ANTLRFileStream;
               [exec]   [javadoc]                         ^
               [exec]   [javadoc] /tmp/patchtestthejas/trunk/src/org/apache/pig/parser/QueryParserFileStream.java:29: cannot find symbol
               [exec]   [javadoc] symbol: class ANTLRFileStream
               [exec]   [javadoc] public class QueryParserFileStream extends ANTLRFileStream {
               [exec]   [javadoc]                                            ^
          
          
          Show
          Thejas M Nair added a comment - Unit tests have passed with javadoc.patch . The javadoc warnings were caused by absence of antlr jar in javadoc classpath - [exec] [javadoc] Constructing Javadoc information... [exec] [javadoc] /tmp/patchtestthejas/trunk/src/org/apache/pig/parser/QueryParserFileStream.java:23: package org.antlr.runtime does not exist [exec] [javadoc] import org.antlr.runtime.ANTLRFileStream; [exec] [javadoc] ^ [exec] [javadoc] /tmp/patchtestthejas/trunk/src/org/apache/pig/parser/QueryParserFileStream.java:29: cannot find symbol [exec] [javadoc] symbol: class ANTLRFileStream [exec] [javadoc] public class QueryParserFileStream extends ANTLRFileStream { [exec] [javadoc] ^
          Hide
          Xuefu Zhang added a comment -

          +1
          Patch looks good.

          Show
          Xuefu Zhang added a comment - +1 Patch looks good.
          Hide
          Thejas M Nair added a comment -

          javadoc.patch committed to trunk.

          Show
          Thejas M Nair added a comment - javadoc.patch committed to trunk.
          Hide
          Xuefu Zhang added a comment -

          Test-Patch run result is shown below, and the javadoc warning was due to the instability of the sun java site.

          [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 9 new or modified tests.
          [exec]
          [exec] -1 javadoc. The javadoc tool appears to have generated 1 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
          Xuefu Zhang added a comment - Test-Patch run result is shown below, and the javadoc warning was due to the instability of the sun java site. [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 9 new or modified tests. [exec] [exec] -1 javadoc. The javadoc tool appears to have generated 1 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
          Thejas M Nair added a comment -

          +1

          Show
          Thejas M Nair added a comment - +1
          Hide
          Thejas M Nair added a comment -

          NewParser-4.patch committed to trunk .

          Show
          Thejas M Nair added a comment - NewParser-4.patch committed to trunk .
          Hide
          Xuefu Zhang added a comment -

          Tree Parser, DefaultDataTypeInserter

          Show
          Xuefu Zhang added a comment - Tree Parser, DefaultDataTypeInserter
          Hide
          Xuefu Zhang added a comment -

          For NewParser-5.patch, the following is the test-patch run result. Note that additional javac warnings come from the generated code.

          [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 10 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 741 javac compiler warnings (more than the trunk's current 730 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
          Xuefu Zhang added a comment - For NewParser-5.patch, the following is the test-patch run result. Note that additional javac warnings come from the generated code. [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 10 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 741 javac compiler warnings (more than the trunk's current 730 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
          Alan Gates added a comment -

          There was a question on whether the default type inserter should be done as a visitor on the AST or on the logical plan. We want to limit the number of visitors we create on the AST since they require a definition of the grammar in each visitor. But I think the default type inserter makes sense on the AST. Moving this to the logical plan would most likely involve putting information in the logical plan that shouldn't be there (like whether there was an AS defined on this clause). Since the AST already has this information it makes more sense to do that processing there.

          Show
          Alan Gates added a comment - There was a question on whether the default type inserter should be done as a visitor on the AST or on the logical plan. We want to limit the number of visitors we create on the AST since they require a definition of the grammar in each visitor. But I think the default type inserter makes sense on the AST. Moving this to the logical plan would most likely involve putting information in the logical plan that shouldn't be there (like whether there was an AS defined on this clause). Since the AST already has this information it makes more sense to do that processing there.
          Hide
          Thejas M Nair added a comment -

          +1
          NewParser-5.patch committed to trunk.

          Show
          Thejas M Nair added a comment - +1 NewParser-5.patch committed to trunk.
          Hide
          Xuefu Zhang added a comment -

          For NewParser-6.patch, the following is the test-patch run result. Note that additional javac warnings come from the generated code.

          [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 8 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 742 javac compiler warnings (more than the trunk's current 741 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
          Xuefu Zhang added a comment - For NewParser-6.patch, the following is the test-patch run result. Note that additional javac warnings come from the generated code. [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 8 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 742 javac compiler warnings (more than the trunk's current 741 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
          Xuefu Zhang added a comment -

          Patch NewParser-6.patch added schema alias name validation. The code was refactored to combine two visitors into one.

          Show
          Xuefu Zhang added a comment - Patch NewParser-6.patch added schema alias name validation. The code was refactored to combine two visitors into one.
          Hide
          Thejas M Nair added a comment -

          Review comments on NewParser-6.patch -

          • AstValidator.g - HashSet would be a better choice than HashMap for keeping track of unique schema aliases
          • ParserTestingUtils.java - insertDefaultDataType and checkSchemaAlias do the same thing. We can consolidate them together into getValidatedAST().
          • TestAstValidator.java- insertDefaultDataType is also not used currently, i think it will be good to call the consolidated function (getValidatedAST() ? ) from testDefaultDataTypeInsertion
          • TestAstValidator.java - can we also check for line and column number in the error message ?
          Show
          Thejas M Nair added a comment - Review comments on NewParser-6.patch - AstValidator.g - HashSet would be a better choice than HashMap for keeping track of unique schema aliases ParserTestingUtils.java - insertDefaultDataType and checkSchemaAlias do the same thing. We can consolidate them together into getValidatedAST(). TestAstValidator.java- insertDefaultDataType is also not used currently, i think it will be good to call the consolidated function (getValidatedAST() ? ) from testDefaultDataTypeInsertion TestAstValidator.java - can we also check for line and column number in the error message ?
          Hide
          Xuefu Zhang added a comment -

          Updated the patch (#6) based on some of the feedback.

          Line number/column number is not asserted because of Antlr's recovery feature. The numbers are shown in the error messages, but it's not available without turning the recovery off, which requires some additional work. Will provide the capability later.

          Show
          Xuefu Zhang added a comment - Updated the patch (#6) based on some of the feedback. Line number/column number is not asserted because of Antlr's recovery feature. The numbers are shown in the error messages, but it's not available without turning the recovery off, which requires some additional work. Will provide the capability later.
          Hide
          Thejas M Nair added a comment -

          +1 for NewParser-7.patch
          Patch committed to trunk.

          Show
          Thejas M Nair added a comment - +1 for NewParser-7.patch Patch committed to trunk.
          Hide
          Xuefu Zhang added a comment -

          Patch includes first portion of logical plan generation. This is not complete and subject to subsequent changes. We need to get it in to make the whole process incremental.

          The following is the test-patch run result. The additional javac warnings are due to generated code.

          [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 generated 924 javac compiler warnings (more than the trunk's current 742 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
          Xuefu Zhang added a comment - Patch includes first portion of logical plan generation. This is not complete and subject to subsequent changes. We need to get it in to make the whole process incremental. The following is the test-patch run result. The additional javac warnings are due to generated code. [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 generated 924 javac compiler warnings (more than the trunk's current 742 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
          Thejas M Nair added a comment -

          +1 for NewParser-8.patches .
          I also ran the unit tests, all passed.
          Patch committed to trunk.

          Show
          Thejas M Nair added a comment - +1 for NewParser-8.patches . I also ran the unit tests, all passed. Patch committed to trunk.
          Hide
          Xuefu Zhang added a comment -

          Patch NewParser-9.patch includes second portion of logical plan generation. Still, work is not complete and subject to subsequent changes.

          The following is the test-patch run result. The additional javac warnings are due to generated code. Ant test passed.
          [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 943 javac compiler warnings (more than the trunk's current 924 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
          Xuefu Zhang added a comment - Patch NewParser-9.patch includes second portion of logical plan generation. Still, work is not complete and subject to subsequent changes. The following is the test-patch run result. The additional javac warnings are due to generated code. Ant test passed. [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 943 javac compiler warnings (more than the trunk's current 924 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
          Thejas M Nair added a comment -

          +1 for NewParser-9.patch
          Patch committed to trunk.

          Show
          Thejas M Nair added a comment - +1 for NewParser-9.patch Patch committed to trunk.
          Hide
          Xuefu Zhang added a comment -

          NewParser-10.patch is the 3rd patch of logical plan generation.

          Show
          Xuefu Zhang added a comment - NewParser-10.patch is the 3rd patch of logical plan generation.
          Hide
          Xuefu Zhang added a comment -

          NewParser-10.patch is the 3rd patch for logical plan generation.

          Show
          Xuefu Zhang added a comment - NewParser-10.patch is the 3rd patch for logical plan generation.
          Hide
          Xuefu Zhang added a comment -

          Test patch run result is below. New javacc warnings are from the generated parser code.
          Nightly test is in progress.

          [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 954 javac compiler warnings (more than the trunk's current 943 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
          Xuefu Zhang added a comment - Test patch run result is below. New javacc warnings are from the generated parser code. Nightly test is in progress. [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 954 javac compiler warnings (more than the trunk's current 943 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
          Xuefu Zhang added a comment -

          Nightly test (ant test) passed for patch NewParser-10.patch.

          Show
          Xuefu Zhang added a comment - Nightly test (ant test) passed for patch NewParser-10.patch.
          Hide
          Thejas M Nair added a comment -

          +1 for NewParser-10.patch
          Patch committed to trunk.

          Show
          Thejas M Nair added a comment - +1 for NewParser-10.patch Patch committed to trunk.
          Hide
          Xuefu Zhang added a comment -

          Further patch for logical plan generation, mostly about foreach inner plan.

          Show
          Xuefu Zhang added a comment - Further patch for logical plan generation, mostly about foreach inner plan.
          Hide
          Xuefu Zhang added a comment -

          Further work for logical plan generation. Largely about foreach inner plan generation.

          Show
          Xuefu Zhang added a comment - Further work for logical plan generation. Largely about foreach inner plan generation.
          Hide
          Xuefu Zhang added a comment -

          Test-pathc run result. Additional javac warnings coming from the generated parser code. Release audit warnings seemed coming from new methods added to the existing classes. Both kinds of warnings can be ignored.

          [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 generated 969 javac compiler warnings (more than the trunk's current 953 warnings).
          [exec]
          [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
          [exec]
          [exec] -1 release audit. The applied patch generated 482 release audit warnings (more than the trunk's current 478 warnings).

          Show
          Xuefu Zhang added a comment - Test-pathc run result. Additional javac warnings coming from the generated parser code. Release audit warnings seemed coming from new methods added to the existing classes. Both kinds of warnings can be ignored. [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 generated 969 javac compiler warnings (more than the trunk's current 953 warnings). [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] -1 release audit. The applied patch generated 482 release audit warnings (more than the trunk's current 478 warnings).
          Hide
          Xuefu Zhang added a comment -

          "Ant test" passed for patch NewParser-11.patch.

          Show
          Xuefu Zhang added a comment - "Ant test" passed for patch NewParser-11.patch.
          Hide
          Thejas M Nair added a comment -

          NewParser-11.patch committed to trunk with some additional comments to describe LogicalPlanBuilder.processExpressionPlan(..)

          Show
          Thejas M Nair added a comment - NewParser-11.patch committed to trunk with some additional comments to describe LogicalPlanBuilder.processExpressionPlan(..)
          Hide
          Xuefu Zhang added a comment -

          This patch is about schema specification cleanup (as clause in pig statements). It also includes various fixes including union onschema support (partial).

          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 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.

          Show
          Xuefu Zhang added a comment - This patch is about schema specification cleanup (as clause in pig statements). It also includes various fixes including union onschema support (partial). 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 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.
          Hide
          Thejas M Nair added a comment -

          NewParser-12.patch review comment :
          For bag schema specification, the parser is ignoring the tuple name. ie in case of bag schema such as "a :

          { t : (x : int)}

          ", the tuple name "t" is being ignored.
          The name of the tuple in the bag is not useful because it cannot be accessed with that name, so it makes sense to make it optional. But if we plan to ignore the name (ie, describe command will not show the tuple name), i think we should deprecate that syntax and warn the user.

          ie something like -

          grunt> l = load 'file' as (a : { t : (a : int)});
          WARN: specifying name of the tuple  in a bag is deprecated, the tuple name will be ignored. Tuple inside the bag "a" has been named "t".
          

          This change can be part of the next patch.
          Committed NewParser-12.patch to trunk.

          Show
          Thejas M Nair added a comment - NewParser-12.patch review comment : For bag schema specification, the parser is ignoring the tuple name. ie in case of bag schema such as "a : { t : (x : int)} ", the tuple name "t" is being ignored. The name of the tuple in the bag is not useful because it cannot be accessed with that name, so it makes sense to make it optional. But if we plan to ignore the name (ie, describe command will not show the tuple name), i think we should deprecate that syntax and warn the user. ie something like - grunt> l = load 'file' as (a : { t : (a : int )}); WARN: specifying name of the tuple in a bag is deprecated, the tuple name will be ignored. Tuple inside the bag "a" has been named "t" . This change can be part of the next patch. Committed NewParser-12.patch to trunk.
          Hide
          Xuefu Zhang added a comment -

          Patch includes:
          1. Union OnSchema migration from the old plan
          2. Repackaging for new logical plan visitors.

          TestPatch run: ( Release audit warnings due to new folder/file)

          [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 generated 484 release audit warnings (more than the trunk's current 482 warnings).

          Ant test passed.

          Show
          Xuefu Zhang added a comment - Patch includes: 1. Union OnSchema migration from the old plan 2. Repackaging for new logical plan visitors. TestPatch run: ( Release audit warnings due to new folder/file) [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 generated 484 release audit warnings (more than the trunk's current 482 warnings). Ant test passed.
          Hide
          Thejas M Nair added a comment -

          Made a minor change to NewParser-13.patch (see NewParser-13.2.patch), to move package org.apache.pig.newplan.visitor to src.org.apache.pig.newplan.logical.visitor .
          Committed patch to trunk.

          Show
          Thejas M Nair added a comment - Made a minor change to NewParser-13.patch (see NewParser-13.2.patch), to move package org.apache.pig.newplan.visitor to src.org.apache.pig.newplan.logical.visitor . Committed patch to trunk.
          Hide
          Xuefu Zhang added a comment -

          A visitor for new logical plan. It does some schema validations, but it mainly converts column alias in projection and de-reference to column index. However, this patch doesn't process scalar projection yet.

          Test-Patch run result:
          [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.

          Ant test passes except a zebra test failure which doesn't seem to be related.

          Show
          Xuefu Zhang added a comment - A visitor for new logical plan. It does some schema validations, but it mainly converts column alias in projection and de-reference to column index. However, this patch doesn't process scalar projection yet. Test-Patch run result: [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. Ant test passes except a zebra test failure which doesn't seem to be related.
          Hide
          Thejas M Nair added a comment -

          +1
          NewParser-14.patch committed to trunk.

          Show
          Thejas M Nair added a comment - +1 NewParser-14.patch committed to trunk.
          Hide
          Xuefu Zhang added a comment -

          Includes:
          1. Scalar support
          2. Minor parser grammar fixes
          3. Better schema validation and more test case.

          Show
          Xuefu Zhang added a comment - Includes: 1. Scalar support 2. Minor parser grammar fixes 3. Better schema validation and more test case.
          Hide
          Xuefu Zhang added a comment -

          Test-patch run result for patch NewParser-15.patch: (release warning is due to new files added.)
          [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 generated 499 release audit warnings (more than the trunk's current 496 warnings).

          Ant test passed.

          Show
          Xuefu Zhang added a comment - Test-patch run result for patch NewParser-15.patch: (release warning is due to new files added.) [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 generated 499 release audit warnings (more than the trunk's current 496 warnings). Ant test passed.
          Hide
          Thejas M Nair added a comment -

          +1 NewParser-15.patch
          Patch committed to trunk.

          Some comments about the patch which can be addressed in next patch -

          • in LogicalPlanGenerator.g, the exception InvalidScalarProjectionException should include a useful error message as well.
            +                     if( schema != null && pos >= schema.size() ) {
            +                         throw new InvalidScalarProjectionException( input, scalarExpr );
            +                     }
            
            should be something like -
            +                     if( schema != null && pos >= schema.size() ) {
            +                         throw new InvalidScalarProjectionException( input, scalarExpr, "Position " + pos + " is greater that schema size :" + schema.size() );
            +                     }
            
          • ScalarVisitor class description needs to be corrected.
          Show
          Thejas M Nair added a comment - +1 NewParser-15.patch Patch committed to trunk. Some comments about the patch which can be addressed in next patch - in LogicalPlanGenerator.g, the exception InvalidScalarProjectionException should include a useful error message as well. + if ( schema != null && pos >= schema.size() ) { + throw new InvalidScalarProjectionException( input, scalarExpr ); + } should be something like - + if ( schema != null && pos >= schema.size() ) { + throw new InvalidScalarProjectionException( input, scalarExpr, "Position " + pos + " is greater that schema size :" + schema.size() ); + } ScalarVisitor class description needs to be corrected.
          Hide
          Daniel Dai added a comment -

          Review for NewParser-18.patch: https://reviews.apache.org/r/459/

          Show
          Daniel Dai added a comment - Review for NewParser-18.patch: https://reviews.apache.org/r/459/
          Hide
          Thejas M Nair added a comment -

          NewParser-19.3.patch has fixes for findbugs, release audit warnings in NewParser-19.patch.

          Show
          Thejas M Nair added a comment - NewParser-19.3.patch has fixes for findbugs, release audit warnings in NewParser-19.patch.
          Hide
          Thejas M Nair added a comment -

          Review comments in https://reviews.apache.org/r/459/ for NewParser-18.patch/NewParser-19.*.patch will be addressed in another patch in this jira.
          Committed NewParser-19.3.patch to trunk.

          Show
          Thejas M Nair added a comment - Review comments in https://reviews.apache.org/r/459/ for NewParser-18.patch/NewParser-19.*.patch will be addressed in another patch in this jira. Committed NewParser-19.3.patch to trunk.
          Hide
          Thejas M Nair added a comment -

          The new parser changes also fixes one behavior seen in earlier versions including 0.8 .
          With the changes in trunk - flatten of bag with null schema will result in a null schema.

          For example,
          in 0.8 -

          grunt> describe g;
          g: {group: bytearray,a: {null}}
          grunt> f = foreach g generate $0 , flatten(a);
          grunt> describe f;
          f: {group: bytearray,bytearray}
          

          in trunk with new parser changes -

          grunt> describe g;
          g: {group: bytearray,a: {(null)}}
          grunt> f = foreach g generate $0 , flatten(a);
          grunt> describe f;
          Schema for f unknown.
          
          Show
          Thejas M Nair added a comment - The new parser changes also fixes one behavior seen in earlier versions including 0.8 . With the changes in trunk - flatten of bag with null schema will result in a null schema. For example, in 0.8 - grunt> describe g; g: {group: bytearray,a: { null }} grunt> f = foreach g generate $0 , flatten(a); grunt> describe f; f: {group: bytearray,bytearray} in trunk with new parser changes - grunt> describe g; g: {group: bytearray,a: {( null )}} grunt> f = foreach g generate $0 , flatten(a); grunt> describe f; Schema for f unknown.
          Hide
          Olga Natkovich added a comment -

          is this only changes describe output or does it have other non-backward comptible changes?

          Show
          Olga Natkovich added a comment - is this only changes describe output or does it have other non-backward comptible changes?
          Hide
          Thejas M Nair added a comment -

          is this only changes describe output or does it have other non-backward comptible changes?

          It changes the schema of the statement, so any statements that relies on a non null schema of foreach statement will not work any more.
          But I think the new behavior is correct.

          For example -

          grunt> describe g;
          g: {group: bytearray,a: {(null)}}
          grunt> f = foreach g generate $0 , flatten(a);
          grunt> f2 = foreach f generate group; -- this would give an error - Projected field [group] does not exist in schema: null.
          

          It also affects lineage tracing, in case of co-group where inputs don't have schema.

          a = load 'a' using PigStorage('a') ;
          b = load 'a' using PigStorage('b') ;
          c = cogroup a by $0, b by $0 ;
          d = foreach c generate group, flatten(a), flatten(b)  ;
          e = foreach d generate $1 + 1, $2 + 1  ;
          -- in 0.8 the load func spec of a would have been associated with $1, and that of b with $2 .
          

          This also means that some valid statements that would not work with 0.8 will now work -

          a = load 'a' using PigStorage('a') ;
          b = group a by $0;
          c = foreach b generate group, flatten(b);
          d = foreach c generate $2; 
          -- in 0.8 this would have given an error - Error during parsing. Out of bound access. Trying to access non-existent column: 2. Schema {group: bytearray,bytearray} has 2 column(s).
          -- in trunk this will work as it should
          
          
          Show
          Thejas M Nair added a comment - is this only changes describe output or does it have other non-backward comptible changes? It changes the schema of the statement, so any statements that relies on a non null schema of foreach statement will not work any more. But I think the new behavior is correct. For example - grunt> describe g; g: {group: bytearray,a: {( null )}} grunt> f = foreach g generate $0 , flatten(a); grunt> f2 = foreach f generate group; -- this would give an error - Projected field [group] does not exist in schema: null . It also affects lineage tracing, in case of co-group where inputs don't have schema. a = load 'a' using PigStorage('a') ; b = load 'a' using PigStorage('b') ; c = cogroup a by $0, b by $0 ; d = foreach c generate group, flatten(a), flatten(b) ; e = foreach d generate $1 + 1, $2 + 1 ; -- in 0.8 the load func spec of a would have been associated with $1, and that of b with $2 . This also means that some valid statements that would not work with 0.8 will now work - a = load 'a' using PigStorage('a') ; b = group a by $0; c = foreach b generate group, flatten(b); d = foreach c generate $2; -- in 0.8 this would have given an error - Error during parsing. Out of bound access. Trying to access non-existent column: 2. Schema {group: bytearray,bytearray} has 2 column(s). -- in trunk this will work as it should
          Hide
          Xuefu Zhang added a comment -

          I think the behavior change was largely due to the way we interpret the schema, probably having nothing to do with parser.

          It's basically a question of how we treat a schema partially known (unknown). It doesn't seem easy to correctly address the issue. However, treating a schema partially known as unknown seems fine: user can always use $0 instead of "group" to access the known field.

          On the other hand, treating flattan( a ) as one field of byte array seems misleading.

          Show
          Xuefu Zhang added a comment - I think the behavior change was largely due to the way we interpret the schema, probably having nothing to do with parser. It's basically a question of how we treat a schema partially known (unknown). It doesn't seem easy to correctly address the issue. However, treating a schema partially known as unknown seems fine: user can always use $0 instead of "group" to access the known field. On the other hand, treating flattan( a ) as one field of byte array seems misleading.
          Hide
          Xuefu Zhang added a comment -

          For NewParser-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 18 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.

          Nightly and unit tests all passed.

          Show
          Xuefu Zhang added a comment - For NewParser-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 18 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. Nightly and unit tests all passed.
          Hide
          Thejas M Nair added a comment -

          +1 for NewParser-22.patch.
          Patch committed to trunk.

          Show
          Thejas M Nair added a comment - +1 for NewParser-22.patch. Patch committed to trunk.
          Hide
          Thejas M Nair added a comment -

          NewParser-23.patch

          • Re-enables test cases in TestSchema,TestTypeCheckingValidatorNewLP
          • Moves ProjectStarExpander invocation to LogicalPlanBuilder, so that it happens before relation-as-scalar determination.
          • address review comments in BinCondExpression, LogicalSchema,Util
          • Fixes issues in typechecking/lineage tracing.

          test-patch has been successful. unit tests are still running.
          [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
          Thejas M Nair added a comment - NewParser-23.patch Re-enables test cases in TestSchema,TestTypeCheckingValidatorNewLP Moves ProjectStarExpander invocation to LogicalPlanBuilder, so that it happens before relation-as-scalar determination. address review comments in BinCondExpression, LogicalSchema,Util Fixes issues in typechecking/lineage tracing. test-patch has been successful. unit tests are still running. [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
          Xuefu Zhang added a comment -

          Add SchemaAliasVisitor
          Improve new parser error message and handling.

          Unit test and end2end test passed.

          test-patch run results: (javacc warning are caused by generated code)

          [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 generated 863 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 does not increase the total number of release audit warnings.

          Show
          Xuefu Zhang added a comment - Add SchemaAliasVisitor Improve new parser error message and handling. Unit test and end2end test passed. test-patch run results: (javacc warning are caused by generated code) [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 generated 863 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 does not increase the total number of release audit warnings.
          Hide
          Thejas M Nair added a comment -

          Review of NewParser-24.patch -
          in SchemaAliasVisitor.validate() the check could be done in single loop if the alias is stored in a HashSet and its presence is checked. That will reduce the complexity.
          I will make the change in a new patch.
          NewParser-23.patch has unit test failure, I will submit new patch with fix, which will include this change.

          Show
          Thejas M Nair added a comment - Review of NewParser-24.patch - in SchemaAliasVisitor.validate() the check could be done in single loop if the alias is stored in a HashSet and its presence is checked. That will reduce the complexity. I will make the change in a new patch. NewParser-23.patch has unit test failure, I will submit new patch with fix, which will include this change.
          Hide
          Thejas M Nair added a comment -

          NewParser-24.patch committed to trunk.

          Show
          Thejas M Nair added a comment - NewParser-24.patch committed to trunk.
          Hide
          Thejas M Nair added a comment -

          NewParser-23.2.patch - NewParser-23.patch with fixes for unit tests (support for project-star with alias).

          Show
          Thejas M Nair added a comment - NewParser-23.2.patch - NewParser-23.patch with fixes for unit tests (support for project-star with alias).
          Hide
          Mridul Muralidharan added a comment -

          Not sure what the scope of this JIRA is, but it will be a good idea to get rid of the pre-processor and integrate that into the parser.
          This will lead to consistent error messages (so no need to muck around with -debug, etc) while allowing for easier integration of pig in embedded mode.

          Regards,
          Mridul

          Show
          Mridul Muralidharan added a comment - Not sure what the scope of this JIRA is, but it will be a good idea to get rid of the pre-processor and integrate that into the parser. This will lead to consistent error messages (so no need to muck around with -debug, etc) while allowing for easier integration of pig in embedded mode. Regards, Mridul
          Hide
          Daniel Dai added a comment -

          Review comments for typechecker, lineage, projectStarExpander:
          Patch looks very good. Only two minor comments:
          1. We need to print out CompilationMessageCollector warnings in PigServer
          2. Seems we does not handle the case "foreach generate *, *". It is not a major use case, but should be addressed for completeness.

          Show
          Daniel Dai added a comment - Review comments for typechecker, lineage, projectStarExpander: Patch looks very good. Only two minor comments: 1. We need to print out CompilationMessageCollector warnings in PigServer 2. Seems we does not handle the case "foreach generate *, *". It is not a major use case, but should be addressed for completeness.
          Hide
          Thejas M Nair added a comment -

          1. We need to print out CompilationMessageCollector warnings in PigServer

          I will address this in a new patch.

          2. Seems we does not handle the case "foreach generate *, *". It is not a major use case, but should be addressed for completeness.

          Created PIG-1897 to address this.

          Show
          Thejas M Nair added a comment - 1. We need to print out CompilationMessageCollector warnings in PigServer I will address this in a new patch. 2. Seems we does not handle the case "foreach generate *, *". It is not a major use case, but should be addressed for completeness. Created PIG-1897 to address this.
          Hide
          Thejas M Nair added a comment -

          Unit test and test-patch succeeded with NewParser-23.2.patch . Patch committed to trunk.

          Show
          Thejas M Nair added a comment - Unit test and test-patch succeeded with NewParser-23.2.patch . Patch committed to trunk.
          Hide
          Alex Rovner added a comment -

          As of now when I import pig-trunk into eclipse I get the following error:
          Project 'pig-trunk' is missing required library: 'lib/antlr-3.2.jar' pig-trunk Build path Build Path Problem

          From what I understand it has to do with PIG project switching over to antlr but no dependency has been define in ivy?

          Show
          Alex Rovner added a comment - As of now when I import pig-trunk into eclipse I get the following error: Project 'pig-trunk' is missing required library: 'lib/antlr-3.2.jar' pig-trunk Build path Build Path Problem From what I understand it has to do with PIG project switching over to antlr but no dependency has been define in ivy?
          Hide
          Xuefu Zhang added a comment -

          Yes, Pig now requires antlr jars and the dependency is set on ivy. Make sure that you're on the latest trunk. If you go to the top-level directory and do "ant jar", it should get all the required jars thru ivy. Dependency on "lib/antlr-3.2.jar" used to be there, but now these jars are pulled by ivy.

          I'm not sure if this is the best place for this discussion. If the problem remains, you can send an email on pig user list to get help there.

          Show
          Xuefu Zhang added a comment - Yes, Pig now requires antlr jars and the dependency is set on ivy. Make sure that you're on the latest trunk. If you go to the top-level directory and do "ant jar", it should get all the required jars thru ivy. Dependency on "lib/antlr-3.2.jar" used to be there, but now these jars are pulled by ivy. I'm not sure if this is the best place for this discussion. If the problem remains, you can send an email on pig user list to get help there.
          Hide
          Alex Rovner added a comment -

          I am using latest trunk version: http://svn.apache.org/repos/asf/pig/trunk

          After further digging it seems that this issue is there because of .eclipse.templates/.classpath

          This file has an entry to antlr thats pointing to the wrong path: <classpathentry exported="true" kind="lib" path="lib/antlr-3.2.jar"/>
          I believe correct entry should be: <classpathentry exported="true" kind="lib" path="build/ivy/lib/Pig/antlr-3.2.jar"/>

          Another suspect is that antlr dependency in ivy.xml is commented out.

          I will try to verify both of the suspects right now and post a patch if successful.

          Show
          Alex Rovner added a comment - I am using latest trunk version: http://svn.apache.org/repos/asf/pig/trunk After further digging it seems that this issue is there because of .eclipse.templates/.classpath This file has an entry to antlr thats pointing to the wrong path: <classpathentry exported="true" kind="lib" path="lib/antlr-3.2.jar"/> I believe correct entry should be: <classpathentry exported="true" kind="lib" path="build/ivy/lib/Pig/antlr-3.2.jar"/> Another suspect is that antlr dependency in ivy.xml is commented out. I will try to verify both of the suspects right now and post a patch if successful.
          Hide
          Xuefu Zhang added a comment -

          Fix an issue where parallel should be set to 1 when pig is in local mode. Minor fix, so no test case is provided.

          Show
          Xuefu Zhang added a comment - Fix an issue where parallel should be set to 1 when pig is in local mode. Minor fix, so no test case is provided.
          Hide
          Xuefu Zhang added a comment -

          Run test locally, and it passed. (for NewParser-25.patch.

          Show
          Xuefu Zhang added a comment - Run test locally, and it passed. (for NewParser-25.patch.
          Hide
          Daniel Dai added a comment -

          +1 for NewParser-25.patch. NewParser-25.patch committed to trunk.

          Show
          Daniel Dai added a comment - +1 for NewParser-25.patch. NewParser-25.patch committed to trunk.
          Hide
          Olga Natkovich added a comment -

          Major work on this is done. Lets track bugs that we find in separate tickets

          Show
          Olga Natkovich added a comment - Major work on this is done. Lets track bugs that we find in separate tickets

            People

            • Assignee:
              Xuefu Zhang
              Reporter:
              Alan Gates
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development