You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pig.apache.org by rd...@apache.org on 2011/05/03 02:23:43 UTC

svn commit: r1098870 - in /pig/branches/branch-0.9: src/org/apache/pig/parser/PigMacro.java src/org/apache/pig/parser/QueryParser.g src/org/apache/pig/parser/QueryParserDriver.java test/org/apache/pig/test/TestMacroExpansion.java

Author: rding
Date: Tue May  3 00:23:42 2011
New Revision: 1098870

URL: http://svn.apache.org/viewvc?rev=1098870&view=rev
Log:
PIG-1998: Allow macro to return void

Modified:
    pig/branches/branch-0.9/src/org/apache/pig/parser/PigMacro.java
    pig/branches/branch-0.9/src/org/apache/pig/parser/QueryParser.g
    pig/branches/branch-0.9/src/org/apache/pig/parser/QueryParserDriver.java
    pig/branches/branch-0.9/test/org/apache/pig/test/TestMacroExpansion.java

Modified: pig/branches/branch-0.9/src/org/apache/pig/parser/PigMacro.java
URL: http://svn.apache.org/viewvc/pig/branches/branch-0.9/src/org/apache/pig/parser/PigMacro.java?rev=1098870&r1=1098869&r2=1098870&view=diff
==============================================================================
--- pig/branches/branch-0.9/src/org/apache/pig/parser/PigMacro.java (original)
+++ pig/branches/branch-0.9/src/org/apache/pig/parser/PigMacro.java Tue May  3 00:23:42 2011
@@ -23,7 +23,6 @@ import java.io.StreamTokenizer;
 import java.io.StringReader;
 import java.io.StringWriter;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -103,7 +102,7 @@ class PigMacro {
             throw new RuntimeException(msg);
         }
         boolean isVoidReturn = false;
