voonhous commented on code in PR #19800:
URL: https://github.com/apache/hudi/pull/19800#discussion_r3920958947
##########
packaging/hudi-utilities-bundle/pom.xml:
##########
@@ -158,6 +158,8 @@
<include>org.apache.hive:hive-service-rpc</include>
<include>org.apache.hive:hive-metastore</include>
<include>org.apache.hive:hive-jdbc</include>
+ <!-- MapredParquetInputFormat, superclass of the bundled
hudi-hadoop-mr input formats -->
Review Comment:
**major:** #16374 also names `utilities-slim-bundle`, and that half is a
`hudi-spark-bundle` gap this PR leaves open. The slim bundle carries no
`hudi-hadoop-mr` or Hive artifacts (its `utilities-bundle-shade-hive` profile
only flips a relocation prefix) and runs together with the spark bundle
(`packaging/bundle-validation/validate.sh:405`), whose
`spark-bundle-shade-hive` profile whitelists `hudi-hadoop-mr` (`pom.xml:82`)
and the same five Hive artifacts (`pom.xml:114-118`) with no hive-exec, so it
hits the identical CNFE. Could we mirror the fix there in this PR, or drop
"Closes #16374"?
##########
packaging/hudi-utilities-bundle/pom.xml:
##########
@@ -158,6 +158,8 @@
<include>org.apache.hive:hive-service-rpc</include>
<include>org.apache.hive:hive-metastore</include>
<include>org.apache.hive:hive-jdbc</include>
+ <!-- MapredParquetInputFormat, superclass of the bundled
hudi-hadoop-mr input formats -->
+ <include>org.apache.hive:hive-exec</include>
Review Comment:
**major:** This include ships 5,109 un-relocated
`org.apache.hadoop.hive.ql.*` classes whose `HiveConf` references were
rewritten: on the built profile jar, `javap` shows
`SessionState(org.apache.hudi.org.apache.hadoop.hive.conf.HiveConf)` and
`Hive.get(org.apache.hudi.org.apache.hadoop.hive.conf.HiveConf)`. Spark's
`HiveClientImpl` calls both with the canonical `HiveConf`, so wherever the
bundle's copy wins (`spark.driver.userClassPathFirst`, bundle dropped into
`$SPARK_HOME/jars`) it is a `NoSuchMethodError` -- the exact failure #961,
which added this profile, cited for turning Hive shading off by default. If
hive-exec stays, could a shade `<filter>` on `org.apache.hive:hive-exec` keep
only `org/apache/hadoop/hive/ql/io/parquet/**` and
`org/apache/hadoop/hive/ql/exec/vector/VectorizedInputFormatInterface.class`,
so `SessionState`/`Hive`/`Driver` never ship?
##########
packaging/hudi-utilities-bundle/pom.xml:
##########
@@ -355,6 +357,31 @@
<scope>${utilities.bundle.hive.scope}</scope>
</dependency>
+ <!--
+ Supplies org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat,
which is the superclass of
Review Comment:
**major:** Is bundling hive-exec the right layer for this fix? Every caller
outside `hudi-hadoop-mr` (`HiveSyncTool`, `HoodieHiveSyncClient`,
`AdbSyncTool`, `DataHubTableProperties`, `CreateHoodieTableCommand`,
`HoodieHiveCatalog`) consumes the three names as plain strings, and
`hudi-trino/.../HudiTableTypeUtils.java:22-23` already hardcodes them. If
`HoodieInputFormatUtils.get{InputFormat,OutputFormat,SerDe}ClassName` returned
string constants, no Hive class would load at sync time: that closes #16374 for
the utilities and spark bundles under both shade-hive profiles with no
bundling, no un-relocated `ql` classes and no size growth, and it lines up with
#15841 (filed after #8147, the previous report of this exact CNFE), which
pushes the other way on shipping hive-exec. Would you consider that instead?
##########
packaging/hudi-utilities-bundle/pom.xml:
##########
@@ -355,6 +357,31 @@
<scope>${utilities.bundle.hive.scope}</scope>
</dependency>
+ <!--
+ Supplies org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat,
which is the superclass of
+ HoodieParquetInputFormatBase in hudi-hadoop-mr, which is whitelisted in
the shade artifactSet above. Without
+ this, the bundle ships those subclasses with their superclass missing
and any use of the Hive input formats fails
+ with ClassNotFoundException on MapredParquetInputFormat (HUDI-7321).
+
+ The core classifier is what the rest of the build uses
(hive.exec.classifier): it carries Hive's own
+ classes without Hive's shaded copies of third-party libraries, so it
does not drag protobuf, guava and
+ friends into this bundle. Scope follows utilities.bundle.hive.scope like
the hive artifacts above, so it
+ is provided by default and only packaged under
-Putilities-bundle-shade-hive.
Review Comment:
**minor:** Not blocking. Nothing builds this profile in CI (`git grep
shade-hive` hits only four poms; the `bot.yml` bundle builds pass no `-P`, and
`validate.sh` runs no Hive sync), so no CI run has ever exercised it and a
green build says nothing about this change -- while the commons-io downgrade
below ships in the default jar CI does build. Would a one-line note in
`packaging/README.md` on what `-Putilities-bundle-shade-hive` promises, plus a
jar-content assertion for `MapredParquetInputFormat` in bundle validation under
the profile, be worth adding here?
##########
packaging/hudi-utilities-bundle/pom.xml:
##########
@@ -158,6 +158,8 @@
<include>org.apache.hive:hive-service-rpc</include>
<include>org.apache.hive:hive-metastore</include>
<include>org.apache.hive:hive-jdbc</include>
+ <!-- MapredParquetInputFormat, superclass of the bundled
hudi-hadoop-mr input formats -->
+ <include>org.apache.hive:hive-exec</include>
Review Comment:
**minor:** Not blocking. `hive-exec:core` also contributes
`META-INF/services/org.apache.hadoop.fs.FileSystem` listing
`ProxyLocalFileSystem`, which does not override `getScheme()`, so ServiceLoader
rebinds `file://` to it: on the profile jar with a Hadoop-only classpath,
`FileSystem.get(file:///)` returns `ProxyLocalFileSystem`, whose `rename`
returns false when the destination exists. Any Spark-with-Hive classpath
already behaves this way, but it is new for the hive-less one this profile
targets. Could the hive-exec `<filter>` also drop that services file?
##########
packaging/hudi-utilities-bundle/pom.xml:
##########
@@ -355,6 +357,31 @@
<scope>${utilities.bundle.hive.scope}</scope>
</dependency>
+ <!--
+ Supplies org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat,
which is the superclass of
+ HoodieParquetInputFormatBase in hudi-hadoop-mr, which is whitelisted in
the shade artifactSet above. Without
+ this, the bundle ships those subclasses with their superclass missing
and any use of the Hive input formats fails
+ with ClassNotFoundException on MapredParquetInputFormat (HUDI-7321).
+
+ The core classifier is what the rest of the build uses
(hive.exec.classifier): it carries Hive's own
+ classes without Hive's shaded copies of third-party libraries, so it
does not drag protobuf, guava and
+ friends into this bundle. Scope follows utilities.bundle.hive.scope like
the hive artifacts above, so it
+ is provided by default and only packaged under
-Putilities-bundle-shade-hive.
+ -->
+ <dependency>
+ <groupId>${hive.groupid}</groupId>
+ <artifactId>hive-exec</artifactId>
Review Comment:
**blocker:** The sync path this targets still fails one call later. On the
profile jar with a Hadoop-only classpath, `getInputFormatClassName` and
`getOutputFormatClassName` now resolve, but `getSerDeClassName(PARQUET)`
(`HiveSyncTool.java:437`, `HoodieHiveSyncClient.java:396`) throws
`NoClassDefFoundError: org/apache/hadoop/hive/serde2/AbstractSerDe`:
`ParquetHiveSerDe`'s superclass lives in `hive-serde`, which the `core`
classifier omits (0 `serde2/` entries in the core jar; the uber jar the
reporter used has 578) and this bundle does not whitelist. Should `hive-serde`
be added the same way (with the wildcard exclusion), with a smoke check that
covers all three `HiveSyncTool` calls -- or would the string-constant route in
the thread above make both unnecessary?
##########
packaging/hudi-utilities-bundle/pom.xml:
##########
@@ -355,6 +357,31 @@
<scope>${utilities.bundle.hive.scope}</scope>
</dependency>
+ <!--
+ Supplies org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat,
which is the superclass of
+ HoodieParquetInputFormatBase in hudi-hadoop-mr, which is whitelisted in
the shade artifactSet above. Without
+ this, the bundle ships those subclasses with their superclass missing
and any use of the Hive input formats fails
+ with ClassNotFoundException on MapredParquetInputFormat (HUDI-7321).
+
+ The core classifier is what the rest of the build uses
(hive.exec.classifier): it carries Hive's own
+ classes without Hive's shaded copies of third-party libraries, so it
does not drag protobuf, guava and
+ friends into this bundle. Scope follows utilities.bundle.hive.scope like
the hive artifacts above, so it
+ is provided by default and only packaged under
-Putilities-bundle-shade-hive.
+ -->
+ <dependency>
+ <groupId>${hive.groupid}</groupId>
+ <artifactId>hive-exec</artifactId>
+ <version>${hive.version}</version>
+ <classifier>${hive.exec.classifier}</classifier>
+ <scope>${utilities.bundle.hive.scope}</scope>
+ <!--
+ Deliberately no <exclusions> here. The root pom's dependencyManagement
entry for hive-exec already
+ excludes javax.mail, the jetty aggregate, pentaho, log4j 1.x, log4j2,
slf4j-log4j12 and hbase, several
+ of which the enforcer bans outright. Declaring exclusions locally
replaces that managed set rather
+ than adding to it, which would quietly let those back in.
+ -->
Review Comment:
**blocker:** This declaration changes the default bundle, not just the
profile. `hive-exec:core` at depth 1 makes its transitive `commons-io:2.4` win
nearest-wins mediation over `hudi-common`'s 2.14.0, and commons-io is
whitelisted + relocated, so the shipped default jar drops to 2.4 (re-opens
CVE-2024-47554, HUDI-8805 / #12709) and loses 232 relocated classes. Shaded
`org.apache.tika.io.TikaInputStream` still references one of them
(`UnsynchronizedByteArrayInputStream`), so `TikaDocumentParser` throws
`NoClassDefFoundError` on this jar. Could we exclude hive-exec's transitives
outright, as below? None of them are whitelisted, so the profile jar is
unchanged. The comment's premise also does not hold on Maven 3.9.14: with a
local exclusion added, the managed `javax.mail`/`log4j`/`janino`/`jetty-all`
exclusions still apply.
```suggestion
<!-- hive-exec's own classes are all this bundle wants: its
transitives are either declared above or
supplied by the cluster, and letting them into mediation
downgrades commons-io to 2.4. -->
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
```
<details>
<summary>evidence</summary>
```
mvn dependency:tree -pl packaging/hudi-utilities-bundle -Dverbose
-Dincludes=commons-io:commons-io (no profile)
+- org.apache.hudi:hudi-spark_2.12 -> hudi-common ->
(commons-io:commons-io:jar:2.14.0:compile - omitted for conflict with 2.4)
unzip -p hudi-utilities-bundle_2.12-*.jar
META-INF/maven/commons-io/commons-io/pom.properties
version=2.4 (master: version=2.14.0)
javap -v org/apache/tika/io/TikaInputStream.class | grep Unsynchronized
org/apache/hudi/org/apache/commons/io/input/UnsynchronizedByteArrayInputStream
(0 such entries left in the jar)
dependencies/hudi-utilities-bundle_2.12.txt still records commons-io 2.14.0
```
</details>
--
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]
