Skip to content

Commit a26b90a

Browse files
timsaucerclaude
andcommitted
Report physical partitioning, and stop two panics escaping as panics
Groundwork for a multi-library distributed-execution example. Each item here is something that example needs and cannot get today. `ExecutionPlan.output_partitioning` is new. `partition_count` already existed but discards everything except the count, so a driver deciding how to split work across workers could not tell hash-distributed output from merely counted output, nor read the hash keys. It returns a `PhysicalPartitioning`, named to keep it distinct from `datafusion.expr.Partitioning` — that one is the logical partitioning `repartition_by_hash` takes as a request, this one is what a built plan does. Physical expressions have no Python representation, so the hash keys are returned in their displayed form. `SessionContext.execute` now bounds-checks the partition index. 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. `SessionConfig.set` no longer routes through `SessionConfig::set_str`, which unwraps. An unknown namespace — `datafusion.runtime.*`, or a config extension not yet installed — aborted with a `PanicException`, which derives from `BaseException` and so escapes `except Exception`. `information_schema. df_settings` lists keys in both categories, so replaying settings onto a worker hit this first. Two docstrings on `ExecutionPlan` 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 one, and false of the physical layer, which inlines the batches: verified by decoding on a context sharing nothing with the encoder and executing. A test pins it, since it is what lets a worker run a plan the driver encoded. Also documents, rather than fixes, the `ForeignExecutionPlan` arm in the example provider's physical codec. It claims every other library's nodes, which the extension guide tells authors not to do — but it is load-bearing: `EnsureCooperative` runs 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 makes 31 of the 51 tests in the query-planner example fail, all on that node. The comment now says so, and says a planner that controls its own physical optimizer rules needs no such arm. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 9372e1b commit a26b90a

8 files changed

Lines changed: 369 additions & 13 deletions

File tree

‎crates/core/src/context.rs‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ use datafusion::execution::options::{ArrowReadOptions, ReadOptions};
4444
use datafusion::execution::runtime_env::RuntimeEnvBuilder;
4545
use datafusion::execution::session_state::SessionStateBuilder;
4646
use datafusion::execution::{FunctionRegistry, TaskContextProvider};
47+
use datafusion::physical_plan::ExecutionPlanProperties;
4748
use datafusion::prelude::{
4849
AvroReadOptions, CsvReadOptions, DataFrame, JsonReadOptions, ParquetReadOptions,
4950
};
@@ -193,8 +194,17 @@ impl PySessionConfig {
193194
Self::from(self.config.clone().with_parquet_pruning(enabled))
194195
}
195196

