timsaucer opened a new pull request, #1720: URL: https://github.com/apache/datafusion-python/pull/1720
# Which issue does this PR close? Part of #1719. Closes nothing on its own — it is the first PR of a stack, and #1719 is closed by the example that follows. **Stacked on #1679**, so review that first; this PR's base is `feat/ffi-with-extensions` and the diff shown here is only the four commits' worth of changes described below. # Rationale for this change #1719 builds a multi-library distributed-execution example with real OS-process workers. Four things it needs are missing or actively harmful today, and each was found by trying to build the example rather than by reading the code. They are separated out here so the example PR is example code and documentation only. Two of them are error paths that did not raise. A Rust `panic!` reaching Python through PyO3 arrives as `PanicException`, which derives from `BaseException`, so it escapes `except Exception` and cannot be handled by ordinary code. Both were reachable from perfectly reasonable calls. # What changes are included in this PR? **`ExecutionPlan.output_partitioning`** (G3). `partition_count` already existed but throws away everything except the count, so a driver deciding how to spread work across workers could not tell hash-distributed output from merely counted output, nor read the hash keys. The new accessor returns a `PhysicalPartitioning` exposing `scheme`, `partition_count` and `hash_expressions`. It is deliberately **not** called `Partitioning`: `datafusion.expr.Partitioning` already exists and is the *logical* partitioning `repartition_by_hash` takes as a request, whereas this is what a built plan actually does. Physical expressions have no Python representation, so the hash keys are returned in their displayed form. **`SessionContext.execute` bounds-checks the partition index** (G4). The plan's leaves index their partition vector directly, so an out-of-range index reached `MemorySourceConfig` and panicked. The panic was caught as a tokio `JoinError` and arrived as `index out of bounds: the len is 2 but the index is 5` — naming neither the plan nor the index the caller passed. Now a `ValueError` naming both. **`SessionConfig.set` no longer aborts on an unknown namespace** (G5). It routed through `SessionConfig::set_str`, which unwraps, so `datafusion.runtime.memory_limit` — or a config extension not yet installed — produced a `PanicException`. It now goes through `options_mut().set` and propagates the error. This is the first thing hit when replaying settings read back from `information_schema.df_settings`, which lists `datafusion.runtime.*` keys that have no `ConfigOptions` namespace at all. **Two stale docstrings corrected** (G8). Both `ExecutionPlan.to_bytes` and `from_bytes` claimed that a table registered from record batches cannot be serialized. That is true of `LogicalPlan`, whose `try_encode_table_provider` has no arm for a memory table, and false of the physical layer, which inlines the batches into the encoded scan. Verified by decoding on a context that shares nothing with the encoder — no codecs, no tables — and executing it. A test pins this, because it is what allows a worker to run a plan the driver encoded. **One comment, no behaviour change** (G2). The `ForeignExecutionPlan` arm in the example provider's physical codec claims every *other* library's nodes, which `extension-guide/checklist.md` tells authors never to do. I tried to narrow it and reverted: it is load-bearing. `EnsureCooperative` runs on the host during a foreign planner's `create_physical_plan` and hands the library back a `ForeignExecutionPlan` wrapping the host's `CooperativeExec`, which has no reachable `try_to_proto`. Narrowing the arm to `DataSourceExec` alone makes **31 of the 51** tests in `datafusion-ffi-query-planner-example` fail, every one on that node. The comment now records why the arm exists, what it costs, and that a planner controlling its own physical optimizer rules needs no such arm. The underlying gap is G1 in #1719 and belongs upstream. # Are there any user-facing changes? Yes, all additive or strictly-better failures. No `api change` label: nothing that previously worked behaves differently. - New `ExecutionPlan.output_partitioning` and new `PhysicalPartitioning` class, exported from `datafusion`. `partition_count` is untouched and agrees with `output_partitioning.partition_count`. - `SessionContext.execute` raises `ValueError` for an out-of-range partition where it previously panicked. - `SessionConfig.set` raises for an unknown config namespace where it previously raised `PanicException`. Code that caught `BaseException` to work around this can stop. - `docs/source/user-guide/upgrade-guides.md` documents all three. Verified with `pytest python/` (1443 passed, 12 skipped, including 6 new doctests), both example crates' suites (55 and 51 passed), and `pre-commit run --all-files`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
