-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-26319][SQL][Test] Add appendReadColumns Unit Test for HiveShimSuite #23268
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
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveShim.scala
Outdated
Show resolved
Hide resolved
sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveShim.scala
Outdated
Show resolved
Hide resolved
|
Let's close this one. |
|
@sadhen What is the motivation of this PR? |
|
Nevermind, just do not like the coding style personally. |
|
Let us avoid unnecessary code refactoring, but your test case is welcomed. |
|
Yup, the test looks okay in that way. Let's file a JIRA and only leave the test case. |
|
OK, I will modify the PR several hours later. |
|
Test build #99895 has finished for PR 23268 at commit
|
This reverts commit b68e7b1.
|
@HyukjinKwon Please re-review. |
HyukjinKwon
left a comment
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.
Looks fine.
|
Let's fix PR description as well. You can leave the comments above resolved. |
|
@HyukjinKwon I've updated the desc. |
|
Test build #99904 has finished for PR 23268 at commit
|
|
Merged to master. |
…Suite ## What changes were proposed in this pull request? Add appendReadColumns Unit Test for HiveShimSuite. ## How was this patch tested? ``` $ build/sbt > project hive > testOnly *HiveShimSuite ``` Closes apache#23268 from sadhen/refactor/hiveshim. Authored-by: Darcy Shen <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]>
What changes were proposed in this pull request?
Add appendReadColumns Unit Test for HiveShimSuite.
How was this patch tested?