Fix shading issues with Iceberg integration

[andygrove]

Describe the bug

Although the Comet/Iceberg integration in #1920 and apache/iceberg#13378 appears to work, it requires both the Comet and Iceberg jars to be on the classpath, even though the Iceberg jar contains a copy of the Comet classes.

One reason that this is required is due to shading issues with the Arrow Java library. Arrow contains JNI classes that cannot be shaded because the fully qualified Java method names must match the native function names in the C code, and Comet does not relocate these classes, but Iceberg does. If the Comet jar is not on the classpath, then queries fail at runtime because Arrow cannot load the JNI classes.

java.lang.RuntimeException: Could not find class Lorg/apache/arrow/c/jni/PrivateData;
	at java.base/java.lang.ClassLoader$NativeLibrary.load0(Native Method)
	at java.base/java.lang.ClassLoader$NativeLibrary.load(ClassLoader.java:2450)
	at java.base/java.lang.ClassLoader$NativeLibrary.loadLibrary(ClassLoader.java:2506)
	at java.base/java.lang.ClassLoader.loadLibrary0(ClassLoader.java:2705)
	at java.base/java.lang.ClassLoader.loadLibrary(ClassLoader.java:2635)
	at java.base/java.lang.Runtime.load0(Runtime.java:768)
	at java.base/java.lang.System.load(System.java:1854)
	at org.apache.comet.shaded.arrow.c.jni.JniLoader.load(JniLoader.java:90)

Another issue I ran into, which was potentially only happening due to a specific class loading order, is that Comet did not recognize the SupportsComet interface being implemented in Iceberg's SparkBatchQueryScan. I suspect that this was due to having two different versions of SupportsComet on the classpath (one in each jar), but I have not been able to prove that this is the case.

This issue is to review these issues and perhaps remove the requirement to have the Comet jar on the classpath and make the Iceberg jar work alone.

It may be beneficial to try and reduce the number of Arrow versions in use as well.

Spark 3.4.3 uses Arrow 11.0.0
Comet uses Arrow 18.3.0
Iceberg uses Arrow 15.0.2

Steps to reproduce

No response

Expected behavior

No response

Additional context

No response

[andygrove]

@huaxingao I found that updating the Iceberg spark modules to prevent shading of some Arrow classes allows me to run tests successfully without needing the Comet jar on the classpath:

    relocate ('org.apache.arrow', 'org.apache.iceberg.shaded.org.apache.arrow') {
      exclude 'org.apache.arrow.c.**'
    }

[huaxingao]

grep -r "org.apache.arrow.c" .         
Binary file ./flink/v2.0/flink-runtime/build/libs/iceberg-flink-runtime-2.0-1.10.0-SNAPSHOT.jar matches
Binary file ./spark/v3.4/spark-runtime/build/libs/iceberg-spark-runtime-3.4_2.12-1.10.0-SNAPSHOT.jar matches

I checked iceberg: it doesn't have direct dependency on org.apache.arrow.c. I think it's safe to exclude 'org.apache.arrow.c.**'

[huaxingao]

I submitted an iceberg PR for this

[parthchandra]

I've been thinking about this. If the version of arrow used by Iceberg is different from the one used by Comet, and the version used by Comet has a fix in the C data interface that Comet requires, then 'unshading' the arrow c data interface package in Iceberg's arrow would potentially result in the wrong version getting picked up.
It is perhaps safer to keep the version in Iceberg shaded so the only version in play is the one used by Comet.

[huaxingao]

@andygrove @parthchandra
Do we want to keep the current Arrow shading in Iceberg as is and put both the Comet and Iceberg JARs on the classpath? If that's the plan, I will close this apache/iceberg#13410

[andygrove]

@andygrove @parthchandra Do we want to keep the current Arrow shading in Iceberg as is and put both the Comet and Iceberg JARs on the classpath? If that's the plan, I will close this apache/iceberg#13410

Yes, I think so. I will close this issue.

[parthchandra]

Thanks @huaxingao @andygrove, yes, I think it is safer to leave the Arrow C data API shading in Iceberg as is.