Cheolsoo, I updated patch to address your comments in RB. Johnny, my replies to your questions are at the bottom of this comment.
There was a comment where you noted that ParamLoader was being called repeatedly. To address this, and a lot of other messiness with param substitution, I did a refactoring so all methods that substitute parameters call PigContext's doParamSubstitution(), which instantiates and manages ParamSubstitutionPreprocessor. This ensures consistent behavior between the param substitution called in Main, GruntParser, and several other places, and solves the ParamLoader problem. Managed by PigContext, the loading of parameters is separated from substitution itself, is not repeated when substitution is done many times (i.e. for macros).
As for registerJars and registerCode, I got stuck. I understand your concern about separating parser and server. But QPD returns a LogicalPlan, and I'm pretty sure that the LP generation needs the UDF output schemas to be defined, which requires the UDFs to be registered. I think this could be fixed by splitting the AST generation from the LP generation and allowing PigServer to register resources requested by macros in between the two. But that seems like a big change and I don't want the scope of this patch to expand too much.
What do you think?
To answer your questions:
1) The query parser seems to be reparsing everything every time it reads a new line of Pig code. So the same macros are parsed over and over. This patch caches the AST's from the first parsing, so when a new line of the Pig script is parsed, the macro is not re-parsed. Also in terms of actually fetching the files, the import sequence goes "test.pig" imports "macros_1.pig" imports "macros_2.pig"; when "test.pig" imports "macros_2.pig" in the next line, it has already been acquired, so it is not loaded. Same idea for udfs, jars. I'm not sure how best to test this within Pig without a mocking framework to intercept when QPD parses a macro and ensure it is not doing it repeatedly. Any ideas?
2) It should not, since param substitution in the main pigscript happens before macro expansion. This is just to propagate the parameters to imported macro files.
3) duplicatedImportTest is not supposed to fail; the duplicated import should just be ignored. This is silly within one pigscript, but the it makes sense in the test.pig/macros_1.pig/macros_2.pig example I described above, since both test and macros_1 should be able to import macros_2.