Merge branch 'main' into centralize-float-canonicalization

Author: mkleen

.ai/skills/datafusion-ffi/SKILL.md

@@ -197,21 +197,21 @@ Templates: `test_ffi_table_provider_local_bypass` and `test_round_trip_ffi_table
 
 What unit tests catch: Rust-level correctness (translation logic, lifetime bugs, leaks under valgrind/miri, Send/Sync, error propagation, codec round-trips). What they **cannot** catch: real ABI bugs. Both producer and consumer share `#[repr(C)]` layout because they are the exact same struct definition in memory.
 
-#### b. Cross-library integration tests (`tests/ffi_*.rs`, gated by the `integration-tests` feature)
+#### b. Cross-library integration tests (`integration-tests/tests/`, gated by the `integration-tests` feature)
 
-The crate is published as `crate-type = ["cdylib", "rlib"]`. The integration tests in `datafusion/ffi/tests/` use `libloading` to `dlopen` the crate's own `cdylib` and call `datafusion_ffi_get_module` — a `#[unsafe(no_mangle)] extern "C"` entry point defined in `src/tests/mod.rs` and gated by `#[cfg(feature = "integration-tests")]`. The test executable links against the rlib (consumer side); the dlopen'd cdylib is the producer side. Even though both are built from the same source, they are independent compilation outputs going through the actual FFI symbol path.
+The integration tests live in the unpublished `datafusion-ffi-integration-tests` crate in `datafusion/ffi/integration-tests/`, so that `datafusion-ffi` itself is a plain rlib. That crate is built as a `cdylib` that exports `datafusion_ffi_get_module` — a `#[unsafe(no_mangle)] extern "C"` entry point defined in `src/tests/mod.rs` and gated by `#[cfg(feature = "integration-tests")]`. Its tests use `libloading` to `dlopen` that `cdylib`. The test executable links against the `datafusion-ffi` rlib (consumer side); the dlopen'd cdylib is the producer side. Even though both are built from the same source, they are independent compilation outputs going through the actual FFI symbol path.
 
 Run with:
 
 ```bash
-cargo test -p datafusion-ffi --features integration-tests
+cargo test -p datafusion-ffi -p datafusion-ffi-integration-tests --features integration-tests
 ```
 
 To add coverage for a new wrapper:
 
 1. **Add a constructor** in `src/tests/<area>.rs` (or a new file there). Return a populated `FFI_X` from a known-good native type.
 2. **Wire it into `ForeignLibraryModule`** in `src/tests/mod.rs`: add a field of type `extern "C" fn(...) -> FFI_X` and populate it in `datafusion_ffi_get_module`. This struct is the cross-library contract. Adding a field is itself an ABI change for the test module; integration tests will rebuild the cdylib automatically.
-3. **Add the test** in `tests/ffi_<area>.rs` under `#[cfg(feature = "integration-tests")] mod tests { … }`. Call `datafusion_ffi::tests::utils::get_module()` to load the cdylib, invoke your constructor through the returned `ForeignLibraryModule`, convert into `Arc<dyn X>`, and exercise every method.
+3. **Add the test** in `integration-tests/tests/cases/<area>.rs`, registered in `cases/mod.rs`, under `mod tests { … }`. Call `datafusion_ffi::tests::utils::get_module()` to load the cdylib, invoke your constructor through the returned `ForeignLibraryModule`, convert into `Arc<dyn X>`, and exercise every method.
 
 When adding a method to an existing wrapper, reuse an existing fixture and constructor when a small trait-method override can cover it. This applies both to omitted default methods and methods newly added to the trait. Add a dedicated test type or `ForeignLibraryModule` field only when the existing fixtures cannot cover the method.
 
@@ -347,7 +347,7 @@ When reviewing a PR that touches `datafusion/ffi/`:
 7. **Stabby types** for strings/vecs; crate-local `FFI_Option`/`FFI_Result` for optional/fallible payloads?
 8. **Async** uses `FfiFuture` + `.into_ffi()`, never blocking?
 9. **Codec** present on any method that ships an `Expr` / plan?
