Mailing-List: contact commits-help@pig.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@pig.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1743527 - in /pig/trunk: CHANGES.txt
 src/org/apache/pig/parser/PigMacro.java
 test/org/apache/pig/test/TestMacroExpansion.java
Date: Thu, 12 May 2016 16:46:27 -0000
To: commits@pig.apache.org
From: knoguchi@apache.org
Message-Id: <20160512164627.B32983A0331@svn01-us-west.apache.org>
archived-at: Thu, 12 May 2016 16:46:34 -0000

Author: knoguchi
Date: Thu May 12 16:46:27 2016
New Revision: 1743527

URL: http://svn.apache.org/viewvc?rev=1743527&view=rev
Log:
PIG-4880: Overlapping of parameter substitution names inside&outside a macro fails with NPE (knoguchi)

Modified:
    pig/trunk/CHANGES.txt
    pig/trunk/src/org/apache/pig/parser/PigMacro.java
    pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java

Modified: pig/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/pig/trunk/CHANGES.txt?rev=1743527&r1=1743526&r2=1743527&view=diff
==============================================================================
--- pig/trunk/CHANGES.txt (original)
+++ pig/trunk/CHANGES.txt Thu May 12 16:46:27 2016
@@ -119,6 +119,8 @@ PIG-4639: Add better parser for Apache H
 
 BUG FIXES
 
+PIG-4880: Overlapping of parameter substitution names inside&outside a macro fails with NPE (knoguchi)
+
 PIG-4881: TestBuiltin.testUniqueID failing on hadoop-1.x (knoguchi)
 
 PIG-4888: Line number off when reporting syntax error inside a macro (knoguchi)

Modified: pig/trunk/src/org/apache/pig/parser/PigMacro.java
URL: http://svn.apache.org/viewvc/pig/trunk/src/org/apache/pig/parser/PigMacro.java?rev=1743527&r1=1743526&r2=1743527&view=diff
==============================================================================
--- pig/trunk/src/org/apache/pig/parser/PigMacro.java (original)
+++ pig/trunk/src/org/apache/pig/parser/PigMacro.java Thu May 12 16:46:27 2016
@@ -168,14 +168,9 @@ class PigMacro {
 
             Map<String, String> paramVal = pc.getParamVal();
             for (Map.Entry<String, String> e : pigContext.getParamVal().entrySet()) {
-                if (paramVal.containsKey(e.getKey())) {
-                    throw new ParserException(
-                        "Macro contains argument or return value " + e.getKey() + " which conflicts " +
-                        "with a Pig parameter of the same name."
-                    );
-                } else {
-                    paramVal.put(e.getKey(), e.getValue());
-                }
+                // overwrite=false since macro parameters should have precedence
+                // over commandline parameters (if keys overlap)
+                pc.processOrdLine(e.getKey(), e.getValue(), false);
             }
             
             ParameterSubstitutionPreprocessor psp = new ParameterSubstitutionPreprocessor(pc);

Modified: pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java
URL: http://svn.apache.org/viewvc/pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java?rev=1743527&r1=1743526&r2=1743527&view=diff
==============================================================================
--- pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java (original)
+++ pig/trunk/test/org/apache/pig/test/TestMacroExpansion.java Thu May 12 16:46:27 2016
@@ -2280,6 +2280,135 @@ public class TestMacroExpansion {
         verify(script, expected);
     }
 
+    // When  declare-in-macro, macro param and command-line param contain the
+    // same name, last declare wins
+    @Test
+    public void testParamOverLap1() throws Exception {
+        String macro =
+            "DEFINE mygroupby(REL, key, number) RETURNS G {\n" +
+            "    %declare number 333;\n"  +
+            "    $G = GROUP $REL by $key parallel $number;\n" +
+            "};";
+        createFile("my_macro.pig", macro);
+
+        String script =
+            "%declare number 111;\n" +
+            "IMPORT 'my_macro.pig';\n" +
+            "data = LOAD '1234.txt' USING PigStorage() AS (i: int);\n" +
+            "result = mygroupby(data, i, 222);\n" +
+            "STORE result INTO 'test.out' USING PigStorage();";
+
+        String expected =
+            "data = LOAD '1234.txt' USING PigStorage() AS i:int;\n" +
+            "result = GROUP data by (i) parallel 333;\n" +
+            "STORE result INTO 'test.out' USING PigStorage();\n";
+
+        verify(script, expected);
+    }
+
+    // When  default-in-macro, macro param and command-line param contain the
+    // same name, then default should be ignored and macro param to be taken
+    @Test
+    public void testParamOverLap2() throws Exception {
+        String macro =
+            "DEFINE mygroupby(REL, key, number) RETURNS G {\n" +
+            "    %default number 333;\n"  +
+            "    $G = GROUP $REL by $key parallel $number;\n" +
+            "};";
+        createFile("my_macro.pig", macro);
+
+        String script =
+            "%declare number 111;\n" +
+            "IMPORT 'my_macro.pig';\n" +
+            "data = LOAD '1234.txt' USING PigStorage() AS (i: int);\n" +
+            "result = mygroupby(data, i, 222);\n" +
+            "STORE result INTO 'test.out' USING PigStorage();";
+
+        String expected =
+            "data = LOAD '1234.txt' USING PigStorage() AS i:int;\n" +
+            "result = GROUP data by (i) parallel 222;\n" +
+            "STORE result INTO 'test.out' USING PigStorage();\n";
+
+        verify(script, expected);
+    }
+
+    // Overlapping of  macro param and command-line param used to be disallowed.
+    // Now, simply taking the macro param when this happens
+    @Test
+    public void testParamOverLap3() throws Exception {
+        String macro =
+            "DEFINE mygroupby(REL, key, number) RETURNS G {\n" +
+            "    $G = GROUP $REL by $key parallel $number;\n" +
+            "};";
+        createFile("my_macro.pig", macro);
+
+        String script =
+            "%default number 111;\n" +
+            "IMPORT 'my_macro.pig';\n" +
+            "data = LOAD '1234.txt' USING PigStorage() AS (i: int);\n" +
+            "result = mygroupby(data, i, 222);\n" +
+            "STORE result INTO 'test.out' USING PigStorage();";
+
+        String expected =
+            "data = LOAD '1234.txt' USING PigStorage() AS i:int;\n" +
+            "result = GROUP data by (i) parallel 222;\n" +
+            "STORE result INTO 'test.out' USING PigStorage();\n";
+
+        verify(script, expected);
+    }
+
+    // Testing inline declare and commandline param overlap.
+    // testParamOverLap1 should cover this case as well but creating a specific
+    // case since this pair used to fail with NPE
+    @Test
+    public void testParamOverLap4() throws Exception {
+        String macro =
+            "DEFINE mygroupby(REL, key) RETURNS G {\n" +
+            "    %declare number 333;\n"  +
+            "    $G = GROUP $REL by $key parallel $number;\n" +
+            "};";
+        createFile("my_macro.pig", macro);
+
+        String script =
+            "%default number 111;\n" +
+            "IMPORT 'my_macro.pig';\n" +
+            "data = LOAD '1234.txt' USING PigStorage() AS (i: int);\n" +
+            "result = mygroupby(data, i);\n" +
+            "STORE result INTO 'test.out' USING PigStorage();";
+
+        String expected =
+            "data = LOAD '1234.txt' USING PigStorage() AS i:int;\n" +
+            "result = GROUP data by (i) parallel 333;\n" +
+            "STORE result INTO 'test.out' USING PigStorage();\n";
+
+        verify(script, expected);
+    }
+
+    // default-in-macro should yield to command-line param
+    @Test
+    public void testParamOverLap5() throws Exception {
+        String macro =
+            "DEFINE mygroupby(REL, key) RETURNS G {\n" +
+            "    %default number 333;\n"  +
+            "    $G = GROUP $REL by $key parallel $number;\n" +
+            "};";
+        createFile("my_macro.pig", macro);
+
+        String script =
+            "%declare number 111;\n" +
+            "IMPORT 'my_macro.pig';\n" +
+            "data = LOAD '1234.txt' USING PigStorage() AS (i: int);\n" +
+            "result = mygroupby(data, i);\n" +
+            "STORE result INTO 'test.out' USING PigStorage();";
+
+        String expected =
+            "data = LOAD '1234.txt' USING PigStorage() AS i:int;\n" +
+            "result = GROUP data by (i) parallel 111;\n" +
+            "STORE result INTO 'test.out' USING PigStorage();\n";
+
+        verify(script, expected);
+    }
+
     //-------------------------------------------------------------------------
     
     private void testMacro(String content) throws Exception {