HIVE-23980: Shade Guava from hive-exec in Hive 2.3

[viirya]

What changes were proposed in this pull request?

This PR proposes to shade Guava from hive-exec in Hive 2.3 branch.

Why are the changes needed?

When trying to upgrade Guava in Spark, found the following error. A Guava method became package-private since Guava version 20. So there is incompatibility with Guava versions > 19.0.

sbt.ForkMain$ForkError: sbt.ForkMain$ForkError: java.lang.IllegalAccessError: tried to access method com.google.common.collect.Iterators.emptyIterator()Lcom/google/common/collect/UnmodifiableIterator; from class org.apache.hadoop.hive.ql.exec.FetchOperator
at org.apache.hadoop.hive.ql.exec.FetchOperator.<init>(FetchOperator.java:108)
at org.apache.hadoop.hive.ql.exec.FetchTask.initialize(FetchTask.java:87)
at org.apache.hadoop.hive.ql.Driver.compile(Driver.java:541)
at org.apache.hadoop.hive.ql.Driver.compileInternal(Driver.java:1317)
at org.apache.hadoop.hive.ql.Driver.runInternal(Driver.java:1457)
at org.apache.hadoop.hive.ql.Driver.run(Driver.java:1237)
at org.apache.hadoop.hive.ql.Driver.run(Driver.java:1227)

This is a problem for downstream clients. Hive project noticed that problem too in HIVE-22126, however that only targets 4.0.0. It'd be nicer if we can also shade Guava from current Hive versions, e.g. Hive 2.3 line.

Does this PR introduce any user-facing change?

Yes. Guava will be shaded from hive-exec.

How was this patch tested?

Built Hive locally and checked jar content.

[viirya]

cc @sunchao

[sunchao]

LGTM

[viirya]

@sunchao Do you know how to trigger the tests?

[viirya]

@sunchao Thanks for merging it in branch-2. Does this also need to be merged? Or just close it?

[sunchao]

@viirya sorry for the delay - I've cherry-picked the change to branch-2.3 as well so you can just close this.

[viirya]

Ok, thanks @sunchao

[viirya]

Thanks @sunchao for adding jenkins file to branch-2.3. Re-open this.

Reverting the previous approval since it has some issue

[viirya]

@sunchao The CI test results look much better now. Seems to me the failed tests are not related to shading guava.

[sunchao]

Thanks @viirya . Yes agree they do not look like related. I re-triggered CI just to be sure.

[viirya]

Internally we test this patch and pass all Spark tests. I think it gives us more confidence to have this.

[sunchao]

Thanks @viirya ! the new PR looks almost good to me except one nit.

Also comparing to the original patch, we don't have changes to HiveRelDecorrelator, HiveAggregate and HiveSubQueryRemoveRule. This is unnecessary because we've shaded Guava within hive-exec? (some of the APIs like operandJ do not exist in the Calcite version used by branch-2.3 also).

[viirya]

Thanks @viirya ! the new PR looks almost good to me except one nit.

Also comparing to the original patch, we don't have changes to HiveRelDecorrelator, HiveAggregate and HiveSubQueryRemoveRule. This is unnecessary because we've shaded Guava within hive-exec? (some of the APIs like operandJ do not exist in the Calcite version used by branch-2.3 also).

The change to HiveAggregate just to remove unused parameter groupSets in deriveRowType. Not related to shading guava, so I don't apply it.

The change from operand to operandJ in HiveSubQueryRemoveRule and HiveRelDecorrelator, cannot apply to branch-2.3 because operandJ is not in calcite 1.10.0. The API was add since calcite 1.17.0 (https://github.com/apache/calcite/commit/d59b639d/).

[sunchao]

Yes, my main question is whether it is safe to skip the changes on HiveSubQueryRemoveRule and HiveRelDecorrelator. It looks fine to me since we've already shaded calcite within hive/ql.

[viirya]

Yes, my main question is whether it is safe to skip the changes on HiveSubQueryRemoveRule and HiveRelDecorrelator. It looks fine to me since we've already shaded calcite within hive/ql.

Yeah, it should be fine. Calcite uses guava API so shading guava causes no method error if we don't include calcite in shaded jar of hive/ql.

[sunchao]

Thanks again @viirya . This LGTM.

[viirya]

Thank you @sunchao. I think this is ready to go, or we need to wait for a couple days?

[sunchao]

Merged to branch-2.3 and branch-2. Thanks @viirya .

[viirya]

Thank you @sunchao