-10. **Unit tests** include both local-bypass and `mock_foreign_marker_id` forced-foreign cases? **Integration tests** in `tests/ffi_*.rs` exist for any wrapper that takes/returns non-trivial FFI types, and for *every* layout change? `cargo test -p datafusion-ffi --features integration-tests` must pass before merging an ABI-affecting PR.
+10. **Unit tests** include both local-bypass and `mock_foreign_marker_id` forced-foreign cases? **Integration tests** in `integration-tests/tests/` exist for any wrapper that takes/returns non-trivial FFI types, and for *every* layout change? `cargo test -p datafusion-ffi -p datafusion-ffi-integration-tests --features integration-tests` must pass before merging an ABI-affecting PR.
 11. **No `datafusion` runtime dep** crept into `Cargo.toml`?
 12. **`Arc::clone(&x)`** everywhere — no implicit `x.clone()` on `Arc` (the lint will reject it but worth pre-flagging).
 13. **`cargo clippy --all-targets --all-features -- -D warnings`** clean on the crate?

.asf.yaml

@@ -69,6 +69,7 @@ github:
           - "cargo check datafusion-spark features"
           - "cargo test (amd64)"
           - "cargo test 'extended_tests' (amd64)"
+          - "microbenchmark tests (amd64)"
           - "cargo test hash collisions (amd64)"
           - "cargo test datafusion-cli (amd64)"
           - "cargo examples (amd64)"

.github/actions/setup-rustup/action.yaml

@@ -0,0 +1,36 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+name: Install rustup
+description: 'Install rustup and the toolchain from rust-toolchain.toml on a runner without Rust, such as ubuntu-slim'
+runs:
+  using: "composite"
+  steps:
+    - name: Install rustup and the toolchain
+      shell: bash
+      run: |
+        RETRY=("ci/scripts/retry" timeout 120)
+        "${RETRY[@]}" curl -sSf -o "$RUNNER_TEMP/rustup-init.sh" https://sh.rustup.rs
+        sh "$RUNNER_TEMP/rustup-init.sh" -y --no-modify-path --profile minimal --default-toolchain none
+        CARGO_BIN="${CARGO_HOME:-$HOME/.cargo}/bin"
+        export PATH="$CARGO_BIN:$PATH"
+        echo "$CARGO_BIN" >> "$GITHUB_PATH"
+        # With no toolchain argument, rustup installs the toolchain and the
+        # components that rust-toolchain.toml names.
+        "${RETRY[@]}" rustup toolchain install
+    - name: Configure rust runtime env
+      uses: ./.github/actions/setup-rust-runtime

.github/dependabot.yml

@@ -62,14 +62,6 @@ updates:
         update-types:
           - "minor"
           - "patch"
-  # dev/depcheck is a standalone crate excluded from the workspace, so it is
-  # not covered by the workspace entry above
-  - package-ecosystem: cargo
-    directory: "/dev/depcheck"
-    schedule:
-      interval: weekly
-    target-branch: main
-    labels: [auto-dependencies]
   # the wasm test app has its own npm lockfile
   - package-ecosystem: npm
     directory: "/datafusion/wasmtest/datafusion-wasm-app"

.github/pull_request_template.md

