-
Notifications
You must be signed in to change notification settings - Fork 240
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
Upgrade ORC version from 1.5.8 to 1.5.10 #4051
Conversation
Signed-off-by: Chong Gao <res_life@163.com>
build |
|
integration_tests/pom.xml
Outdated
@@ -211,32 +211,6 @@ | |||
<classifier>${spark.version.classifier}</classifier> | |||
</configuration> | |||
</plugin> | |||
<plugin> |
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.
@pxLi and @tgravescs git blame indicates that the two of you touched this. It looks like it was added in on purpose so we get version info in both jars. Is that correct?
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.
Yes this is to make sure all jars have version properties which we then use later to make sure all the jars we pull are built from same version of source code, please put it back.
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.
Reverted.
so can we have a description on this as to why we are upgrading and why did we chose 1.5.12 when Spark 3.0.1 support 1.5.10? It probably doesn't matter anymore since we are shading it, but if it doesn't matter why aren't we upgrading to newer? |
Signed-off-by: Chong Gao <res_life@163.com>
Signed-off-by: Chong Gao <res_life@163.com>
The default "buildver" version is 3.0.1, so let's use 1.5.10. The purpose of upgrading ORC I can see is to keep consistent when the "buildver" is 3.0.1. |
build |
@@ -1224,7 +1224,7 @@ private case class GpuOrcFileFilterHandler( | |||
val searchArg = readerOpts.getSearchArgument | |||
if (searchArg != null && orcReader.getRowIndexStride != 0) { | |||
val sa = new SargApplier(searchArg, orcReader.getRowIndexStride, evolution, | |||
orcReader.getWriterVersion, useUTCTimestamp) | |||
orcReader.getWriterVersion, useUTCTimestamp, false, false) |
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.
Actually on second thought, we shouldn't be passing false, false
here. We should be following what ORC 1.5.10 is doing here which is looking up the parameters from the reader and options.
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.
Updated and added a test case.
Signed-off-by: Chong Gao <res_life@163.com>
build |
This fixes #3227
Signed-off-by: Chong Gao res_life@163.com