Add PyO3 class mutability guidelines reference to contributor guide

Author: kosiew

docs/source/contributor-guide/ffi.rst

@@ -137,6 +137,67 @@ and you want to create a sharable FFI counterpart, you could write:
     let my_provider = MyTableProvider::default();
     let ffi_provider = FFI_TableProvider::new(Arc::new(my_provider), false, None);
 
+.. _ffi_pyclass_mutability:
+
+PyO3 class mutability guidelines
+--------------------------------
+
+PyO3 bindings should present immutable wrappers whenever a struct stores shared or
+interior-mutable state. In practice this means that any ``#[pyclass]`` containing an
+``Arc<RwLock<_>>`` or similar synchronized primitive must opt into ``#[pyclass(frozen)]``
+unless there is a compelling reason not to.
+
+The :mod:`datafusion` configuration helpers illustrate the preferred pattern. The
+``PyConfig`` class in :file:`src/config.rs` stores an ``Arc<RwLock<ConfigOptions>>`` and is
+explicitly frozen so callers interact with configuration state through provided methods
+instead of mutating the container directly:
+
+.. code-block:: rust
+
+    #[pyclass(name = "Config", module = "datafusion", subclass, frozen)]
+    #[derive(Clone)]
+    pub(crate) struct PyConfig {
+        config: Arc<RwLock<ConfigOptions>>,
+    }
+
+The same approach applies to execution contexts. ``PySessionContext`` in
+:file:`src/context.rs` stays frozen even though it shares mutable state internally via
+``SessionContext``. This ensures PyO3 tracks borrows correctly while Python-facing APIs
+clone the inner ``SessionContext`` or return new wrappers instead of mutating the
+existing instance in place:
+
+.. code-block:: rust
+
+    #[pyclass(frozen, name = "SessionContext", module = "datafusion", subclass)]
+    #[derive(Clone)]
+    pub struct PySessionContext {
+        pub ctx: SessionContext,
+    }
+
+Occasionally a type must remain mutable—for example when PyO3 attribute setters need to
+update fields directly. In these rare cases add an inline justification so reviewers and
+future contributors understand why ``frozen`` is unsafe to enable. ``DataTypeMap`` in
+:file:`src/common/data_type.rs` includes such a comment because PyO3 still needs to track
+field updates:
+
+.. code-block:: rust
+
+    // TODO: This looks like this needs pyo3 tracking so leaving unfrozen for now
+    #[derive(Debug, Clone)]
+    #[pyclass(name = "DataTypeMap", module = "datafusion.common", subclass)]
+    pub struct DataTypeMap {
+        #[pyo3(get, set)]
+        pub arrow_type: PyDataType,
+        #[pyo3(get, set)]
+        pub python_type: PythonType,
+        #[pyo3(get, set)]
+        pub sql_type: SqlType,
+    }
+
+When reviewers encounter a mutable ``#[pyclass]`` without a comment, they should request
+an explanation or ask that ``frozen`` be added. Keeping these wrappers frozen by default
+helps avoid subtle bugs stemming from PyO3's interior mutability tracking.
+
 If you were interfacing with a library that provided the above ``FFI_TableProvider`` and
 you needed to turn it back into an ``TableProvider``, you can turn it into a
 ``ForeignTableProvider`` with implements the ``TableProvider`` trait.

docs/source/contributor-guide/introduction.rst

@@ -26,6 +26,10 @@ We welcome and encourage contributions of all kinds, such as:
 In addition to submitting new PRs, we have a healthy tradition of community members reviewing each other’s PRs.
 Doing so is a great way to help the community as well as get more familiar with Rust and the relevant codebases.
 
+Before opening a pull request that touches PyO3 bindings, please review the
+:ref:`PyO3 class mutability guidelines <ffi_pyclass_mutability>` so you can flag missing
+``#[pyclass(frozen)]`` annotations during development and review.
+
 How to develop
 --------------