-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-16528][SQL] Fix NPE problem in HiveClientImpl #14200
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
Conversation
|
test this please |
There are some calls to methods or fields (getParameters, properties) which are then passed to Java/Scala collection converters. Unfortunately those fields can be null in some cases and then the conversions throws NPE. We fix it by wrapping calls to those fields and methods with option and then do the conversion.
667abc0 to
49c5451
Compare
|
OK by me |
|
Test build #62310 has finished for PR 14200 at commit
|
|
Test build #62311 has finished for PR 14200 at commit
|
|
LGTM - merging in master/2.0. |
## What changes were proposed in this pull request? There are some calls to methods or fields (getParameters, properties) which are then passed to Java/Scala collection converters. Unfortunately those fields can be null in some cases and then the conversions throws NPE. We fix it by wrapping calls to those fields and methods with option and then do the conversion. ## How was this patch tested? Manually tested with a custom Hive metastore. Author: Jacek Lewandowski <[email protected]> Closes #14200 from jacek-lewandowski/SPARK-16528. (cherry picked from commit 31ca741) Signed-off-by: Reynold Xin <[email protected]>
| description = d.getDescription, | ||
| locationUri = d.getLocationUri, | ||
| properties = d.getParameters.asScala.toMap) | ||
| properties = Option(d.getParameters).map(_.asScala.toMap).orNull) |
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.
Is Map.empty a better default value? Or we should update the properties field in CatalogDatabase to indicate that it's nullable.
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.
Perhaps... however this would change the semantics which was out of the scope of this ticket.
What changes were proposed in this pull request?
There are some calls to methods or fields (getParameters, properties) which are then passed to Java/Scala collection converters. Unfortunately those fields can be null in some cases and then the conversions throws NPE. We fix it by wrapping calls to those fields and methods with option and then do the conversion.
How was this patch tested?
Manually tested with a custom Hive metastore.