-
Notifications
You must be signed in to change notification settings - Fork 28k
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.
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
[SPARK-24930][SQL] Improve exception information when using LOAD DATA LOCAL INPATH #21881
Conversation
ok to test |
Test build #93622 has finished for PR 21881 at commit
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just nits about exception msg.
throw new AnalysisException(s"LOAD DATA input path does not exist: $path") | ||
// If user have no permission to access the given input path, `File.exists()` return false | ||
// , `LOAD DATA input path does not exist` can confuse users. | ||
throw new AnalysisException(s"LOAD DATA input path does not exist: `$path` or current " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: no need to print the $path twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, Thanks!
Test case? |
@gatorsmile Hi, i am not sure how to build this scene in test case, just assert whether the exception info contains the key message |
Can one of the admins verify this patch? |
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
What changes were proposed in this pull request?
In fact, the input path exists, but the mr user does not have permission to access the directory
/root/
,so the message throwed byAnalysisException
can confuse user.How was this patch tested?
existing test case
Please review http://spark.apache.org/contributing.html before opening a pull request.