196-
fn set(&self, key: &str, value: &str) -> Self {
197-
Self::from(self.config.clone().set_str(key, value))
197+
/// Set a config option by key.
198+
///
199+
/// Not routed through `SessionConfig::set_str`, which unwraps the result:
200+
/// an unknown namespace -- `datafusion.runtime.*`, or a config extension
201+
/// that has not been installed yet -- would abort as a `PanicException`
202+
/// rather than raise. `information_schema.df_settings` lists keys in both
203+
/// of those categories, so replaying it is otherwise unsafe.
204+
fn set(&self, key: &str, value: &str) -> PyDataFusionResult<Self> {
205+
let mut config = self.config.clone();
206+
config.options_mut().set(key, value)?;
207+
Ok(Self::from(config))
198208
}
199209

200210
pub fn with_extension(&self, extension: Bound<PyAny>) -> PyResult<Self> {
@@ -1412,6 +1422,18 @@ impl PySessionContext {
14121422
) -> PyDataFusionResult<PyRecordBatchStream> {
14131423
let ctx: TaskContext = TaskContext::from(&self.ctx.state());
14141424
let plan = plan.plan.clone();
1425+
// Checked here because the leaves index their partitions directly: a
1426+
// `MemorySourceConfig` panics with a bare `index out of bounds`, which
1427+
// surfaces as a `JoinError::Panic` naming neither the plan nor the
1428+
// partition the caller asked for.
1429+
let partition_count = plan.output_partitioning().partition_count();
1430+
if part >= partition_count {
1431+
return Err(PyValueError::new_err(format!(
1432+
"Partition index {part} is out of range for a plan with \
1433+
{partition_count} partition(s)"
1434+
))
1435+
.into());
1436+
}
14151437
let stream = spawn_future(py, async move { plan.execute(part, Arc::new(ctx)) })?;
14161438
Ok(PyRecordBatchStream::new(stream))
14171439
}

‎crates/core/src/lib.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ fn _internal(py: Python, m: Bound<'_, PyModule>) -> PyResult<()> {
9494
m.add_class::<metrics::PyMetricsSet>()?;
9595
m.add_class::<metrics::PyMetric>()?;
9696
m.add_class::<physical_plan::PyExecutionPlan>()?;
97+
m.add_class::<physical_plan::PyPhysicalPartitioning>()?;
9798
m.add_class::<record_batch::PyRecordBatch>()?;
9899
m.add_class::<record_batch::PyRecordBatchStream>()?;
99100

‎crates/core/src/physical_plan.rs‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
use std::sync::Arc;
1919

20+
use datafusion::physical_expr::Partitioning;
2021
use datafusion::physical_plan::{ExecutionPlan, ExecutionPlanProperties, displayable};
2122
use datafusion_proto::physical_plan::AsExecutionPlan;
2223
use prost::Message;
@@ -126,6 +127,82 @@ impl PyExecutionPlan {
126127
pub fn partition_count(&self) -> usize {
127128
self.plan.output_partitioning().partition_count()
128129
}
130+
131+
#[getter]
132+
pub fn output_partitioning(&self) -> PyPhysicalPartitioning {
133+
self.plan.output_partitioning().clone().into()
134+
}
135+
}
136+
137+
/// How a physical plan's output rows are spread across its partitions.
138+
///
139+
/// Distinct from `datafusion.expr.Partitioning`, which is the *logical*
140+
/// partitioning `DataFrame.repartition` takes as a request. This one reports
141+
/// what a built plan actually does.
142+
#[pyclass(
143+
from_py_object,
144+
frozen,
145+
name = "PhysicalPartitioning",
146+
module = "datafusion",
147+
subclass
148+
)]
149+
#[derive(Debug, Clone)]
150+
pub struct PyPhysicalPartitioning {
151+
partitioning: Partitioning,
152+
}
153+
154+
#[pymethods]
155+
impl PyPhysicalPartitioning {
156+
/// Which partitioning scheme this is.
157+
///
158+
/// One of `RoundRobinBatch`, `Hash`, `Range`, or `UnknownPartitioning`.
159+
/// `UnknownPartitioning` is what a plan reports when it knows how many
160+
/// partitions it has but nothing about how rows are distributed between
161+
/// them, which is the common case for a file scan.
162+
#[getter]
163+
pub fn scheme(&self) -> &'static str {
164+
match self.partitioning {
165+
Partitioning::RoundRobinBatch(_) => "RoundRobinBatch",
166+
Partitioning::Hash(_, _) => "Hash",
167+
Partitioning::Range(_) => "Range",
168+
Partitioning::UnknownPartitioning(_) => "UnknownPartitioning",
169+
}
170+
}
171+
172+
#[getter]
173+
pub fn partition_count(&self) -> usize {
174+
self.partitioning.partition_count()
175+
}
176+
177+
/// The expressions rows are hashed on, or `None` for other schemes.
178+
///
179+
/// These are physical expressions, which have no Python representation, so
180+
/// they are returned in their displayed form.
181+
#[getter]
182+
pub fn hash_expressions(&self) -> Option<Vec<String>> {
183+
match &self.partitioning {
184+
Partitioning::Hash(exprs, _) => {
185+
Some(exprs.iter().map(|expr| format!("{expr}")).collect())
186+
}
187+
_ => None,
188+
}
189+
}
190+
191+
fn __repr__(&self) -> String {
192+
format!("{}", self.partitioning)
193+
}
194+
}
195+
196+
impl From<Partitioning> for PyPhysicalPartitioning {
197+
fn from(partitioning: Partitioning) -> Self {
198+
Self { partitioning }
199+
}
200+
}
201+
202+
impl From<PyPhysicalPartitioning> for Partitioning {
203+
fn from(partitioning: PyPhysicalPartitioning) -> Self {
204+
partitioning.partitioning
205+
}
129206
}
130207