-        if (rets.size() == 1 && rets.get(0).equals("void")) {           
+        if (rets.isEmpty()) {           
             if (outputs != null && outputs.length > 0) {
                 String msg = getErrorMessage(file, line, "Cannot expand macro '"
                         + name + "'",
@@ -240,7 +239,7 @@ class PigMacro {
      * Validates that return alias exists in the macro body.
      */
     void validate() throws IOException {
-        if (rets.size() == 0 || (rets.size() == 1 && rets.get(0).equals("void"))) {
+        if (rets.isEmpty()) {
             return;
         }
         
@@ -254,61 +253,30 @@ class PigMacro {
         st.wordChars('$', '$');
         st.lowerCaseMode(false);
         st.ordinaryChar('/');
-        st.slashSlashComments(true);
         st.slashStarComments(true);
         
-        String prevWord = null;
-        int lookahead = st.nextToken();
-        while (lookahead != StreamTokenizer.TT_EOF) {
-            if (lookahead == StreamTokenizer.TT_WORD) {
-                if (st.sval.charAt(0) == '$' && st.sval.length() > 1) {
-                    // check if this is define statement
-                    if (prevWord != null && prevWord.equalsIgnoreCase("define")) {
-                        testSet.add(st.sval.substring(1));
-                        prevWord = st.sval; 
-                    } else {
-                        prevWord = st.sval;
-                        lookahead = st.nextToken();
-                        if (lookahead == StreamTokenizer.TT_EOF) { 
-                            break;
-                        } else if (lookahead == StreamTokenizer.TT_WORD
-                                && st.sval.equalsIgnoreCase("if")) { 
-                            // this is an alias for split statements
-                            testSet.add(prevWord.substring(1));
-                            prevWord = st.sval; 
-                        } else if (lookahead == '=') {
-                            // check if this is regular statement: alias = ...
-                            lookahead = st.nextToken();
-                            // check for pattern such as '=='
-                            if (lookahead == StreamTokenizer.TT_WORD) {
-                                testSet.add(prevWord.substring(1));
-                                prevWord = st.sval; 
-                            } else {
-                                st.pushBack();
-                            }
-                        } else if (lookahead == ',') {
-                            // possible mult-alias inlining of a macro
-                            ArrayList<String> mlist = new ArrayList<String>();
-                            mlist.add(prevWord);
-                            int n = isMultiValueReturn(st, mlist, true);
-                            if (n == StreamTokenizer.TT_EOF) break;    
-                            if (n == 0) {
-                                for (String s : mlist) {
-                                    testSet.add(s.substring(1));
-                                }
-                                prevWord = null;
-                            } else if (n == StreamTokenizer.TT_WORD) {
-                                prevWord = st.sval;
-                            }
+        while (st.nextToken() != StreamTokenizer.TT_EOF) {
+            if (matchWord(st, "define", false) && matchDollarAlias(st, true)) {
+                testSet.add(st.sval.substring(1));
+            } else if (matchDollarAlias(st, false)) {
+                String prevWord = st.sval;
+                if (matchWord(st, "if", true)) {
+                    testSet.add(prevWord.substring(1));
+                } else if (matchChar(st, '=', true) && !matchChar(st, '=', true)) {
+                     testSet.add(prevWord.substring(1)); 
+                } else if (matchChar(st, ',', true)) {
+                    // possible mult-alias inlining of a macro
+                    ArrayList<String> mlist = new ArrayList<String>();
+                    mlist.add(prevWord);
+                    if (isMultiValueReturn(st, mlist, true)) {    
+                        for (String s : mlist) {
+                            testSet.add(s.substring(1));
                         }
-                    }
-                } else {
-                    prevWord = st.sval;
-                }    
-            } else {
-                prevWord = null;
+                    } 
+                } 
+            } else if (matchChar(st, '-', false) && matchChar(st, '-', true)) {
+                skipSingleLineComment(st);
             }
-            lookahead = st.nextToken();
         }
         
         for (String s : rets) {
@@ -320,29 +288,60 @@ class PigMacro {
     }
     
     // check for multi-value return pattern: alias, alias, ..., alias =
-    private int isMultiValueReturn(StreamTokenizer st, List<String> mlist,
-            boolean comma) throws IOException {
+    private static boolean isMultiValueReturn(StreamTokenizer st,
+            List<String> mlist, boolean comma) throws IOException {
         int lookahead = st.nextToken();
-        if (lookahead != StreamTokenizer.TT_EOF) {
-            if ((comma && lookahead == StreamTokenizer.TT_WORD)
-                    || (!comma && lookahead == ',')) {
-                if (lookahead == StreamTokenizer.TT_WORD
-                        && st.sval.charAt(0) == '$' && st.sval.length() > 1) {
-                    mlist.add(st.sval);
-                }
-                return isMultiValueReturn(st, mlist, !comma);
-            }
-            if (!comma && lookahead == '=') {
-                lookahead = st.nextToken();
-                // check for pattern such as '=='
-                if (lookahead == StreamTokenizer.TT_WORD) {
-                    return 0;
-                } else {
-                    st.pushBack();
-                }
+        if ((comma && lookahead == StreamTokenizer.TT_WORD)
+                || (!comma && matchChar(st, ',', false))) {
+            if (matchDollarAlias(st, false)) {
+                mlist.add(st.sval);
             }
+            return isMultiValueReturn(st, mlist, !comma);
+        }
+        if (!comma && lookahead == '=' && !matchChar(st, '=', true)) {
+            return true;
+        }
+        return false;
+    }
+        
+    private static boolean matchDollarAlias(StreamTokenizer st, boolean next)
+            throws IOException {
+        int type = next ? st.nextToken() : st.ttype;
+        if (type == StreamTokenizer.TT_WORD && st.sval.charAt(0) == '$'
+                && st.sval.length() > 1) {
+            return true;
+        }
+        if (next) st.pushBack();
+        return false;
+    }
+
+    private static boolean matchWord(StreamTokenizer st, String word,
+            boolean next) throws IOException {
+        int type = next ? st.nextToken() : st.ttype;
+        if (type == StreamTokenizer.TT_WORD
+                && st.sval.equalsIgnoreCase(word)) {
+            return true;
+        }
+        if (next) st.pushBack();
+ 
+        return false;
+    }
+    
+    private static boolean matchChar(StreamTokenizer st, int c, boolean next)
+            throws IOException {
+        int type = next ? st.nextToken() : st.ttype;
+        if (type == c) return true;
+        if (next) st.pushBack();
+        return false;
+    }
+
+    private static void skipSingleLineComment(StreamTokenizer st)
+            throws IOException {
+        int lookahead = st.nextToken();
+        while (lookahead != StreamTokenizer.TT_EOF && lookahead != '\n') {
+            lookahead = st.nextToken();
         }
-        return lookahead;
+        st.pushBack();
     }
     
     private static void traverseMacro(Tree t, List<CommonTree> nodes,

Modified: pig/branches/branch-0.9/src/org/apache/pig/parser/QueryParser.g
URL: http://svn.apache.org/viewvc/pig/branches/branch-0.9/src/org/apache/pig/parser/QueryParser.g?rev=1098870&r1=1098869&r2=1098870&view=diff
==============================================================================
--- pig/branches/branch-0.9/src/org/apache/pig/parser/QueryParser.g (original)
+++ pig/branches/branch-0.9/src/org/apache/pig/parser/QueryParser.g Tue May  3 00:23:42 2011
@@ -219,7 +219,7 @@ macro_return_clause 
     : RETURNS alias (COMMA alias)*
         -> ^(RETURN_VAL alias+)
     | RETURNS VOID 
-        -> ^(RETURN_VAL VOID)
+        -> ^(RETURN_VAL)
 ;
 
 macro_body_clause : content

Modified: pig/branches/branch-0.9/src/org/apache/pig/parser/QueryParserDriver.java
URL: http://svn.apache.org/viewvc/pig/branches/branch-0.9/src/org/apache/pig/parser/QueryParserDriver.java?rev=1098870&r1=1098869&r2=1098870&view=diff
==============================================================================
--- pig/branches/branch-0.9/src/org/apache/pig/parser/QueryParserDriver.java (original)
+++ pig/branches/branch-0.9/src/org/apache/pig/parser/QueryParserDriver.java Tue May  3 00:23:42 2011
@@ -283,17 +283,17 @@ public class QueryParserDriver {
         Tree defNode = t.getChild(1);
 
         // get parameter markers
+        ArrayList<String> params = new ArrayList<String>(); 
         Tree paramNode = defNode.getChild(0);
         int n = paramNode.getChildCount();
-        ArrayList<String> params = new ArrayList<String>(); 
         for (int i = 0; i < n; i++) {
             params.add(paramNode.getChild(i).getText());
         }
 
         // get return alias markers
+        ArrayList<String> returns = new ArrayList<String>(); 
         Tree retNode = defNode.getChild(1);
         int m = retNode.getChildCount();
-        ArrayList<String> returns = new ArrayList<String>(); 
         for (int i = 0; i < m; i++) {
             returns.add(retNode.getChild(i).getText());
         }

Modified: pig/branches/branch-0.9/test/org/apache/pig/test/TestMacroExpansion.java
URL: http://svn.apache.org/viewvc/pig/branches/branch-0.9/test/org/apache/pig/test/TestMacroExpansion.java?rev=1098870&r1=1098869&r2=1098870&view=diff
==============================================================================
--- pig/branches/branch-0.9/test/org/apache/pig/test/TestMacroExpansion.java (original)
+++ pig/branches/branch-0.9/test/org/apache/pig/test/TestMacroExpansion.java Tue May  3 00:23:42 2011
@@ -1009,6 +1009,44 @@ public class TestMacroExpansion {
         validateFailure(macro + script, expectedErr);
     }
     
+    @Test // macro doesn't contain return alias
+    public void checkReturnAliasTest1() throws Throwable {
+        String macro = "define group_and_count_1 (A,C) returns B {\n" +
+            "    /* this is a test $B and \n" +
+            "    $B = JOIN $A BY user, $C BY user; */\n" +
+            "    B = JOIN $A BY user, $C BY user;\n" +
+            "};\n";
+        
+        String script = 
+            "alpha = load 'users' as (user, age, zip);\n" +
+            "beta = load 'links' as (user, link, view);\n" +
+            "gamma = group_and_count_1 (alpha,beta);\n" +
+            "store gamma into 'byuser';\n";
+        
+        String expectedErr = "Reason: Macro 'group_and_count_1' missing return alias: B";
+        
+        validateFailure(macro + script, expectedErr);
+    }
+    
+    @Test // macro doesn't contain return alias
+    public void checkReturnAliasTest2() throws Throwable {
+        String macro = "define group_and_count_2 (A,C) returns B {\n" +
+            "    -- $B = JOIN $A BY user, $C BY user; \n" +
+            "    --$B = JOIN $A BY user, $C BY user; \n" +
+            "    B = JOIN $A BY user, $C BY user;\n" +
+            "};\n";
+        
+        String script = 
+            "alpha = load 'users' as (user, age, zip);\n" +
+            "beta = load 'links' as (user, link, view);\n" +
+            "gamma = group_and_count_2 (alpha,beta);\n" +
+            "store gamma into 'byuser';\n";
+        
+        String expectedErr = "Reason: Macro 'group_and_count_2' missing return alias: B";
+        
+        validateFailure(macro + script, expectedErr);
+    }
+    
     @Test
     public void recursiveMacrosTest() throws Exception {
         String macro1 = "define group_and_partition (A, group_key, reducers) returns B {\n" +
@@ -1946,6 +1984,8 @@ public class TestMacroExpansion {
             w.close();
             
             grunt.checkScript(scriptFile);
+            
+            Assert.fail("Expected exception isn't thrown");
         } catch (FrontendException e) {  
             String msg = e.getMessage();
             int pos = msg.indexOf("Reason:");