Close the review's smaller findings

- Ignore the .arrow.tmp files a crash mid-write leaves behind, not just the
  published shuffle output.
- The capsule-protocol skill's rule on fallback was categorical; the
  distributed example is the repository's first legitimate exception, and an
  agent following the skill would have "fixed" it into breakage. The skill
  now carves out the rewriting planner and points at the guide.
- A non-UTF-8 table directory was lossily mangled into the logical payload
  while file paths in the same function were rejected outright; both are
  rejected now.
- dfx-udfs declared datafusion-common and datafusion-expr and used neither;
  a crate meant to be copied should not carry a misleading dependency list.
- build.yml states the assumption container:off makes about build and test
  landing on the same runner image, and both ways out if it ever bites.
- test.yml runs run_tpch.py against the dataset it already generates, so the
  script the README advertises cannot rot while its suites stay green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: timsaucer

.ai/skills/ffi-capsule-protocol/SKILL.md

@@ -67,8 +67,13 @@ above: it takes a second argument, the planner assembled so far. A session has
 one planner slot, so planners compose by nesting rather than by chaining, and
 the host hands each bundle the previous layer instead of letting it capture one.
 Wrap `fallback` and delegate to it; returning a planner that ignores it discards
-every layer beneath, including one the session already had. It runs after every
-bundle's codecs are installed, so `ctx` carries the final chains.
+every layer beneath, including one the session already had. The one legitimate
+exception is a planner that *rewrites* the plan (splitting it into stages, say):
+it cannot delegate, because a fallback returns nodes it can neither downcast nor
+serialize, so it plans for itself against its own optimizer rules —
+`examples/distributed/engine-library` is the worked case, and
+`docs/source/extension-guide/query-planners.md` argues it. The hook runs after
+every bundle's codecs are installed, so `ctx` carries the final chains.
 
 That is also the only hook where it does. `__datafusion_session_components__`
 runs before anything is installed, so its `ctx` still carries the chains the

.github/workflows/build.yml

@@ -192,6 +192,13 @@ jobs:
       # container, which drops a container start and an in-container rustup
       # install per library and lets the Swatinem/rust-cache above cover them.
       #
+      # "A runner like this one" is an assumption, not a guarantee: build and
+      # test are separate jobs, and during GitHub's ubuntu-latest image
+      # migrations they can land on different images. A wheel built against a
+      # newer glibc then fails to import on the test runner. If that ever
+      # bites, pin both jobs to the same explicit image -- or put these builds
+      # back in the manylinux container, which made it impossible.
+      #
       # None of them pass `target`, so their artifacts land in `target/debug`
       # alongside the priming build's. Naming a target would move them under
       # `target/<triple>/debug` and the priming build would be wasted.

.github/workflows/test.yml

@@ -163,3 +163,12 @@ jobs:
         run: |
           cd examples/tpch
           uv run --no-project pytest _tests.py
+
+      # The script the distributed example's README advertises, against the
+      # dataset generated above -- so the exact entry point a reader will
+      # copy-paste cannot rot while its test suite stays green. The row cap
+      # keeps the reshard and the worker round trip to seconds; correctness
+      # on the full shape is the pytest suites' job, this is the script.
+      - name: Run distributed TPC-H example
+        if: matrix.wheel-tag == 'abi3'
+        run: uv run --no-project python examples/distributed/run_tpch.py --rows 400000

.gitignore

@@ -44,4 +44,6 @@ examples/distributed/*/uv.lock
 # directory, so one appearing in the tree means a relative shuffle_dir
 # reached a stage node and the files landed in the working directory
 # instead -- which is how three of them were committed once already.
-examples/distributed/**/stage-*-part-*.arrow
+# The trailing `*` also catches the `.<pid>-<n>.arrow.tmp` files a
+# writer stages before its rename, which a crash mid-write leaves behind.
+examples/distributed/**/stage-*-part-*.arrow*

Cargo.lock

@@ -114,8 +114,12 @@ impl PartitionedParquetTable {
             Some(schema) => schema,
             None => Arc::new(Self::read_schema(&paths[0])?),
         };
+        let directory = directory
+            .to_str()
+            .ok_or_else(|| DataFusionError::Plan(format!("non-UTF-8 path {directory:?}")))?
+            .to_string();
         Ok(Self {
-            directory: directory.to_string_lossy().into_owned(),
+            directory,
             files,
             schema,
         })

examples/distributed/storage-library/src/table_provider.rs

@@ -30,8 +30,6 @@ publish = false
 arrow = { workspace = true }
 arrow-schema = { workspace = true }
 datafusion = { workspace = true }
-datafusion-common = { workspace = true, default-features = false }
-datafusion-expr = { workspace = true }
 datafusion-ffi = { workspace = true }
 datafusion-functions-window = { workspace = true }
 datafusion-proto = { workspace = true }