@@ -1,3 +1,8 @@
+> [!IMPORTANT]
+>
+> **Fork CI setup:** For pull requests from a fork to `main`, enable workflows in your fork's **Actions** tab before you push.
+> See the [fork CI setup guide](https://datafusion.apache.org/contributor-guide/#pull-requests-from-forks).
+
 ## Which issue does this PR close?
 
 <!--
@@ -10,7 +15,7 @@ We generally require a GitHub issue to be filed for all bug fixes and enhancemen
 
 <!--
  Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
- Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.  
+ Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
 
 Please explain the problem you are trying to solve in terms of the user-visible
 behavior, rather than the implementation.

.github/workflows/audit.yml

@@ -41,11 +41,13 @@ permissions:
 
 jobs:
   security_audit:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-slim
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
+      - name: Setup Rust toolchain
+        uses: ./.github/actions/setup-rustup
       - name: Install cargo-audit
-        uses: taiki-e/install-action@ab6895351a88b908e96a4697860d4ffea5e57dbe  # v2.87.27
+        uses: taiki-e/install-action@8cca0918f8471c33515202b73f56ab27f2bed283  # v2.87.29
         with:
           tool: cargo-audit
       - name: Run audit check

.github/workflows/breaking_changes_detector.yml

@@ -18,7 +18,8 @@
 # Detect semver-incompatible (breaking) API changes in crates modified by a PR.
 #
 # Only public workspace crates that have file changes are checked.
-# Internal crates (benchmarks, test-utils, sqllogictest, doc) are excluded.
+# Crates with `publish = false` (such as benchmarks and test-utils) and
+# datafusion-sqllogictest are excluded.
 #
 # This workflow only runs cargo-semver-checks and uploads the result as an
 # artifact. The actual PR comment is posted by a companion workflow
@@ -78,18 +79,17 @@ jobs:
           echo "packages=$PACKAGES" >> "$GITHUB_OUTPUT"
           echo "Changed crates: $PACKAGES"
 
-      # `datafusion-substrait` (and crates that depend on it via sqllogictest)
-      # have a build script that calls protoc, which is not preinstalled on
-      # ubuntu-latest runners.
-      - name: Install Protobuf Compiler
+      # The `substrait` crate's build script runs protoc, which is not
+      # preinstalled on ubuntu-latest runners.
+      - name: Install protoc
         if: steps.changed_crates.outputs.packages != ''
-        run: |
-          sudo apt-get update
-          sudo apt-get install -y protobuf-compiler
+        uses: taiki-e/install-action@8cca0918f8471c33515202b73f56ab27f2bed283  # v2.87.29
+        with:
+          tool: protoc@3.21.12
 
       - name: Install cargo-semver-checks
         if: steps.changed_crates.outputs.packages != ''
-        uses: taiki-e/install-action@ab6895351a88b908e96a4697860d4ffea5e57dbe  # v2.87.27
+        uses: taiki-e/install-action@8cca0918f8471c33515202b73f56ab27f2bed283  # v2.87.29
         with:
           tool: cargo-semver-checks
 

.github/workflows/breaking_changes_detector_comment.yml

@@ -61,7 +61,7 @@ jobs:
   comment-on-pr:
     name: Comment on pull request
     if: github.event.workflow_run.event == 'pull_request'
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-slim
     # Scoped to the minimum needed to upsert/delete the sticky comment.
     permissions:
       actions: read

.github/workflows/codeql.yml

@@ -45,11 +45,11 @@ jobs:
         persist-credentials: false
 
     - name: Initialize CodeQL
-      uses: github/codeql-action/init@24c54180a607b1449ed407dd24f251e4e9147c8d # v4
+      uses: github/codeql-action/init@6f2ac81f5cd81fe351a197ba0b00d91d03cd2aee # v4
       with:
         languages: actions
 
     - name: Perform CodeQL Analysis
-      uses: github/codeql-action/analyze@24c54180a607b1449ed407dd24f251e4e9147c8d # v4
+      uses: github/codeql-action/analyze@6f2ac81f5cd81fe351a197ba0b00d91d03cd2aee # v4
       with:
         category: "/language:actions"

.github/workflows/dependencies.yml

@@ -40,36 +40,19 @@ permissions:
   contents: read
 
 jobs:
-  depcheck:
-    name: Circular Dependency Check
-    runs-on: ubuntu-latest
-    container:
-      image: amd64/rust
-    steps:
-      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
-        with:
-          submodules: true
-          fetch-depth: 1
-      - name: Setup Rust toolchain
-        uses: ./.github/actions/setup-builder
-        with:
-          rust-version: stable
-      - name: Check dependencies
-        run: bash ci/scripts/check_circular_dependencies.sh
-
   detect-unused-dependencies:
     name: Detect Unused Dependencies
-    runs-on: ubuntu-latest
-    container:
-      image: amd64/rust
+    runs-on: ubuntu-slim
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1  # v7.0.1
+      - name: Setup Rust toolchain
+        uses: ./.github/actions/setup-rustup
       - name: Load tool versions
         run: |
           . ci/scripts/utils/tool_versions.sh
           echo "CARGO_MACHETE_VERSION=${CARGO_MACHETE_VERSION}" >> "$GITHUB_ENV"
       - name: Install cargo-machete
-        uses: taiki-e/install-action@ab6895351a88b908e96a4697860d4ffea5e57dbe  # v2.87.27
+        uses: taiki-e/install-action@8cca0918f8471c33515202b73f56ab27f2bed283  # v2.87.29
         with:
           tool: cargo-machete@${{ env.CARGO_MACHETE_VERSION }}
       - name: Detect unused dependencies