Skip to content

Fix Error: java.lang.IllegalArgumentException: Can not create a Path from an empty string #1126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yanghua merged 1 commit into from
Dec 24, 2019
Merged

Fix Error: java.lang.IllegalArgumentException: Can not create a Path from an empty string #1126

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion hudi-client/src/main/java/org/apache/hudi/table/HoodieCopyOnWriteTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ private static PairFlatMapFunction<Iterator<Tuple2<String, String>>, String, Par
Tuple2<String, String> partitionDelFileTuple = iter.next();
String partitionPath = partitionDelFileTuple._1();
String delFileName = partitionDelFileTuple._2();
Path deletePath = new Path(new Path(basePath, partitionPath), delFileName);
Path deletePath = FSUtils.getPartitionPath(FSUtils.getPartitionPath(basePath, partitionPath), delFileName);
Copy link
Member

@vinothchandar vinothchandar Dec 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this simply need a new Path(FSUtils.getPartitionPath(basePath, partitionPath), delFileName) ? We are overloading calling of FSUtils.getPartitionPath. for somethings that's not a partitionpath?

Also can we please ensure there is a JIRA for fix.. or we follow the [MINOR] ... convention?

@cdmikechen @yanghua wdyt

Copy link
Contributor

@yanghua yanghua Dec 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this simply need a new Path(FSUtils.getPartitionPath(basePath, partitionPath), delFileName) ? We are overloading calling of FSUtils.getPartitionPath. for somethings that's not a partitionpath?

I saw FsUtils#getPartitionPath can only avoid the second arg to be null. However, the outer new Path(..., delFileName); The delFileName can not be null?

Also can we please ensure there is a JIRA for fix.. or we follow the [MINOR] ... convention?

Agree, I have mentioned in the approvement comment. Not sure whether [MINOR] is a normal prefix if there is not a Jira issue to track. IMO, We should clearly record where. Or does it exist but I don't know?

Copy link
Contributor Author

@cdmikechen cdmikechen Dec 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vinothchandar
If by name, it should be better to just modifyFSUtils.getPartitionPath(basePath, partitionPath). Maybe I should revert and put another PR?
In hadoop 2.7+, I found that new Path() api use more stringent checks than ever before, if user use null to use new Path(path, null) in hadoop2.7-, it doesn't report error.
If it's for API compatibility, is it better to adjust its name to buildMultiPath() or else?So that all similar methods can be used, thus to ensure that no exception will happen.

Copy link
Contributor Author

@cdmikechen cdmikechen Dec 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vinothchandar
Yeah, I will open a JIRA issue first next time and then decide whether to submit PR according to the question.
If by name, it should be better to just modifyFSUtils.getPartitionPath(basePath, partitionPath). Maybe I should revert and put another PR?
In hadoop 2.7+, I found that new Path() api use more stringent checks than ever before, if user use null to use new Path(path, null) in hadoop2.7-, it doesn't report error.
If it's for API compatibility, is it better to adjust its name to buildMultiPath() or else?So that all similar methods can be used, thus to ensure that no exception will happen.

Copy link
Member

@vinothchandar vinothchandar Jan 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can do a follow up small fix here.. confirm delFileName cannot be null and then remove the outer FSUtils.getPartitionPath. This one can have [MINOR] prefix IMO

String deletePathStr = deletePath.toString();
Boolean deletedFileResult = deleteFileAndGetResult(fs, deletePathStr);
if (!partitionCleanStatMap.containsKey(partitionPath)) {
Expand Down