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/10 00:51:35 UTC
svn commit: r1101260 - in /pig/branches/branch-0.9: ./ src/org/apache/pig/
src/org/apache/pig/parser/ test/org/apache/pig/test/
Author: rding
Date: Mon May 9 22:51:34 2011
New Revision: 1101260
URL: http://svn.apache.org/viewvc?rev=1101260&view=rev
Log:
PIG-2012: Comments at the begining of the file throws off line numbers in errors
Modified:
pig/branches/branch-0.9/CHANGES.txt
pig/branches/branch-0.9/src/org/apache/pig/PigServer.java
pig/branches/branch-0.9/src/org/apache/pig/parser/PigMacro.java
pig/branches/branch-0.9/src/org/apache/pig/parser/PigParserNode.java
pig/branches/branch-0.9/src/org/apache/pig/parser/PigParserNodeAdaptor.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/src/org/apache/pig/parser/QueryParserUtils.java
pig/branches/branch-0.9/src/org/apache/pig/parser/SourceLocation.java
pig/branches/branch-0.9/test/org/apache/pig/test/TestMacroExpansion.java
Modified: pig/branches/branch-0.9/CHANGES.txt
URL: http://svn.apache.org/viewvc/pig/branches/branch-0.9/CHANGES.txt?rev=1101260&r1=1101259&r2=1101260&view=diff
==============================================================================
--- pig/branches/branch-0.9/CHANGES.txt (original)
+++ pig/branches/branch-0.9/CHANGES.txt Mon May 9 22:51:34 2011
@@ -174,6 +174,8 @@ PIG-1696: Performance: Use System.arrayc
BUG FIXES
+PIG-2012: Comments at the begining of the file throws off line numbers in errors (rding)
+
PIG-2043: Ship antlr-runtime.jar to backend (daijy)
PIG-2049: Pig should display TokenMgrError message consistently across all parsers (rding)
Modified: pig/branches/branch-0.9/src/org/apache/pig/PigServer.java
URL: http://svn.apache.org/viewvc/pig/branches/branch-0.9/src/org/apache/pig/PigServer.java?rev=1101260&r1=1101259&r2=1101260&view=diff
==============================================================================
--- pig/branches/branch-0.9/src/org/apache/pig/PigServer.java (original)
+++ pig/branches/branch-0.9/src/org/apache/pig/PigServer.java Mon May 9 22:51:34 2011
@@ -1575,8 +1575,13 @@ public class PigServer {
scriptCache.add( "" );
currentLineNum++;
}
- scriptCache.add( query );
- currentLineNum = startLine;
+ BufferedReader br = new BufferedReader(new StringReader(query));
+ String line = br.readLine();
+ while (line != null) {
+ scriptCache.add(line);
+ currentLineNum++;
+ line = br.readLine();
+ }
}
} else {
scriptCache.add( query );
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=1101260&r1=1101259&r2=1101260&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 Mon May 9 22:51:34 2011
@@ -36,6 +36,7 @@ import org.antlr.runtime.tree.CommonTree
import org.antlr.runtime.tree.Tree;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
+import org.apache.pig.parser.PigParserNode.InvocationPoint;
import org.apache.pig.tools.parameters.ParameterSubstitutionPreprocessor;
class PigMacro {
@@ -50,6 +51,9 @@ class PigMacro {
private Map<String, PigMacro> seen;
private Set<String> macroStack;
private long idx = 0;
+
+ // The start line number of this macro in the script
+ private int startLine = 0;
PigMacro(String name, String file, List<String> params,
List<String> returns, String body, Map<String, PigMacro> seen) {
@@ -71,9 +75,17 @@ class PigMacro {
Set<String> getStack() { return macroStack; }
- private CommonTree inline(String[] inputs, String[] outputs, int lineNumber,
+ void setStartLine(int start) {
+ this.startLine = start;
+ }
+
+ int getStartLine() {
+ return startLine;
+ }
+
+ private CommonTree inline(String[] inputs, String[] outputs, CommonTree t,
String file) throws ParserException {
- String in = substituteParams(inputs, outputs, lineNumber, file);
+ String in = substituteParams(inputs, outputs, t.getLine(), file);
Set<String> masks = new HashSet<String>();
if (inputs != null) {
@@ -86,7 +98,7 @@ class PigMacro {
masks.add(s);
}
- return maskAlias(in, masks, lineNumber, file);
+ return maskAlias(in, masks, t, file);
}
@@ -155,8 +167,13 @@ class PigMacro {
return writer.toString();
}
- private CommonTree maskAlias(String in, Set<String> masks, int line,
+ private CommonTree maskAlias(String in, Set<String> masks, CommonTree tree,
String file) throws ParserException {
+
+ // this is the MACRO_INLINE node. the real line number is in the
+ // macro name node
+ int line = tree.getChild(0).getLine();
+
CharStream input = null;
try {
// parse macro body into ast
@@ -171,8 +188,8 @@ class PigMacro {
CommonTokenStream tokens = new CommonTokenStream(lex);
QueryParser.query_return result = null;
- QueryParser parser = QueryParserUtils.createParser(tokens);
-
+ QueryParser parser = QueryParserUtils.createParser(tokens, startLine-1);
+
try {
result = parser.query();
} catch (RecognitionException e) {
@@ -198,7 +215,17 @@ class PigMacro {
+ body);
throw new ParserException(msg);
}
-
+
+ // add macro invocation points to the expanded macro tree
+ PigParserNode pnode = (PigParserNode)tree;
+ List<InvocationPoint> invStack = pnode.getInvocationStack();
+ List<InvocationPoint> newInvStack = (invStack == null) ? new ArrayList<InvocationPoint>()
+ : new ArrayList<InvocationPoint>(invStack);
+
+ InvocationPoint pt = new InvocationPoint(line, file, name);
+ newInvStack.add(pt);
+ setInvocationStack(ast, newInvStack);
+
// recursively expand the inline macros
List<CommonTree> inlineNodes = new ArrayList<CommonTree>();
traverseMacro(ast, inlineNodes, "MACRO_INLINE");
@@ -235,6 +262,15 @@ class PigMacro {
return commonTree;
}
+ private static void setInvocationStack(Tree ast, List<InvocationPoint> stack) {
+ PigParserNode node = (PigParserNode)ast;
+ node.setInvocationStack(stack);
+ int n = node.getChildCount();
+ for (int i = 0; i < n; i++) {
+ setInvocationStack(node.getChild(i), stack);
+ }
+ }
+
/*
* Validates that return alias exists in the macro body.
*/
@@ -412,7 +448,7 @@ class PigMacro {
params[i] = t.getChild(2).getChild(i).getText();
}
- return macro.inline(params, rets, t.getLine(), file);
+ return macro.inline(params, rets, t, file);
}
private static String getErrorMessage(String file, int line, String header,
Modified: pig/branches/branch-0.9/src/org/apache/pig/parser/PigParserNode.java
URL: http://svn.apache.org/viewvc/pig/branches/branch-0.9/src/org/apache/pig/parser/PigParserNode.java?rev=1101260&r1=1101259&r2=1101260&view=diff
==============================================================================
--- pig/branches/branch-0.9/src/org/apache/pig/parser/PigParserNode.java (original)
+++ pig/branches/branch-0.9/src/org/apache/pig/parser/PigParserNode.java Mon May 9 22:51:34 2011
@@ -17,6 +17,8 @@
*/
package org.apache.pig.parser;
+import java.util.List;
+
import org.antlr.runtime.Token;
import org.antlr.runtime.tree.CommonTree;
import org.antlr.runtime.tree.Tree;
@@ -26,17 +28,32 @@ public class PigParserNode extends Commo
// the script file this node belongs to
private String fileName = null;
- public PigParserNode(Token t, String fileName) {
+ // macro body starting line number
+ private int startLine = 0;
+
+ // stack of macro invocations
+ private List<InvocationPoint> invokStack;
+
+ public PigParserNode(Token t, String fileName, int lineOffset) {
super(t);
+ if (t != null && lineOffset > 0) {
+ t.setLine(t.getLine() + lineOffset);
+ }
this.fileName = fileName;
}
+
+ public PigParserNode(Token t, String fileName, Token start) {
+ this(t, fileName, 0);
+ this.startLine = start.getLine();
+ }
public PigParserNode(PigParserNode node) {
super(node);
this.fileName = node.getFileName();
+ this.invokStack = node.invokStack;
+ this.startLine = node.startLine;
}
-
public Tree dupNode() {
return new PigParserNode(this);
}
@@ -44,5 +61,47 @@ public class PigParserNode extends Commo
public String getFileName() {
return fileName;
}
+
+ public int getStartLine() {
+ return startLine;
+ }
+
+ public void setInvocationStack(List<InvocationPoint> stack) {
+ invokStack = stack;
+ }
+
+ public List<InvocationPoint> getInvocationStack() {
+ return invokStack;
+ }
+
+ public InvocationPoint getNextInvocationPoint() {
+ if (invokStack == null || invokStack.isEmpty()) return null;
+ return invokStack.remove(0);
+ }
+
+ // 'macro' is invoked at 'line' of 'file'
+ public static class InvocationPoint {
+ private int line;
+ private String file;
+ private String macro;
+
+ public InvocationPoint(int line, String file, String macro) {
+ this.line = line;
+ this.file = file;
+ this.macro = macro;
+ }
+
+ public int getLine() {
+ return line;
+ }
+
+ public String getFile() {
+ return file;
+ }
+
+ public String getMacro() {
+ return macro;
+ }
+ }
}
Modified: pig/branches/branch-0.9/src/org/apache/pig/parser/PigParserNodeAdaptor.java
URL: http://svn.apache.org/viewvc/pig/branches/branch-0.9/src/org/apache/pig/parser/PigParserNodeAdaptor.java?rev=1101260&r1=1101259&r2=1101260&view=diff
==============================================================================
--- pig/branches/branch-0.9/src/org/apache/pig/parser/PigParserNodeAdaptor.java (original)
+++ pig/branches/branch-0.9/src/org/apache/pig/parser/PigParserNodeAdaptor.java Mon May 9 22:51:34 2011
@@ -24,13 +24,16 @@ public class PigParserNodeAdaptor extend
private String source;
- PigParserNodeAdaptor(String source) {
+ private int lineOffset;
+
+ PigParserNodeAdaptor(String source, int lineOffset) {
this.source = source;
+ this.lineOffset = lineOffset;
}
@Override
public Object create(Token t) {
- return new PigParserNode(t, source);
+ return new PigParserNode(t, source, lineOffset);
}
}
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=1101260&r1=1101259&r2=1101260&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 Mon May 9 22:51:34 2011
@@ -222,7 +222,7 @@ macro_return_clause
;
macro_body_clause : content
- -> ^(MACRO_BODY { new PigParserNode(new CommonToken(1, $content.text), this.getSourceName()) } )
+ -> ^(MACRO_BODY { new PigParserNode(new CommonToken(1, $content.text), this.getSourceName(), $content.start) } )
;
macro_clause : macro_param_clause macro_return_clause macro_body_clause
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=1101260&r1=1101259&r2=1101260&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 Mon May 9 22:51:34 2011
@@ -394,6 +394,10 @@ public class QueryParserDriver {
throw new ParserException(msg);
}
+ // set the starting line number of the macro
+ PigParserNode pnode = (PigParserNode)bodyNode.getChild(0);
+ pm.setStartLine(pnode.getStartLine());
+
seen.put(mn, pm);
// delete this node
@@ -469,5 +473,5 @@ public class QueryParserDriver {
sb.append(". Reason: ").append(reason);
}
return sb.toString();
- }
+ }
}
Modified: pig/branches/branch-0.9/src/org/apache/pig/parser/QueryParserUtils.java
URL: http://svn.apache.org/viewvc/pig/branches/branch-0.9/src/org/apache/pig/parser/QueryParserUtils.java?rev=1101260&r1=1101259&r2=1101260&view=diff
==============================================================================
--- pig/branches/branch-0.9/src/org/apache/pig/parser/QueryParserUtils.java (original)
+++ pig/branches/branch-0.9/src/org/apache/pig/parser/QueryParserUtils.java Mon May 9 22:51:34 2011
@@ -229,9 +229,13 @@ public class QueryParserUtils {
}
static QueryParser createParser(CommonTokenStream tokens) {
+ return createParser(tokens, 0);
+ }
+
+ static QueryParser createParser(CommonTokenStream tokens, int lineOffset) {
QueryParser parser = new QueryParser(tokens);
PigParserNodeAdaptor adaptor = new PigParserNodeAdaptor(
- tokens.getSourceName());
+ tokens.getSourceName(), lineOffset);
parser.setTreeAdaptor(adaptor);
return parser;
}
Modified: pig/branches/branch-0.9/src/org/apache/pig/parser/SourceLocation.java
URL: http://svn.apache.org/viewvc/pig/branches/branch-0.9/src/org/apache/pig/parser/SourceLocation.java?rev=1101260&r1=1101259&r2=1101260&view=diff
==============================================================================
--- pig/branches/branch-0.9/src/org/apache/pig/parser/SourceLocation.java (original)
+++ pig/branches/branch-0.9/src/org/apache/pig/parser/SourceLocation.java Mon May 9 22:51:34 2011
@@ -18,11 +18,15 @@
package org.apache.pig.parser;
+import org.apache.pig.parser.PigParserNode.InvocationPoint;
+
public class SourceLocation {
private String file = null; // Name of the source, null if unknown.
private int line = -1; // line number, -1 if unknown.
private int offset = -1; // offset, -f if unknown.
+ private PigParserNode node; // corresponding parser tree node
+
public SourceLocation() {
}
@@ -36,12 +40,14 @@ public class SourceLocation {
this.file = tree.getFileName();
this.line = tree.getLine();
this.offset = tree.getCharPositionInLine();
+ this.node = tree;
}
public SourceLocation(SourceLocation location) {
this.file = location.file;
this.line = location.line;
this.offset = location.offset;
+ this.node = location.node;
}
public String file() {
@@ -56,12 +62,27 @@ public class SourceLocation {
return offset;
}
+ public PigParserNode node() {
+ return node;
+ }
+
@Override
public String toString() {
if( line == -1 )
return "";
- StringBuilder sb = new StringBuilder( "<" );
+ StringBuilder sb = new StringBuilder();
+ if (node != null) {
+ InvocationPoint pt = node.getNextInvocationPoint();
+ while (pt != null) {
+ sb.append("\n");
+ sb.append("at expanding macro '" + pt.getMacro() + "' ("
+ + pt.getFile() + ":" + pt.getLine() + ")");
+ pt = node.getNextInvocationPoint();
+ }
+ sb.append("\n");
+ }
+ sb.append( "<" );
if( file != null )
sb.append( "file " + file + ", " );
sb.append( "line " + line +", column " + offset + "> " );
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=1101260&r1=1101259&r2=1101260&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 Mon May 9 22:51:34 2011
@@ -34,6 +34,7 @@ import org.apache.pig.impl.PigContext;
import org.apache.pig.impl.logicalLayer.FrontendException;
import org.apache.pig.tools.grunt.Grunt;
import org.apache.pig.tools.pigstats.PigStats;
+import org.apache.pig.tools.pigstats.ScriptState;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Assert;
@@ -1115,6 +1116,70 @@ public class TestMacroExpansion {
verify(script, expected);
}
+ @Test // PIG-2012
+ public void lineNumberTest1() throws Throwable {
+ StringBuilder sb = new StringBuilder();
+ sb.append("-- Given daily input and a particular year, analyze how\n")
+ .append("-- stock prices changed on days dividends were paid out.\n");
+
+ sb.append("define dividend_analysis (daily, year, daily_symbol, daily_open, daily_close)\n" +
+ "returns analyzed {\n" +
+ "divs = load 'NYSE_dividends' as (exchange:chararray,\n" +
+ " symbol:chararray, date:chararray, dividends:float);\n" +
+ "divsthisyear = filter divs by date matches '$year-.*';\n" +
+ "dailythisyear = filter $daily by date matches '$year-.*';\n" +
+ "jnd = join divsthisyear by symbol, dailythisyear by $daily_symbol;\n" +
+ "$analyzed = foreach jnd generate $daily_symbol, $daily_close - $daily_open;\n" +
+ "};\n");
+
+ sb.append("daily = load 'NYSE_daily' as (exchange:chararray, symbol:chararray,\n" +
+ "date:chararray, open:float, high:float, low:float, close:float,\n" +
+ "volume:int, adj_close:float);\n" +
+ "results = dividend_analysis(daily, '2009', 'symbol', 'open', 'close');\n" +
+ "store results into 'output';\n");
+
+ String expectedErr = "at expanding macro 'dividend_analysis' (myscript.pig:15)\n" +
+ "<file myscript.pig, line 10, column 35> Invalid field projection. Projected field [symbol] " +
+ "does not exist in schema:";
+
+ validateFailure(sb.toString(), expectedErr, "at");
+ }
+
+ @Test // PIG-2012
+ public void lineNumberTest2() throws Throwable {
+ StringBuilder sb = new StringBuilder();
+ sb.append("-- Given daily input and a particular year, analyze how\n")
+ .append("-- stock prices changed on days dividends were paid out.\n");
+
+ sb.append("define dividend_analysis (daily, year, daily_symbol, daily_open, daily_close)\n" +
+ "returns analyzed {\n" +
+ "divs = load 'NYSE_dividends' as (exchange:chararray,\n" +
+ " symbol:chararray, date:chararray, dividends:float);\n" +
+ "divsthisyear = filter divs by date matches '$year-.*';\n" +
+ "dailythisyear = filter $daily by date matches '$year-.*';\n" +
+ "jnd = join divsthisyear by symbol, dailythisyear by $daily_symbol;\n" +
+ "$analyzed = foreach jnd generate $daily_symbol, $daily_close - $daily_open;\n" +
+ "};\n");
+
+ sb.append("define addition_macro (daily, year, daily_symbol, daily_open, daily_close) returns B {\n" +
+ "$B = dividend_analysis($daily, '$year', '$daily_symbol', '$daily_open', '$daily_close');\n" +
+ "};\n");
+
+ sb.append("daily = load 'NYSE_daily' as (exchange:chararray, symbol:chararray,\n" +
+ "date:chararray, open:float, high:float, low:float, close:float,\n" +
+ "volume:int, adj_close:float);\n" +
+ "results = addition_macro(daily, '2009', 'symbol', 'open', 'close');\n" +
+ "store results into 'output';\n");
+
+ String expectedErr =
+ "at expanding macro 'addition_macro' (myscript.pig:18)\n" +
+ "at expanding macro 'dividend_analysis' (myscript.pig:13)\n" +
+ "<file myscript.pig, line 10, column 35> Invalid field projection. Projected field [symbol] " +
+ "does not exist in schema:";
+
+ validateFailure(sb.toString(), expectedErr, "at");
+ }
+
@Test
public void recursiveMacrosTest3() throws Exception {
String macro1 = "define group_and_partition (A, group_key, reducers) returns B, D {\n" +
@@ -2015,7 +2080,10 @@ public class TestMacroExpansion {
}
private void validateFailure(String piglatin, String expectedErr, String keyword) throws Throwable {
- String scriptFile = "mymacrotest.pig";
+ String scriptFile = "myscript.pig";
+
+ ScriptState ss = ScriptState.get();
+ ss.setFileName(scriptFile);
try {
BufferedReader br = new BufferedReader(new StringReader(piglatin));
@@ -2028,10 +2096,17 @@ public class TestMacroExpansion {
grunt.checkScript(scriptFile);
Assert.fail("Expected exception isn't thrown");
- } catch (FrontendException e) {
+ } catch (FrontendException e) {
String msg = e.getMessage();
int pos = msg.indexOf(keyword);
- Assert.assertEquals(expectedErr, msg.substring(pos));
+ if (pos < 0) {
+ Throwable cause = e.getCause();
+ if (cause != null) {
+ msg = cause.getMessage();
+ pos = msg.indexOf(keyword);
+ }
+ }
+ Assert.assertEquals(expectedErr, msg.substring(pos, pos+expectedErr.length()));
} finally {
new File(scriptFile).delete();
}