GH-41836: [Java] Fix an undefined symbol error when ARROW_S3=OFF

[PHILO-HE]

Rationale for this change

Fix undefined symbol error reported at runtime after ARROW_S3=OFF is used in compiling arrow Java.

What changes are included in this PR?

Are these changes tested?

Tested in my local. Not sure whether we need to add some test.

Are there any user-facing changes?

No.

• GitHub Issue: [Java] Undefined symbol error when compile arrow java with ARROW_S3=OFF #41836

[github-actions]

⚠️ GitHub issue #41836 has been automatically assigned in GitHub to PR creator.

[vibhatha]

@github-actions crossbow submit -g java

[github-actions]

Revision: 1ef554d51d772a7c5970abc3b8f53afbb0924be6

Submitted crossbow builds: ursacomputing/crossbow @ actions-8150e0c94c

Task	Status
java-jars
test-conda-python-3.10-spark-v3.5.0
test-conda-python-3.11-spark-master
test-conda-python-3.8-spark-v3.5.0
verify-rc-source-java-linux-almalinux-8-amd64
verify-rc-source-java-linux-conda-latest-amd64
verify-rc-source-java-linux-ubuntu-20.04-amd64
verify-rc-source-java-linux-ubuntu-22.04-amd64
verify-rc-source-java-macos-amd64

[kou]

Could you confirm whether arrow/util/config.h is included here implicitly?

[PHILO-HE]

@kou, thanks for your review! I didn't find this header is included implicitly.

[kou]

OK. How about the following?

diff --git a/java/dataset/src/main/cpp/jni_wrapper.cc b/java/dataset/src/main/cpp/jni_wrapper.cc
index 19a43c8d2f..f1819aab0b 100644
--- a/java/dataset/src/main/cpp/jni_wrapper.cc
+++ b/java/dataset/src/main/cpp/jni_wrapper.cc
@@ -25,9 +25,8 @@
 #include "arrow/c/helpers.h"
 #include "arrow/dataset/api.h"
 #include "arrow/dataset/file_base.h"
-#include "arrow/filesystem/localfs.h"
+#include "arrow/filesystem/api.h"
 #include "arrow/filesystem/path_util.h"
-#include "arrow/filesystem/s3fs.h"
 #include "arrow/engine/substrait/util.h"
 #include "arrow/engine/substrait/serde.h"
 #include "arrow/engine/substrait/relation.h"

arrow/filesystem/api.h includes arrow/util/config.h explicitly and ARROW_S3 must be defined if S3 is enabled with arrow/filesystem/api.h.

[PHILO-HE]

@kou, by including arrow/filesystem/api.h, a few extra headers are directly or indirectly included, but they are not actually used here. Can we just keep the current change?

[kou]

If we want to keep the current style, we need to include arrow/util/config.h explicitly before we use ARROW_S3.

[PHILO-HE]

@kou, I just realized what you meant. The code was updated. Thanks!

[kou]

Could you test that this change doesn't break #36934? A reproduced program exists in the description of #36934.
(You need to enable S3 for this.)

[PHILO-HE]

Could you test that this change doesn't break #36934? A reproduced program exists in the description of #36934. (You need to enable S3 for this.)

@kou, I just did a test with S3 enabled. This change doesn't break it.

[kou]

+1

Thanks for checking it.

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 052c330.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 76 possible false positives for unstable benchmarks that are known to sometimes produce them.