[
https://issues.apache.org/jira/browse/PIG-1931?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13010859#comment-13010859
]
Xuefu Zhang commented on PIG-1931:
----------------------------------
I haven't finished reviewing (some code may require discussion in person), but
here is my comments so far.
1. QueryParser.g
1) This file is for syntax check and AST generation. It's better to move the
validation to one of the tree parsers.
2)For readability, please divide the long rules into sub-rules, each rule
having only one root (^). Also, remove unnecessary token renaming such as
rets+=alias.
3)Import and returns need to be added to eid (accross all tree parsers as well).
2. AliasMasker.g
1) Please simplify, be consistent with QueryParser.g, as you don't generate
pig script any more. For instance,
output_clause
: ^( OUTPUT stream_cmd ( stream_cmd)* )
it should be just ^( OUTPUT stream_cmd+ )
3. QueryParserDriver.java
1)Error reporting should be consistent with others (especially line numbers and
offsets)
2)I don't see how currentImport is updated anywhere.
3)In inlineMacro(), I don't see how nodes variable is updated. Isn't it always
empty?
4. MacroExpansionDebug.g
1) pasted/copied code should be removed. For example, getErrorMessage()
2) I see a few unused member variables/methods.
3) File name is confusing. It seems having nothing to do with macro expansion.
It just prints a script from an AST.
5. PigMacro.java
1)We should restrain from throwing runtime exception
> Integrate Macro Expansion with New Parser
> -----------------------------------------
>
> Key: PIG-1931
> URL: https://issues.apache.org/jira/browse/PIG-1931
> Project: Pig
> Issue Type: Bug
> Components: impl
> Affects Versions: 0.9.0
> Reporter: Richard Ding
> Assignee: Richard Ding
> Fix For: 0.9.0
>
> Attachments: PIG-1931_1.patch
>
>
> Currently Macro expansion is implemented as a preprocessor (PIG-1793) so that
> it can work with the old parser. Now the new parser replaced old parser in
> trunk and we can integrate macro expansion into the new parser. This has many
> advantages such as better error reporting, less code and making Macro part of
> Pig Latin.
> To aid debugging, Pig command line option -r (dryrun) will produce a script
> with expanded macros (in addition to the script with substituted parameters).
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
