[BOT] Bump Velox version

[velox4j-bot]

Update Velox to latest. Diff: facebookincubator/velox@45a03a6...3da0390

[zhztheplayer]

@copilot check CI failure in ut-al2023

[Copilot]

@copilot check CI failure in ut-al2023

The failure is caused by the new Velox version introducing VELOX_ENABLE_GEO=ON by default, which pulls in s2geometry as a bundled dependency. s2geometry requires absl, but Velox only fetches absl when VELOX_BUILD_TESTING=ON. Since velox4j builds with VELOX_BUILD_TESTING=OFF, absl is never resolved and the CMake config fails.

Fixed in 4653de5 by setting VELOX_ENABLE_GEO OFF in CMakeLists.txt — velox4j doesn't use any geospatial functions, so this feature can be safely disabled.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

• https://api.github.com/repos/facebookincubator/velox/contents/CMake/resolve_dependency_modules/s2geometry
  • Triggering command: /usr/bin/curl curl -s REDACTED (http block)

If you need me to access, download, or install something from one of these locations, you can either:

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

[zhztheplayer]

@copilot which Velox commit triggers this regression?

[Copilot]

@copilot which Velox commit triggers this regression?

The regression was introduced by facebookincubator/velox@fcb63d3 — "feat: S2 presto functions (#15511)".

That commit extended the VELOX_ENABLE_GEO block to also fetch s2geometry, which in turn requires absl. In the old Velox (45a03a6a), VELOX_ENABLE_GEO only pulled in geos; the new version also pulls in s2geometry. Since absl is only resolved inside the if(VELOX_BUILD_TESTING) block (for the Spark query runner), it was never fetched when VELOX_BUILD_TESTING=OFF, breaking the CMake configuration.

[zhztheplayer]

@copilot shelve your fix, add a velox patch:

Index: CMakeLists.txt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/CMakeLists.txt b/CMakeLists.txt
--- a/CMakeLists.txt	(revision 3da0390118fa4b0addbfbdeb03f438ec1b1237a6)
+++ b/CMakeLists.txt	(date 1776413275006)
@@ -766,6 +766,10 @@
   list(PREPEND CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/CMake/resolve_dependency_modules/s2geometry)
   velox_set_source(s2geometry)
   velox_resolve_dependency(s2geometry)
+  if(NOT target absl::base)
+    velox_set_source(absl)
+    velox_resolve_dependency(absl)
+  endif()
 endif()
 
 add_subdirectory(velox)

to see whether the issue can be fixed

[Copilot]

@copilot shelve your fix, add a velox patch:

...

Done in ed3cfbd. The VELOX_ENABLE_GEO OFF workaround has been removed. Instead:

• Added src/main/cpp/cmake/patches/velox/fix-s2geometry-absl-dep.patch — applies the requested patch to Velox's CMakeLists.txt to resolve absl when VELOX_ENABLE_GEO is ON and absl::base target isn't already present.
• Added src/main/cpp/cmake/apply_velox_patches.cmake — a small CMake script that applies both Velox patches in order (needed because PATCH_COMMAND only accepts a single command).
• Updated FetchContent_Declare to invoke the script via cmake -P instead of calling patch directly.

[zhztheplayer]

@copilot nevermind, bring your fix back, and add some comments on the OFF code

[Copilot]

@copilot nevermind, bring your fix back, and add some comments on the OFF code

Done in 64c08d3. The Velox patch approach has been reverted. VELOX_ENABLE_GEO OFF is back, now with a comment explaining the reason:

# Disable GEO support: Velox's VELOX_ENABLE_GEO pulls in s2geometry which
# requires absl, but absl is only resolved by Velox when VELOX_BUILD_TESTING=ON
# (for the Spark query runner). Since velox4j builds with
# VELOX_BUILD_TESTING=OFF and does not use any geospatial functions, disable
# this feature to avoid the unresolved absl dependency.
set(VELOX_ENABLE_GEO OFF)

[zhztheplayer]

@copilot keep your refactor for code patching

[Copilot]

@copilot keep your refactor for code patching

Done in 3534fb0. Both are now in place:

• VELOX_ENABLE_GEO OFF with explanatory comments
• apply_velox_patches.cmake + fix-s2geometry-absl-dep.patch restored, with PATCH_COMMAND using cmake -P to apply all patches in order

[zhztheplayer]

@copilot check ci

[Copilot]

@copilot check ci

The ut-centos7 failure was a linker error — Velox's expression fuzzer test referenced S2CellOp symbols that weren't built because VELOX_ENABLE_GEO=OFF. This happened because VELOX_BUILD_TESTING was only set to OFF when VELOX4J_BUILD_TESTING=OFF, so the UT run left it at Velox's default (ON), causing Velox to build its own tests.

Fixed in 5d07b7a by making VELOX_BUILD_TESTING=OFF unconditional — velox4j never runs Velox's own tests, so this should always be OFF regardless of VELOX4J_BUILD_TESTING.