Skip to content

Commit 111ba80

Browse files
committed
fix: enable URL tables on the shared session
Preserve context and state identity, initialize the URL factory before publishing it, and avoid nested catalog wrappers under one write lock. Cover SQL configuration, registrations, aliases, and real FFI provider lifetimes; document the API change for #1708. Generated-by: Codex (GPT-6)
1 parent 7022baa commit 111ba80

8 files changed

Lines changed: 220 additions & 48 deletions

File tree

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

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -187,13 +187,9 @@ an instruction to derive one first. It is not: the factories are handed the
187187
receiver, and the returned handle shares its allocation. There is nothing to
188188
keep alive separately and nothing to garbage-collect out from under a provider.
189189

190-
`SessionContext.enable_url_table` is the one method that mints a second
191-
allocation for a session. Its result must not outlive the receiver, and it also
192-
forks the session's `SessionState` while keeping its id, so two handles report
193-
one `session_id()` with divergent configuration. That is a bug rather than a
194-
design — tracked in
195-
[apache/datafusion-python#1708](https://github.com/apache/datafusion-python/issues/1708)
196-
— so do not cite it as precedent for deriving a replacement context.
190+
`SessionContext.enable_url_table` follows the same rule: it replaces only the
191+
catalog list through `state_ref()` and returns a handle sharing the original
192+
allocation. Its idempotence check and catalog replacement share one write lock.
197193

198194
## Rule 7 — installing a planner mutates the session, and says so
199195

crates/core/src/context.rs

Lines changed: 33 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,11 @@ use arrow::pyarrow::FromPyArrow;
2727
use datafusion::arrow::datatypes::{DataType, Schema, SchemaRef};
2828
use datafusion::arrow::pyarrow::PyArrowType;
2929
use datafusion::arrow::record_batch::RecordBatch;
30-
use datafusion::catalog::{CatalogProvider, CatalogProviderList, TableProviderFactory};
30+
use datafusion::catalog::{
31+
CatalogProvider, CatalogProviderList, DynamicFileCatalog, TableProviderFactory, UrlTableFactory,
32+
};
3133
use datafusion::common::{DFSchema, ScalarValue, TableReference, exec_err};
34+
use datafusion::datasource::dynamic_file::DynamicListTableFactory;
3235
use datafusion::datasource::file_format::file_compression_type::FileCompressionType;
3336
use datafusion::datasource::file_format::parquet::ParquetFormat;
3437
use datafusion::datasource::listing::{
@@ -423,15 +426,29 @@ impl PySessionContext {
423426
}
424427

425428
pub fn enable_url_table(&self) -> PyResult<Self> {
426-
// Pre-existing caveat, unrelated to query planners: this is the one
427-
// method that mints a second `Arc<SessionContext>` for a session, and
428-
// it also forks the session's state while keeping its id. Any weak
429-
// `FFI_TaskContextProvider` handed out by the receiver stays bound to
430-
// the receiver, so the returned context must not outlive it. See
431-
// `set_session_query_planner` for why everything else mutates in place.
432-
// Tracked as a bug in <https://github.com/apache/datafusion-python/issues/1708>.
429+
let state_ref = self.ctx.state_ref();
430+
{
431+
// Check and replace under one lock so concurrent calls cannot nest
432+
// wrappers or overwrite a newer catalog list.
433+
let mut state = state_ref.write();
434+
if !state.catalog_list().is::<DynamicFileCatalog>() {
435+
let factory = Arc::new(DynamicListTableFactory::default());
436+
// Bind before publishing the catalog: a reader must never see
437+
// a factory whose session store has not been initialized.
438+
factory
439+
.session_store()
440+
.with_state(self.ctx.state_weak_ref());
441+
let catalog_list = Arc::new(DynamicFileCatalog::new(
442+
Arc::clone(state.catalog_list()),
443+
factory as Arc<dyn UrlTableFactory>,
444+
));
445+
// Only the catalog changes. In particular, preserve the state
446+
// and context allocations targeted by weak FFI providers.
447+
state.register_catalog_list(catalog_list);
448+
}
449+
}
433450
Ok(PySessionContext {
434-
ctx: Arc::new(self.ctx.as_ref().clone().enable_url_table()),
451+
ctx: Arc::clone(&self.ctx),
435452
logical_codec: Arc::clone(&self.logical_codec),
436453
physical_codec: Arc::clone(&self.physical_codec),
437454
})
@@ -1434,14 +1451,13 @@ impl PySessionContext {
14341451
/// time, and a payload written through one would resolve to the other on
14351452
/// decode. See [`SESSION_CODEC_ID_PREFIX`].
14361453
///
1437-
/// Handles derived from one session — `with_python_udf_inlining`,
1438-
/// `with_logical_extension_codec`, [`Self::_install_extension_codecs`] —
1439-
/// report the same id even though their codec chains differ, so installing
1440-
/// two of them on one target is refused. That is the intended answer: they
1441-
/// share a `state_ref`, so their payloads would resolve against the same
1442-
/// session and are indistinguishable on decode. Every derivation shares the
1443-
/// session for exactly this reason; `enable_url_table` is the one that does
1444-
/// not, and it is tracked as a bug.
1454+
/// Handles derived from one session — `enable_url_table`,
1455+
/// `with_python_udf_inlining`, `with_logical_extension_codec`,
1456+
/// [`Self::_install_extension_codecs`] — report the same id even though
1457+
/// their codec chains may differ, so installing two of them on one target
1458+
/// is refused. That is the intended answer: they share a `state_ref`, so
1459+
/// their payloads would resolve against the same session and are
1460+
/// indistinguishable on decode.
14451461
#[getter]
14461462
pub fn __datafusion_codec_id__(&self) -> String {
14471463
format!("{SESSION_CODEC_ID_PREFIX}{}", self.ctx.session_id())

docs/source/contributor-guide/ffi-internals.md

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,10 @@ would close the cycle
6464
`SessionContext -> catalog -> FFI provider -> FFI codec -> SessionContext` and
6565
leak it.
6666

67-
`SessionContext.enable_url_table` is the one exception. It clones the
68-
underlying `SessionContext`, so the returned context has an allocation of its
69-
own and must not outlive the receiver. It also forks the session's state while
70-
keeping its id, so two handles report one `session_id()` with divergent
71-
configuration. That is a bug rather than a design, tracked in
72-
[apache/datafusion-python#1708](https://github.com/apache/datafusion-python/issues/1708);
73-
do not copy the pattern.
67+
`SessionContext.enable_url_table` follows the same rule. It replaces only the
68+
catalog list through `state_ref()` and returns a handle sharing the original
69+
allocation. The idempotence check and catalog replacement share one write lock,
70+
so concurrent calls cannot nest wrappers or overwrite a newer catalog list.
7471

7572
(ffi_internals_rebinding)=
7673

docs/source/extension-guide/sessions.md

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ divergence.
5252

5353
## What a derived context shares
5454

55+
{py:meth}`~datafusion.SessionContext.enable_url_table`,
5556
{py:meth}`~datafusion.SessionContext.with_logical_extension_codec`,
5657
{py:meth}`~datafusion.SessionContext.with_physical_extension_codec`,
5758
{py:meth}`~datafusion.SessionContext.with_python_udf_inlining`, and
@@ -105,13 +106,6 @@ The same rule applies to a capsule you take off a context inside your own code:
105106
a codec capsule taken from a throwaway `SessionContext()` names a session that
106107
is already gone and fails on first use.
107108

108-
:::{warning}
109-
{py:meth}`~datafusion.SessionContext.enable_url_table` is an exception to the
110-
one-session-one-allocation rule above: it clones the underlying
111-
`SessionContext`, so the returned context has an allocation of its own and must
112-
not outlive the receiver. It also forks the session's state while keeping its
113-
id, so two handles report one `session_id()` with divergent configuration. That
114-
is a bug rather than a design, tracked in
115-
[apache/datafusion-python#1708](https://github.com/apache/datafusion-python/issues/1708);
116-
do not build on the behaviour.
117-
:::
109+
Enabling URL tables takes effect on the shared session even if the returned
110+
handle is discarded. The returned handle keeps the receiver's codec settings
111+
unchanged, and repeated calls do not nest catalog wrappers.

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,34 @@
2121

2222
## DataFusion 55.0.0
2323

24+
### URL tables share the original session
25+
26+
`SessionContext.enable_url_table()` now enables URL tables on the existing session
27+
and returns another handle on it. Previously it copied session state into a separate
28+
session while retaining the same session id. Configuration and function registrations
29+
could then diverge, and replacing the original handle could invalidate FFI providers.
30+
31+
Before, callers had to use the returned context to query file paths:
32+
33+
```python
34+
ctx = SessionContext()
35+
enabled = ctx.enable_url_table()
36+
# Only enabled could query local file paths as tables.
37+
```
38+
39+
After, both handles share configuration, registrations, and URL table support:
40+
41+
```python
42+
ctx = SessionContext()
43+
ctx.enable_url_table() # Takes effect even when the returned handle is discarded.
44+
```
45+
46+
Existing `ctx = ctx.enable_url_table()` calls continue to work and now retain FFI
47+
providers bound to the original session. Repeated calls have no effect. To keep a
48+
session without URL table support, create a separate `SessionContext` explicitly.
49+
50+
### FFI codec hooks receive the session
51+
2452
This release extends the change made in 52.0.0 to the remaining
2553
{ref}`extension_capsule_protocol` hook methods. Users who contribute their own
2654
`LogicalExtensionCodec` or

examples/datafusion-ffi-query-planner-example/python/tests/_test_three_library_query_planner.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,43 @@ def probe_context(
9393
return ctx, logical_codec, physical_codec
9494

9595

96+
@pytest.mark.parametrize("keep_returned", [False, True])
97+
def test_enable_url_table_preserves_ffi_providers(keep_returned):
98+
"""A catalog's unreachable weak codec remains valid through URL enabling."""
99+
ctx, logical_codec, physical_codec = probe_context(
100+
logical_requires=HOST_ONLY_UDF, physical_requires=HOST_ONLY_UDF, max_rows=100
101+
)
102+
ctx.register_catalog_provider("ffi_catalog", MyCatalogProvider())
103+
session_id = ctx.session_id()
104+
alias = ctx.with_python_udf_inlining(enabled=True)
105+
if keep_returned:
106+
ctx = alias.enable_url_table()
107+
else:
108+
alias.enable_url_table()
109+
del alias
110+
gc.collect()
111+
112+
for _ in range(2):
113+
# Force filter pushdown through a real foreign catalog before planning.
114+
batches = ctx.sql(
115+
"SELECT units FROM ffi_catalog.my_schema.my_table WHERE units > 5"
116+
).collect()
117+
assert sorted(v for b in batches for v in b.column(0).to_pylist()) == [
118+
7,
119+
10,
120+
20,
121+
30,
122+
]
123+
assert logical_codec.table_provider_decode_calls() > 0
124+
assert physical_codec.execution_plan_decode_calls() > 0
125+
assert logical_codec.last_task_context_session_id() == session_id
126+
assert physical_codec.last_task_context_session_id() == session_id
127+
assert logical_codec.task_context_udf_resolutions() > 0
128+
assert physical_codec.task_context_udf_resolutions() > 0
129+
ctx = ctx.enable_url_table()
130+
gc.collect()
131+
132+
96133
def test_logical_codec_resolves_a_host_registered_udf():
97134
"""``try_decode_table_provider`` sees the host session's registry.
98135

python/datafusion/context.py

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,8 @@ class SessionContext:
551551
See :ref:`user_guide_concepts` in the online documentation for more information.
552552
553553
**A context is a handle on a session, not the session itself.** The
554-
``with_*`` methods — :py:meth:`with_logical_extension_codec`,
554+
derivation methods — :py:meth:`enable_url_table`,
555+
:py:meth:`with_logical_extension_codec`,
555556
:py:meth:`with_physical_extension_codec`,
556557
:py:meth:`with_python_udf_inlining`, and :py:meth:`with_extensions` —
557558
return a new context wrapping the *same* underlying session. Only the
@@ -560,10 +561,11 @@ class SessionContext:
560561
either handle is visible to both.
561562
562563
A few things therefore belong to the session rather than to a handle, and
563-
take effect even if the handle that set them is discarded: the query
564-
planner (see :py:meth:`set_query_planner`), and the rebuild of an installed
565-
foreign planner that follows installing a codec. :ref:`extension_sessions`
566-
in the online documentation works through when that matters.
564+
take effect even if the handle that set them is discarded: URL table
565+
support, the query planner (see :py:meth:`set_query_planner`), and the
566+
rebuild of an installed foreign planner that follows installing a codec.
567+
:ref:`extension_sessions` in the online documentation works through when
568+
that matters.
567569
568570
**Keep a context alive for as long as anything derived from it is in use.**
569571
A :py:class:`~datafusion.DataFrame`, logical plan, or exported capsule does
@@ -619,10 +621,26 @@ def global_ctx(cls) -> SessionContext:
619621
return wrapper
620622

621623
def enable_url_table(self) -> SessionContext:
622-
"""Control if local files can be queried as tables.
624+
"""Enable querying local files as tables on the shared session.
625+
626+
The receiver and all handles sharing its session gain URL table support,
627+
even if the returned handle is discarded. Repeated calls have no effect.
628+
Registered catalogs, functions, configuration, and session identity are
629+
preserved, as are FFI providers bound to the session.
623630
624631
Returns:
625-
A new :py:class:`SessionContext` object with url table enabled.
632+
A new :py:class:`SessionContext` handle wrapping the same session.
633+
634+
Examples:
635+
>>> ctx = SessionContext()
636+
>>> enabled = ctx.enable_url_table()
637+
>>> enabled.session_id() == ctx.session_id()
638+
True
639+
>>> ctx.sql("SET datafusion.execution.batch_size = 111").collect()
640+
[]
641+
>>> batches = enabled.sql("SHOW datafusion.execution.batch_size").collect()
642+
>>> batches[0].column(1).to_pylist()
643+
['111']
626644
"""
627645
klass = self.__class__
628646
obj = klass.__new__(klass)

python/tests/test_context.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import gzip
2121
import pathlib
2222
import shutil
23+
from concurrent.futures import ThreadPoolExecutor
2324

2425
import pyarrow as pa
2526
import pyarrow.dataset as ds
@@ -43,6 +44,91 @@ def test_create_context_no_args():
4344
SessionContext()
4445

4546

47+
@pytest.mark.parametrize("discard_returned", [False, True])
48+
def test_enable_url_table_shares_session(tmp_path, discard_returned):
49+
"""URL tables, configuration, and registrations belong to all aliases."""
50+
ctx = SessionContext()
51+
original_config = (
52+
ctx.sql("SHOW datafusion.catalog.create_default_catalog_and_schema")
53+
.collect()[0]
54+
.column(1)
55+
.to_pylist()
56+
)
57+
alias = ctx.with_python_udf_inlining(enabled=False)
58+
session_id = ctx.session_id()
59+
ctx.sql("CREATE SCHEMA existing").collect()
60+
ctx.register_record_batches("existing.numbers", [[pa.record_batch({"n": [7]})]])
61+
ctx.register_udf(
62+
udf(lambda x: x, [pa.int64()], pa.int64(), "immutable", "identity")
63+
)
64+
path = tmp_path / "numbers.csv"
65+
path.write_text("n\n7\n")
66+
67+
if discard_returned:
68+
alias.enable_url_table()
69+
returned = ctx
70+
else:
71+
returned = alias.enable_url_table()
72+
73+
# Both directions must see subsequent SETs, not only the initial snapshot.
74+
for writer, reader, value in [(ctx, returned, 111), (returned, alias, 222)]:
75+
writer.sql(f"SET datafusion.execution.batch_size = {value}").collect()
76+
assert reader.sql("SHOW datafusion.execution.batch_size").collect()[0].column(
77+
1
78+
).to_pylist() == [str(value)]
79+
80+
returned.register_udf(
81+
udf(lambda x: x, [pa.int64()], pa.int64(), "immutable", "later_identity")
82+
)
83+
for handle in (ctx, alias, returned):
84+
assert handle.session_id() == session_id
85+
assert (
86+
handle.sql("SHOW datafusion.catalog.create_default_catalog_and_schema")
87+
.collect()[0]
88+
.column(1)
89+
.to_pylist()
90+
== original_config
91+
)
92+
assert handle.sql("SELECT identity(n) FROM existing.numbers").collect()[
93+
0
94+
].column(0).to_pylist() == [7]
95+
assert handle.sql("SELECT later_identity(9)").collect()[0].column(
96+
0
97+
).to_pylist() == [9]
98+
assert handle.sql(f'SELECT n FROM "{path}"').collect()[0].column(
99+
0
100+
).to_pylist() == [7]
101+
handle.enable_url_table()
102+
handle.enable_url_table()
103+
assert handle.sql(f'SELECT n FROM "{path}"').collect()[0].column(
104+
0
105+
).to_pylist() == [7]
106+
107+
108+
def test_enable_url_table_from_multiple_aliases(tmp_path):
109+
"""Enabling through multiple handles preserves concurrent registrations."""
110+
ctx = SessionContext()
111+
path = tmp_path / "numbers.csv"
112+
path.write_text("n\n7\n")
113+
aliases = [ctx.with_python_udf_inlining(enabled=False) for _ in range(4)]
114+
115+
def enable_and_register(index):
116+
alias = aliases[index]
117+
for _ in range(4):
118+
alias.enable_url_table()
119+
alias.sql(f'SELECT n FROM "{path}"').collect()
120+
alias.register_udf(
121+
udf(lambda x: x, [pa.int64()], pa.int64(), "immutable", f"identity_{index}")
122+
)
123+
124+
with ThreadPoolExecutor(max_workers=4) as executor:
125+
list(executor.map(enable_and_register, range(4)))
126+
for index in range(4):
127+
assert ctx.sql(f"SELECT identity_{index}(7)").collect()[0].column(
128+
0
129+
).to_pylist() == [7]
130+
131+
46132
def test_create_context_session_config_only():
47133
SessionContext(config=SessionConfig())
48134

0 commit comments

Comments
 (0)