cnauroth commented on code in PR #3839:
URL: https://github.com/apache/hive/pull/3839#discussion_r1042642131
##########
ql/pom.xml:
##########
@@ -1113,6 +1113,12 @@
<exclude>META-INF/licenses/slf4j*/**</exclude>
</excludes>
</filter>
+ <filter>
+ <artifact>com.zaxxer:HikariCP</artifact>
+ <excludes>
+ <exclude>META-INF/versions/11/module-info.class</exclude>
+ </excludes>
+ </filter>
Review Comment:
The new version of HikariCP starts declaring itself as a Jigsaw module. When
shading this dependency, the default behavior is to bring in all of the files,
including the compiled module declaration. With that, hive-exec.jar would be
declaring itself as the HikariCP module, which wouldn't be correct:
```
> jar xf ~/git/apache/hive/ql/target/hive-exec-4.0.0-SNAPSHOT.jar
> javap -l -p -c -s META-INF/versions/11/module-info.class
Compiled from "module-info.java"
module [email protected] {
requires java.base;
requires java.sql;
requires java.management;
requires java.naming;
requires org.slf4j;
requires static org.hibernate.orm.core;
requires static simpleclient;
requires static metrics.core;
requires static metrics.healthchecks;
requires static micrometer.core;
exports com.zaxxer.hikari;
exports com.zaxxer.hikari.hibernate;
exports com.zaxxer.hikari.metrics;
exports com.zaxxer.hikari.metrics.dropwizard;
exports com.zaxxer.hikari.metrics.micrometer;
exports com.zaxxer.hikari.metrics.prometheus;
exports com.zaxxer.hikari.pool;
exports com.zaxxer.hikari.util;
}
```
Hadoop has needed to apply similar filter rules on some of the dependencies
it shades:
https://github.com/apache/hadoop/blob/rel/release-3.3.4/hadoop-client-modules/hadoop-client-runtime/pom.xml#L245-L250
Hmmm... on a side note, Hive shades Jackson too, so I wonder if we have an
existing problem on that one. I'll follow up on that later.
In terms of practical problems, I think a conflict could only arise if
running hive-exec.jar in a JVM with module support (Java 9+) and actually
attempting to use module resolution. That's an edge case for sure, but I think
it's best to omit the incorrect module-info.class.
While reviewing this, I also realized that the same problem applies to a few
of our other shaded modules: beeline and jdbc. I've updated the patch to add
the filter rules there too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
