Pig
  1. Pig
  2. PIG-2866

PigServer fails with macros without a script file

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.10.0
    • Fix Version/s: 0.11
    • Component/s: None
    • Labels:
      None
    • Patch Info:
      Patch Available

      Description

      Making a call to PigServer.registerQuery with an InputStream will fail if the script contains a macro. This is because QueryParserDriver.makeMacroDef requires a filename. QueryParserDriver.makeMacroDef should be changed to not assume that the script input is from a file.

      1. PIG-2866.1.patch
        2 kB
        Bill Graham
      2. PIG-2866.2.patch
        4 kB
        Bill Graham
      3. PIG-2866.3.patch
        9 kB
        Bill Graham
      4. PIG-2866.4.patch
        9 kB
        Bill Graham

        Activity

        Bill Graham created issue -
        Hide
        Bill Graham added a comment -

        The job fails with an exception like this:

        Caused by: org.apache.pig.impl.logicalLayer.FrontendException: ERROR 1000: Error during parsing. Can not create a Path from a null string
        	at org.apache.pig.PigServer$Graph.parseQuery(PigServer.java:1595)
        	at org.apache.pig.PigServer$Graph.registerQuery(PigServer.java:1534)
        	at org.apache.pig.PigServer.registerQuery(PigServer.java:516)
        	at org.apache.pig.tools.grunt.GruntParser.processPig(GruntParser.java:990)
        	at org.apache.pig.tools.pigscript.parser.PigScriptParser.parse(PigScriptParser.java:412)
        	at org.apache.pig.tools.grunt.GruntParser.parseStopOnError(GruntParser.java:193)
        	at org.apache.pig.PigServer.registerScript(PigServer.java:590)
        	at org.apache.pig.PigServer.registerScript(PigServer.java:555)
        Caused by: java.lang.IllegalArgumentException: Can not create a Path from a null string
        	at org.apache.hadoop.fs.Path.checkPathArg(Path.java:78)
        	at org.apache.hadoop.fs.Path.<init>(Path.java:90)
        	at org.apache.pig.impl.io.FileLocalizer.fetchFilesInternal(FileLocalizer.java:766)
        	at org.apache.pig.impl.io.FileLocalizer.fetchFile(FileLocalizer.java:733)
        	at org.apache.pig.parser.QueryParserDriver.getMacroFile(QueryParserDriver.java:350)
        	at org.apache.pig.parser.QueryParserDriver.makeMacroDef(QueryParserDriver.java:406)
        	at org.apache.pig.parser.QueryParserDriver.expandMacro(QueryParserDriver.java:268)
        	at org.apache.pig.parser.QueryParserDriver.parse(QueryParserDriver.java:169)
        	at org.apache.pig.PigServer$Graph.parseQuery(PigServer.java:1587)
        
        Show
        Bill Graham added a comment - The job fails with an exception like this: Caused by: org.apache.pig.impl.logicalLayer.FrontendException: ERROR 1000: Error during parsing. Can not create a Path from a null string at org.apache.pig.PigServer$Graph.parseQuery(PigServer.java:1595) at org.apache.pig.PigServer$Graph.registerQuery(PigServer.java:1534) at org.apache.pig.PigServer.registerQuery(PigServer.java:516) at org.apache.pig.tools.grunt.GruntParser.processPig(GruntParser.java:990) at org.apache.pig.tools.pigscript.parser.PigScriptParser.parse(PigScriptParser.java:412) at org.apache.pig.tools.grunt.GruntParser.parseStopOnError(GruntParser.java:193) at org.apache.pig.PigServer.registerScript(PigServer.java:590) at org.apache.pig.PigServer.registerScript(PigServer.java:555) Caused by: java.lang.IllegalArgumentException: Can not create a Path from a null string at org.apache.hadoop.fs.Path.checkPathArg(Path.java:78) at org.apache.hadoop.fs.Path.<init>(Path.java:90) at org.apache.pig.impl.io.FileLocalizer.fetchFilesInternal(FileLocalizer.java:766) at org.apache.pig.impl.io.FileLocalizer.fetchFile(FileLocalizer.java:733) at org.apache.pig.parser.QueryParserDriver.getMacroFile(QueryParserDriver.java:350) at org.apache.pig.parser.QueryParserDriver.makeMacroDef(QueryParserDriver.java:406) at org.apache.pig.parser.QueryParserDriver.expandMacro(QueryParserDriver.java:268) at org.apache.pig.parser.QueryParserDriver.parse(QueryParserDriver.java:169) at org.apache.pig.PigServer$Graph.parseQuery(PigServer.java:1587)
        Hide
        Bill Graham added a comment -

        Here's a workaround for now. Instead of passing an InputStream, pass a file:

        InputStream scriptInputStream = ...
        
        File file = File.createTempFile("foo", "bar");
        file.deleteOnExit();
        OutputStream out = new FileOutputStream(file);
        
        int read = 0;
        byte[] bytes = new byte[1024];
        while ((read = scriptInputStream.read(bytes)) != -1) {
           out.write(bytes, 0, read);
        }
        
        scriptInputStream.close();
        out.flush();
        out.close();
        
        ScriptState.get().setFileName(file.getAbsolutePath()); <---- This is important
        pigServer.registerScript(file.getAbsolutePath());
        
        Show
        Bill Graham added a comment - Here's a workaround for now. Instead of passing an InputStream , pass a file: InputStream scriptInputStream = ... File file = File.createTempFile("foo", "bar"); file.deleteOnExit(); OutputStream out = new FileOutputStream(file); int read = 0; byte[] bytes = new byte[1024]; while ((read = scriptInputStream.read(bytes)) != -1) { out.write(bytes, 0, read); } scriptInputStream.close(); out.flush(); out.close(); ScriptState.get().setFileName(file.getAbsolutePath()); <---- This is important pigServer.registerScript(file.getAbsolutePath());
        Hide
        Bill Graham added a comment -

        Here's a unit test with the fix.

        Show
        Bill Graham added a comment - Here's a unit test with the fix.
        Bill Graham made changes -
        Field Original Value New Value
        Attachment PIG-2866.1.patch [ 12540164 ]
        Bill Graham made changes -
        Patch Info Patch Available [ 10042 ]
        Hide
        Alex Rovner added a comment -

        Thanks Bill!

        Show
        Alex Rovner added a comment - Thanks Bill!
        Hide
        Alex Rovner added a comment -

        Should we change the affected version?

        Show
        Alex Rovner added a comment - Should we change the affected version?
        Bill Graham made changes -
        Fix Version/s 0.11 [ 12318878 ]
        Affects Version/s 0.10.0 [ 12316246 ]
        Hide
        Dmitriy V. Ryaboy added a comment -

        2 comments:
        1) having 2 variables, one named fname and the other named filename is confusing. Please rename.

        2) For the test, you are creating an actual temp file. It's better to use mock.Storage, we should encourage use of that utility throughout tests.

        Show
        Dmitriy V. Ryaboy added a comment - 2 comments: 1) having 2 variables, one named fname and the other named filename is confusing. Please rename. 2) For the test, you are creating an actual temp file. It's better to use mock.Storage, we should encourage use of that utility throughout tests.
        Hide
        Bill Graham added a comment -

        Good call. Here's a patch that changes my new test and the existing tests to use mock.Storage.

        Show
        Bill Graham added a comment - Good call. Here's a patch that changes my new test and the existing tests to use mock.Storage.
        Bill Graham made changes -
        Attachment PIG-2866.2.patch [ 12540760 ]
        Hide
        Bill Graham added a comment -

        Actually, hold off on reviewing patch #2. I need to look into something else.

        Show
        Bill Graham added a comment - Actually, hold off on reviewing patch #2. I need to look into something else.
        Bill Graham made changes -
        Patch Info Patch Available [ 10042 ]
        Hide
        Bill Graham added a comment -

        Here's patch #3. It moves the macro tests to their own class so they can use PigServer in local mode and hence use mock.Storage.

        Show
        Bill Graham added a comment - Here's patch #3. It moves the macro tests to their own class so they can use PigServer in local mode and hence use mock.Storage .
        Bill Graham made changes -
        Attachment PIG-2866.3.patch [ 12540789 ]
        Bill Graham made changes -
        Patch Info Patch Available [ 10042 ]
        Bill Graham made changes -
        Issue Type Bug [ 1 ] Improvement [ 4 ]
        Bill Graham made changes -
        Issue Type Improvement [ 4 ] Bug [ 1 ]
        Hide
        Dmitriy V. Ryaboy added a comment -

        +1, commit after adding Apache headers to TestPigServerWithMacros.java

        Show
        Dmitriy V. Ryaboy added a comment - +1, commit after adding Apache headers to TestPigServerWithMacros.java
        Hide
        Dmitriy V. Ryaboy added a comment -

        oh and no @author annotations per project guidelines.

        Show
        Dmitriy V. Ryaboy added a comment - oh and no @author annotations per project guidelines.
        Hide
        Bill Graham added a comment -

        Aww man, rookie mistakes! Adding version 4 of the patch, which gets it prostyle. Will commit shortly.

        Show
        Bill Graham added a comment - Aww man, rookie mistakes! Adding version 4 of the patch, which gets it prostyle. Will commit shortly.
        Bill Graham made changes -
        Attachment PIG-2866.4.patch [ 12540800 ]
        Hide
        Bill Graham added a comment -

        Committed.

        This JIRA should be marked resolved fixed if we can it out of it's bad state.

        Show
        Bill Graham added a comment - Committed. This JIRA should be marked resolved fixed if we can it out of it's bad state.
        Dmitriy V. Ryaboy made changes -
        Assignee Bill Graham [ billgraham ] Dmitriy V. Ryaboy [ dvryaboy ]
        Hide
        Dmitriy V. Ryaboy added a comment -

        was trying to see if reassigning would bring the close button back.. alas.

        Show
        Dmitriy V. Ryaboy added a comment - was trying to see if reassigning would bring the close button back.. alas.
        Dmitriy V. Ryaboy made changes -
        Assignee Dmitriy V. Ryaboy [ dvryaboy ] Bill Graham [ billgraham ]
        Tony Stevenson made changes -
        Priority Major [ 3 ] Minor [ 4 ]
        Dmitriy V. Ryaboy made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Bill Graham made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Bill Graham
            Reporter:
            Bill Graham
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development