You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by "Johannes Schwenk (JIRA)" <ji...@apache.org> on 2012/05/30 15:41:23 UTC
[jira] [Created] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Johannes Schwenk created PIG-2729:
-------------------------------------
Summary: Macro expansion does not use pig.import.search.path - UnitTest borked
Key: PIG-2729
URL: https://issues.apache.org/jira/browse/PIG-2729
Project: Pig
Issue Type: Bug
Affects Versions: 0.10.0, 0.9.2
Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
Reporter: Johannes Schwenk
org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
import 'mytest2.pig';
and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Johannes Schwenk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Johannes Schwenk updated PIG-2729:
----------------------------------
Attachment: PIG-2729.patch
Hi Rohini,
I attached a new patch that fixes the issue with relative paths. It also includes the patch for TestMacroExpansion. Further I removed the redundant code in QueryParserUtils. The tests in TestQueryParser and TestMacroExpansion all succeed. All tests should be run though, to verify that I have not broken other things...
As to the failing importTwoFilesTest I mentioned earlier: I cannot reproduce this right now. Could you verify that the tests work as expected Rohini?
Thanks,
Johannes
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: PIG-2729.patch, test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Rohini Palaniswamy (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13422496#comment-13422496 ]
Rohini Palaniswamy commented on PIG-2729:
-----------------------------------------
Thanks Johannes. My +1.
I will ask Daniel to review and commit it. Will also ask you to add to the contributors list in jira so that this jira can be assigned to you.
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: PIG-2729.patch, PIG-2729.patch, PIG-2729.patch, PIG-2729.patch, test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Rohini Palaniswamy (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13416394#comment-13416394 ]
Rohini Palaniswamy commented on PIG-2729:
-----------------------------------------
Johannes,
You need to do "git diff --no-prefix" to get the patch so that it can be applied on svn. And it would be easier to review if you can also post the patch in reviewboard.
1) Small nitpick. The paths variable need not be defined outside the if block.
{code}
String[] paths = {};
+ if (srchPath != null) {
+ paths = srchPath.split(","); //Could just be String[] paths = srchPath.split(",");
{code}
2) The problem I mentioned in the previous comment about "../" still exists. You have also removed the f.exists() || f.isAbsolute() || scriptPath.startsWith("./") checks. They are required. Now the search will be looking at the wrong paths even if the file existed or was an absolute path and makes the behavior unpredictable. For eg: If there was a statement, import '/dir1/file1.pig' and the pig.import.search.path was '/dir2,/dir3', then you would be considering files /dir2/dir1/file1.pig and /dir3/dir1/file1.pig. The same thing will happen for relative paths from base dir and ./ and ../.
3) Doing a canonical path for f.exists() is not required. canonicalize results internally in a native call and would just add overhead.
Thanks,
Rohini
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: PIG-2729.patch, test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Johannes Schwenk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Johannes Schwenk updated PIG-2729:
----------------------------------
Attachment: use-search-path-for-imports.patch
I am sorry for all the noise!
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Daniel Dai (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Daniel Dai updated PIG-2729:
----------------------------
Resolution: Fixed
Assignee: Johannes Schwenk
Hadoop Flags: Reviewed
Status: Resolved (was: Patch Available)
Patch committed to 0.10 branch. Thanks Johannes, Rohini!
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Assignee: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: PIG-2729.patch, PIG-2729.patch, PIG-2729.patch, PIG-2729.patch, test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Johannes Schwenk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13416975#comment-13416975 ]
Johannes Schwenk commented on PIG-2729:
---------------------------------------
Hi Rohini,
sorry I didn't know about svn having problems with git diffs - I will fix that and post patches on reviewboard from now on...
1. Will fix that.
2. Sorry, I think I missunderstood you: I somehow thought that you wanted /x/y/../readme.pig beeing resolved to /x/readme.pig for "import '../readme.pig'; if the import path contained /x/y. This is obvoiusly not the case... So the correct behaviour is:
1. Check if fname points to an existing file, is absolute, or relative
yes => return localFileRet
no => goto 2.
2. For each importPath in pig.import.search.path
- concatenate: importPath + File.separator + fname
- check if file exists
yes => return localFileRet
no => continue with 2.
3. Return localFileRet for fname.
4. While doing 1.-3. throw RuntimeException if an IOException was encountered
Have I understood this correctly?
3. Yes, i removed this.
Thanks,
Johannes
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: PIG-2729.patch, test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Johannes Schwenk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Johannes Schwenk updated PIG-2729:
----------------------------------
Attachment: PIG-2729.patch
I included your corrections and good advice. I think the patch has evolved a lot thanks to you Rohini!
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: PIG-2729.patch, PIG-2729.patch, PIG-2729.patch, PIG-2729.patch, test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Rohini Palaniswamy (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13419343#comment-13419343 ]
Rohini Palaniswamy commented on PIG-2729:
-----------------------------------------
Few comments:
1) Exception should not be thrown here as it will break Amazon s3 filesystem support.
{code}
File macroFile = QueryParserUtils.getFileFromSearchImportPath(fname);
+ if (macroFile == null) {
+ throw new FileNotFoundException("Could not find the specified file '"
+ + fname + "' using import search path");
+ }
+ localFileRet = FileLocalizer.fetchFile(pigContext.getProperties(),
+ macroFile.getAbsolutePath());
{code}
It should be
{code}
File localFile = QueryParserUtils.getFileFromSearchImportPath(fname);
localFileRet = localFile == null
? FileLocalizer.fetchFile(pigContext.getProperties(), fname)
: new FetchFileRet(localFile.getCanonicalFile(), false);
{code}
The reason is the macro path could be fully qualified s3 or some other supported file system path. So if we could not find it in the local filesystem with getFileFromSearchImportPath, then FileLocalizer.fetchFile will take care of looking at other filesystems and downloading it locally and returning the local file path. Also it will throw the FileNotFoundException if the file is missing.
2. Again for the same reason of s3 support, it is incorrect to use getFileFromSearchImportPath in this code. And getMacroFile already fetches the file.
{code}
FetchFileRet localFileRet = getMacroFile(fname);
File macroFile = QueryParserUtils.getFileFromSearchImportPath(
+ localFileRet.file.getAbsolutePath());
try {
- in = QueryParserUtils.getImportScriptAsReader(localFileRet.file.getAbsolutePath());
+ in = new BufferedReader(new FileReader(macroFile));
{code}
should be
{code}
in = new BufferedReader(new FileReader(localFileRet.file));
{code}
3. For the tests, can you extract out the common code to a method to cut down on the repetition of code. Something like
{code}
importUsingSearchPathTest() {
verifyImportUsingSearchPath("/tmp/mytest2.pig", "mytest2.pig", "/tmp");
}
importUsingSearchPathTest2() {
verifyImportUsingSearchPath("/tmp/mytest2.pig", "./mytest2.pig", "/tmp");
}
importUsingSearchPathTest3() {
verifyImportUsingSearchPath("/tmp/mytest2.pig", "../mytest2.pig", "/tmp");
}
importUsingSearchPathTest4() {
verifyImportUsingSearchPath("/tmp/mytest2.pig", "/tmp/mytest2.pig", "/foo/bar");
}
verifyImportUsingSearchPath(String macroFilePath, String importFilePath, String importSearchPath) {
.....
}
{code}
4) negtiveUsingSearchPathTest2 and 3 are not very useful, unless some file with same name and garbage text are created in the search path location. That way we can ensure that the right file is being picked up and not the other file.
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: PIG-2729.patch, PIG-2729.patch, test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Rohini Palaniswamy (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13417313#comment-13417313 ]
Rohini Palaniswamy commented on PIG-2729:
-----------------------------------------
No worries. For future reference, https://cwiki.apache.org/confluence/display/PIG/HowToContribute has all the instructions. Its a really nice writeup and I had found it very helpful.
You are right with 1 and 2. It makes it same as the old logic. I was thinking it would keep it simpler if instead of copying all the logic to getMacroFile method, we could just change the name of QueryParserUtil.getImportScriptAsReader to something like getFileFromSearchImportPath and make it return a file instead of bufferedReader and null instead of FileNotFoundException. That way the logic remains exactly same and we don't have to worry about missing something.
{code}
QueryParserUtils.java
- static BufferedReader getImportScriptAsReader(String scriptPath)
- throws FileNotFoundException {
+
+ static File getFileFromSearchImportPath(String scriptPath) {
- return new BufferedReader(new FileReader(f));
+ return f;
- return new BufferedReader(new FileReader(f1));
+ return f1;
-
- throw new FileNotFoundException("Can't find the Specified file "
- + scriptPath);
+ return null;
QueryParserDriver.java getMacroFile():
File localFile = QueryParserUtils.getFileFromSearchImportPath(fname);
localFileRet = localFile == null
? FileLocalizer.fetchFile(pigContext.getProperties(), fname)
: new FetchFileRet(localFile.getCanonicalFile(), false);
{code}
And I checked. importTwoFilesTest still fails. I had to make mytest1.pig and mytest2.pig to mytest4.pig and mytest5.pig to get it to pass. The cleaner thing to do would be to clear the cache in QueryParserDriver before each test. But I think this should be ok for now as the cache is private.
You can verify your patch by running
ant -Djavac.args="-Xlint -Xmaxwarns 1000" clean jar-withouthadoop test -Dtestcase=TestMacroExpansion -logfile /tmp/log
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: PIG-2729.patch, test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Daniel Dai (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13426930#comment-13426930 ]
Daniel Dai commented on PIG-2729:
---------------------------------
Committed to trunk as well.
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Assignee: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: PIG-2729.patch, PIG-2729.patch, PIG-2729.patch, PIG-2729.patch, test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Rohini Palaniswamy (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rohini Palaniswamy updated PIG-2729:
------------------------------------
Fix Version/s: (was: 0.10.0)
0.10.1
0.11
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Assignee: Johannes Schwenk
> Fix For: 0.11, 0.10.1
>
> Attachments: PIG-2729.patch, PIG-2729.patch, PIG-2729.patch, PIG-2729.patch, test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Johannes Schwenk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Johannes Schwenk updated PIG-2729:
----------------------------------
Status: Open (was: Patch Available)
Sorry forgot patch! Will resubmit...
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.10.0, 0.9.2
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: test-macros.tar.gz
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Rohini Palaniswamy (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13420992#comment-13420992 ]
Rohini Palaniswamy commented on PIG-2729:
-----------------------------------------
No issues. Even I am new to pig :). I was just applying what I learnt from the previous jira that I was working on about s3 support.
I uploaded a patch in review board yesterday and it works fine. Not sure what problem you are facing.
The patch looks good. Still have few comments though. Won't bother you more. These are the last ones from me :).
1) Can we add createFile("/tmp/mytest2.pig", garbageMacroContent); as the first line in importUsingSearchPathTest, importUsingSearchPathTest2 and importUsingSearchPathTest3 and createFile("/foo/bar/tmp/mytest2.pig", garbageMacroContent); in importUsingSearchPathTest4. Just an additional way to ensure the right file is being picked up.
2) Delete the mytest3.pig file in negativeUsingSearchPathTest, just in case negativeUsingSearchPathTest2 is executed first and a garbage mytest3.pig file is created. ie:
{code}
public void negativeUsingSearchPathTest() throws Exception {
new File("mytest3.pig").delete();
assertFalse(verifyImportUsingSearchPath("/tmp/mytest3.pig", "mytest3.pig", null));
}
{code}
3) Use Assert.assertFalse instead of assertTrue(!verifyImportUsingSearchPath(..)) in negativeTests.
4) Minor nitpick. It would be nice to declare the static variables (groupAndCountMacro and garbageMacroContent) in the beginning of the class. Not a big deal though.
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: PIG-2729.patch, PIG-2729.patch, PIG-2729.patch, test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Johannes Schwenk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Johannes Schwenk updated PIG-2729:
----------------------------------
Status: Patch Available (was: Open)
This should fix the issue. I ran the tests in org.apache.pig.test.TestMacroExpansion against this. Something is really borked here, since the tests seem to be depending on each other. The (fixed, see above) test importUsingSearchPathTest now completes successfully. The test importTwoFilesTest fails however, when executed together with the other tests. but also succeeds if run as the only test in the class. Renaming mytest1.pig and mytest2.pig in the test to something different made it pass also, so it might be a cleanup issue.
So this patch should definitely work, but the tests for the class should be reworked and included in the test/unit-tests file. I will open a separate issue on that.
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.10.0, 0.9.2
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: test-macros.tar.gz
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Johannes Schwenk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Johannes Schwenk updated PIG-2729:
----------------------------------
Attachment: test-macros.tar.gz
Attached scripts to further illustrate this issue.
To test this extract to your home. Then execute ./run-me .
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Attachments: test-macros.tar.gz
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Rohini Palaniswamy (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13415694#comment-13415694 ]
Rohini Palaniswamy commented on PIG-2729:
-----------------------------------------
Johannes,
Do you have the updated patch which fixes the unit tests also? The static map in QueryParserDriver which caches the macro files is the reason for the test failure once the importUsingSearchPathTest is corrected.
{code}
String srchPath = pigContext.getProperties().getProperty("pig.import.search.path");
+ if (!fname.startsWith("/") && !fname.startsWith(".") && srchPath != null) {
+ String[] paths = srchPath.split(",");
+ for (String path : paths) {
+ String resolvedPath = path + File.separator + fname;
+ if ((new File(resolvedPath)).exists()) {
+ fname = resolvedPath;
+ break;
+ }
+ }
+ }
{code}
The above code also does not handle "../". If the script had "import ../readme.pig" and pig.import.search.path was "/x/y", then /x/y/../readme.pig will be searched for. Also need to remove the redundant QueryParserUtils.getImportScriptAsReader code as getMacroFile will now return a absolute path.
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Johannes Schwenk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Johannes Schwenk updated PIG-2729:
----------------------------------
Fix Version/s: 0.10.0
Release Note: Import search path property pig.import.search.path is now correctly used...
Status: Patch Available (was: Open)
This should fix the issue. I ran the tests in org.apache.pig.test.TestMacroExpansion against this. Something is really borked here, since the tests seem to be depending on each other. The (fixed, see above) test importUsingSearchPathTest now completes successfully. The test importTwoFilesTest fails however, when executed together with the other tests. but also succeeds if run as the only test in the class. Renaming mytest1.pig and mytest2.pig in the test to something different made it pass also, so it might be a cleanup issue.
So this patch should definitely work, but the tests for the class should be reworked and included in the test/unit-tests file. I will open a separate issue on that.
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.10.0, 0.9.2
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: test-macros.tar.gz
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Johannes Schwenk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Johannes Schwenk updated PIG-2729:
----------------------------------
Attachment: PIG-2729.patch
Hi Rohini,
I changed the PIG to your suggestion. I would post this on the review board, but I currently get an Error 500 everytime I try to submit. Test cases in TestMacroExpansion all succeed. Could you take a look again please?
Thanks,
Johannes
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: PIG-2729.patch, PIG-2729.patch, test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Johannes Schwenk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Johannes Schwenk updated PIG-2729:
----------------------------------
Attachment: PIG-2729.patch
Hi Rohini,
thank you very much for your comments! I am still new to the project so some things slip my attention - your advice and patience are much appreciated!
I attached the new patch incorporating your corrections. Could you take a look again?
On a side note: Do you know what is the matter with reviews.apache.org? I still always get the "Error 500" message when I try to submit my patch for review.
Thanks!
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: PIG-2729.patch, PIG-2729.patch, PIG-2729.patch, test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (PIG-2729) Macro expansion does not use
pig.import.search.path - UnitTest borked
Posted by "Johannes Schwenk (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/PIG-2729?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13422978#comment-13422978 ]
Johannes Schwenk commented on PIG-2729:
---------------------------------------
Thanks Rohini!
Meanwhile I was able to create a review request after a hint on the mailing list.
https://reviews.apache.org/r/6150/
> Macro expansion does not use pig.import.search.path - UnitTest borked
> ---------------------------------------------------------------------
>
> Key: PIG-2729
> URL: https://issues.apache.org/jira/browse/PIG-2729
> Project: Pig
> Issue Type: Bug
> Affects Versions: 0.9.2, 0.10.0
> Environment: pig-0.9.2 and pig-0.10.0, hadoop-0.20.2 from Clouderas distribution cdh3u3 on Kubuntu 12.04 64Bit.
> Reporter: Johannes Schwenk
> Fix For: 0.10.0
>
> Attachments: PIG-2729.patch, PIG-2729.patch, PIG-2729.patch, PIG-2729.patch, test-macros.tar.gz, use-search-path-for-imports.patch
>
>
> org.apache.pig.test.TestMacroExpansion, in function importUsingSearchPathTest the import statement is provided with the full path to /tmp/mytest2.pig so the pig.import.search.path is never used. I changed the import to
> import 'mytest2.pig';
> and ran the UnitTest again. This time the test failed as expected from my experience from earlier this day trying in vain to get pig eat my pig.import.search.path property! Other properties in the same custom properties file (provided via -propertyFile command line option) like udf.import.list get read without any problem.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira