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

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

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development