131208
impl From<PyExecutionPlan> for Arc<dyn ExecutionPlan> {

‎docs/source/user-guide/upgrade-guides.md‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,41 @@ installed produces the same bytes as before, as do functions encoded by name.
169169
Regenerate any plan you serialized with an earlier release and stored for later
170170
use, if it was produced by a session with an extension codec installed.
171171

172+
### Physical plans report their partitioning scheme
173+
174+
{py:attr}`~datafusion.ExecutionPlan.output_partitioning` is new, and reports
175+
what {py:attr}`~datafusion.ExecutionPlan.partition_count` leaves out: whether
176+
the rows in those partitions are hash-distributed on known keys, spread
177+
round-robin, or merely counted. It returns a
178+
{py:class}`~datafusion.PhysicalPartitioning`.
179+
180+
```python
181+
partitioning = df.execution_plan().output_partitioning
182+
partitioning.scheme # 'Hash', 'RoundRobinBatch', 'Range', 'UnknownPartitioning'
183+
partitioning.partition_count
184+
partitioning.hash_expressions # display strings, or None
185+
```
186+
187+
This is additive; `partition_count` keeps working and agrees with
188+
`output_partitioning.partition_count`. Note that
189+
{py:class}`datafusion.expr.Partitioning` is a different type: that one is the
190+
*logical* partitioning {py:meth}`~datafusion.DataFrame.repartition_by_hash`
191+
takes as a request, while this one is what a built plan actually does.
192+
193+
### Two error paths that used to abort the interpreter
194+
195+
{py:meth}`~datafusion.SessionContext.execute` now raises `ValueError` for a
196+
partition index that is out of range. Previously the plan's leaves indexed
197+
their partitions directly and the resulting Rust panic surfaced as an error
198+
naming neither the plan nor the index.
199+
200+
{py:meth}`~datafusion.SessionConfig.set` now raises for a key whose namespace
201+
does not exist -- `datafusion.runtime.*`, or a config extension that has not
202+
been installed yet. Previously it aborted with a `PanicException`, which
203+
derives from `BaseException` and so escaped `except Exception`. This matters
204+
when replaying settings read back from `information_schema.df_settings`, which
205+
lists keys in both of those categories.
206+
172207
### Changes to the `datafusion-python-util` crate
173208

174209
Extension libraries written in Rust usually depend on the

‎examples/datafusion-ffi-example/src/physical_extension_codec.rs‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,9 +122,24 @@ impl PhysicalExtensionCodec for CountingPhysicalExtensionCodec {
122122
buf: &mut Vec<u8>,
123123
proto_converter: &dyn PhysicalProtoConverterExtension,
124124
) -> Result<()> {
125-
// The provider owns DataSourceExec. A ForeignExecutionPlan can wrap a
126-
// host-added execution decorator around that scan; retaining the opaque
127-
// wrapper preserves its original library identity without downcasting it.
125+
// `DataSourceExec` is this library's own node. The `ForeignExecutionPlan`
126+
// arm is a workaround, not a pattern to copy, and it is load-bearing:
127+
// a host physical optimizer rule that runs during a foreign planner's
128+
// `create_physical_plan` -- `EnsureCooperative` always does -- hands the
129+
// library back a `ForeignExecutionPlan` wrapping the host's
130+
// `CooperativeExec`. That type has no reachable `try_to_proto`, so
131+
// nothing can encode it natively and `FFI_QueryPlanner` must serialize
132+
// the plan it returns. Claiming it here is what lets those plans
133+
// round-trip at all.
134+
//
135+
// The cost is that this codec also claims every *other* library's
136+
// nodes, since that is the type any node arrives as once it has crossed
137+
// the boundary -- see `extension_codec_order`. Narrowing this to
138+
// `DataSourceExec` alone makes 31 tests in
139+
// `datafusion-ffi-query-planner-example` fail with the error above.
140+
//
141+
// A library whose planner controls its own physical optimizer rules
142+
// never sees a foreign node and needs no such arm.
128143
if node.is::<DataSourceExec>() || node.is::<ForeignExecutionPlan>() {
129144
self.counters
130145
.encode_execution_plan

‎python/datafusion/__init__.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,13 @@
100100
)
101101
from .io import read_avro, read_csv, read_json, read_parquet
102102
from .options import CsvReadOptions
103-
from .plan import ExecutionPlan, LogicalPlan, Metric, MetricsSet
103+
from .plan import (
104+
ExecutionPlan,
105+
LogicalPlan,
106+
Metric,
107+
MetricsSet,
108+
PhysicalPartitioning,
109+
)
104110
from .record_batch import RecordBatch, RecordBatchStream
105111
from .user_defined import (
106112
Accumulator,
@@ -133,6 +139,7 @@
133139
"MetricsSet",
134140
"ParquetColumnOptions",
135141
"ParquetWriterOptions",
142+
"PhysicalPartitioning",
136143
"QueryPlannerExportable",
137144
"RecordBatch",
138145
"RecordBatchStream",

0 commit comments

Comments
 (0)