danny0405 commented on code in PR #19330:
URL: https://github.com/apache/hudi/pull/19330#discussion_r3627256415
##########
packaging/hudi-flink-bundle/pom.xml:
##########
@@ -651,6 +651,121 @@
<scope>${flink.bundle.hive.scope}</scope>
</dependency>
</dependencies>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ <version>${maven-shade-plugin.version}</version>
+ <executions>
+ <execution>
+ <configuration>
+ <artifactSet>
+ <!-- hive-exec needs the ANTLR runtime for Hive DDL parsing
+ (Driver.compile) but does not embed it. -->
+ <includes combine.children="append">
+ <include>org.antlr:antlr-runtime</include>
+ <include>org.antlr:ST4</include>
+ <include>org.antlr:antlr4-runtime</include>
Review Comment:
`artifactSet` only filters artifacts already present in the resolved
dependency graph; it does not add dependencies. Neither the Hive 2 nor Hive 3
profile resolves `antlr4-runtime`, so this entry contributes no classes (the
built Hive-2 bundle contains 121 ANTLR 3 runtime classes and zero
`org/antlr/v4/runtime` classes). If ANTLR 4 is required as the PR description
says, please add it as a profile dependency; otherwise this ineffective include
should be removed. The same applies to the Hive-3 block.
##########
packaging/hudi-flink-bundle/pom.xml:
##########
@@ -651,6 +651,121 @@
<scope>${flink.bundle.hive.scope}</scope>
</dependency>
</dependencies>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ <version>${maven-shade-plugin.version}</version>
+ <executions>
+ <execution>
+ <configuration>
+ <artifactSet>
+ <!-- hive-exec needs the ANTLR runtime for Hive DDL parsing
+ (Driver.compile) but does not embed it. -->
+ <includes combine.children="append">
+ <include>org.antlr:antlr-runtime</include>
+ <include>org.antlr:ST4</include>
+ <include>org.antlr:antlr4-runtime</include>
+ </includes>
+ </artifactSet>
+ <!-- hive-exec is a fat jar that embeds these deps
un-relocated; under Flink
+ child-first classloading they would shadow the versions
Hudi/Flink need.
+ Scoped to this profile so the default (no-Hive)
bundle's ABI is unchanged. -->
+ <relocations combine.children="append">
+ <relocation>
+ <pattern>com.google.common.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}com.google.common.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>com.google.protobuf.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}com.google.protobuf.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.commons.lang3.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.commons.lang3.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.commons.lang.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.commons.lang.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.thrift.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.thrift.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.orc.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.orc.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.json.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.json.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>io.airlift.compress.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}io.airlift.compress.</shadedPattern>
+ </relocation>
+ <!-- Relocate parquet-format-structures embedded in
hive-exec, but keep
+ org.apache.parquet.format.converter.** (provided by
parquet-hadoop) at
+ its original name: it is used with the un-relocated
+ ParquetFileReader.readFooter, so relocating it causes
a NoSuchMethodError. -->
+ <relocation>
+ <pattern>org.apache.parquet.format.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.parquet.format.</shadedPattern>
+ <excludes>
+
<exclude>org.apache.parquet.format.converter.**</exclude>
+ </excludes>
+ </relocation>
+ <relocation>
+ <pattern>org.codehaus.jackson.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.codehaus.jackson.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>com.facebook.fb303.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}com.facebook.fb303.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.iq80.snappy.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.iq80.snappy.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>au.com.bytecode.opencsv.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}au.com.bytecode.opencsv.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>javaewah.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}javaewah.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>javolution.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}javolution.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>jodd.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}jodd.</shadedPattern>
+ </relocation>
+ </relocations>
+ <!-- Strip hive-exec's stale embedded Avro / ORC / Parquet
so the declared
+ modern avro, orc-core and parquet-* artifacts are the
only source for
+ those namespaces (relocated for
avro/orc/parquet.format, canonical for
+ parquet core). Without this we rely on shade's artifact
ordering to make
+ the modern copy win; stripping makes it explicit and
version-safe. -->
+ <filters combine.children="append">
+ <filter>
+ <artifact>org.apache.hive:hive-exec</artifact>
Review Comment:
`hive-exec` also embeds protobuf and Airlift classes, and these relocations
map them into namespaces already populated by modern bundle dependencies:
Hudi-IO supplies protobuf 3.25.5 at `org.apache.hudi.com.google.protobuf`, and
`aircompressor` 0.27 maps to `org.apache.hudi.io.airlift.compress`. The Hive-2
build has 237 protobuf and 32 Airlift class overlaps. Current artifact order
happens to select the modern copies, but that reintroduces exactly the
version/order dependence this filter is intended to remove. Please also exclude
`com/google/protobuf/**` and `io/airlift/compress/**` from `hive-exec` (and
mirror that in Hive 3), or use distinct destinations.
##########
packaging/hudi-flink-bundle/pom.xml:
##########
@@ -651,6 +651,121 @@
<scope>${flink.bundle.hive.scope}</scope>
</dependency>
</dependencies>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-shade-plugin</artifactId>
+ <version>${maven-shade-plugin.version}</version>
+ <executions>
+ <execution>
+ <configuration>
+ <artifactSet>
+ <!-- hive-exec needs the ANTLR runtime for Hive DDL parsing
+ (Driver.compile) but does not embed it. -->
+ <includes combine.children="append">
+ <include>org.antlr:antlr-runtime</include>
+ <include>org.antlr:ST4</include>
+ <include>org.antlr:antlr4-runtime</include>
+ </includes>
+ </artifactSet>
+ <!-- hive-exec is a fat jar that embeds these deps
un-relocated; under Flink
+ child-first classloading they would shadow the versions
Hudi/Flink need.
+ Scoped to this profile so the default (no-Hive)
bundle's ABI is unchanged. -->
+ <relocations combine.children="append">
+ <relocation>
+ <pattern>com.google.common.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}com.google.common.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>com.google.protobuf.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}com.google.protobuf.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.commons.lang3.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.commons.lang3.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.commons.lang.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.commons.lang.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.thrift.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.thrift.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.apache.orc.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.apache.orc.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>org.json.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}org.json.</shadedPattern>
+ </relocation>
+ <relocation>
+ <pattern>io.airlift.compress.</pattern>
+
<shadedPattern>${flink.bundle.shade.prefix}io.airlift.compress.</shadedPattern>
+ </relocation>
+ <!-- Relocate parquet-format-structures embedded in
hive-exec, but keep
+ org.apache.parquet.format.converter.** (provided by
parquet-hadoop) at
+ its original name: it is used with the un-relocated
+ ParquetFileReader.readFooter, so relocating it causes
a NoSuchMethodError. -->
+ <relocation>
+ <pattern>org.apache.parquet.format.</pattern>
Review Comment:
This exclude preserves only the converter's class name; Shade still rewrites
every `org.apache.parquet.format.*` type referenced in its bytecode. In the
built Hive-2 jar, canonical
`org.apache.parquet.format.converter.ParquetMetadataConverter.toParquetMetadata(...)`
now returns `org.apache.hudi.org.apache.parquet.format.FileMetaData`, leaving
a binary-incompatible canonical Parquet API that can still produce
`NoSuchMethodError` for Flink/external Parquet code loaded against the bundle.
Since the filter below already removes all Hive-embedded
`org/apache/parquet/**`, could we drop this relocation and keep the declared
Flink-aligned `parquet-format-structures` canonical, or relocate the whole
Parquet API consistently? The Hive-3 block has the same issue.
--
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]
