Skip to content

Commit e046251

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 b6c6f5b commit e046251

7 files changed

Lines changed: 206 additions & 16 deletions

File tree

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,9 @@ guards this. Its `WHERE` clause is load-bearing: filter pushdown upgrades the
154154
weak handle during logical optimization, before plan serialization could fail
155155
first for an unrelated reason.
156156

157-
`SessionContext.enable_url_table` is the one method that mints a second
158-
allocation for a session. Its result must not outlive the receiver.
157+
`SessionContext.enable_url_table` follows the same rule: it replaces only the
158+
catalog list through `state_ref()` and returns a handle sharing the original
159+
allocation. Its idempotence check and catalog replacement share one write lock.
159160

160161
## Rule 7 — installing a planner mutates the session, and says so
161162

@@ -185,7 +186,7 @@ pins that; changing it should be deliberate.
185186

186187
## Where the truth is
187188

188-
- `docs/source/contributor-guide/ffi.md` — the protocol, the fork caveat.
189+
- `docs/source/contributor-guide/ffi.md` — the protocol and session ownership.
189190
- `docs/source/user-guide/upgrade-guides.md` — every past migration.
190191
- `crates/core/src/codec.rs` — the codec chain: the envelope, identity dispatch,
191192
and the two unframed cases from Rule 8.

crates/core/src/context.rs

Lines changed: 26 additions & 7 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,13 +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. Any
428-
// weak `FFI_TaskContextProvider` handed out by the receiver stays bound
429-
// to the receiver, so the returned context must not outlive it. See
430-
// `set_session_query_planner` for why everything else mutates in place.
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+
}
431450
Ok(PySessionContext {
432-
ctx: Arc::new(self.ctx.as_ref().clone().enable_url_table()),
451+
ctx: Arc::clone(&self.ctx),
433452
logical_codec: Arc::clone(&self.logical_codec),
434453
physical_codec: Arc::clone(&self.physical_codec),
435454
})

docs/source/contributor-guide/ffi.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -417,18 +417,21 @@ codec simply retain the session that built it: a codec handed to a provider is r
417417
registered straight back into that same session, which would close the cycle
418418
`SessionContext -> catalog -> FFI provider -> FFI codec -> SessionContext` and leak it.
419419

420-
`SessionContext.enable_url_table` is the one exception. It clones the underlying
421-
`SessionContext`, so the returned context has an allocation of its own and must not
422-
outlive the receiver.
420+
`SessionContext.enable_url_table` also preserves that allocation. It wraps the
421+
existing catalog list in place, so previously exported weak providers remain valid
422+
while any handle on the session is alive. Repeated calls do not nest catalog wrappers.
423423

424424
### What a derived context shares
425425

426-
`with_logical_extension_codec`, `with_physical_extension_codec`, and
426+
`enable_url_table`, `with_logical_extension_codec`, `with_physical_extension_codec`, and
427427
`with_python_udf_inlining` return a new `SessionContext` wrapping the *same* underlying
428428
session. Only the Python-side codec settings differ; catalogs, tables, registered
429429
functions, and configuration are the one shared session, so a registration on either
430430
side is visible to both.
431431

432+
Enabling URL tables takes effect on the shared session even if the returned handle
433+
is discarded. The returned handle keeps the receiver's codec settings unchanged.
434+
432435
`set_query_planner` does not return anything. The query planner lives in `SessionState`,
433436
so it is a property of the session rather than of a handle on it, and installing one is
434437
visible to every context sharing that session — including ones a `with_*` call returned

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 {ref}`ffi` hook
2553
methods. Users who contribute their own `LogicalExtensionCodec` or
2654
`PhysicalExtensionCodec` via FFI must update

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
@@ -76,6 +76,43 @@ def probe_context(
7676
return ctx, logical_codec, physical_codec
7777

7878

79+
@pytest.mark.parametrize("keep_returned", [False, True])
80+
def test_enable_url_table_preserves_ffi_providers(keep_returned):
81+
"""A catalog's unreachable weak codec remains valid through URL enabling."""
82+
ctx, logical_codec, physical_codec = probe_context(
83+
logical_requires=HOST_ONLY_UDF, physical_requires=HOST_ONLY_UDF, max_rows=100
84+
)
85+
ctx.register_catalog_provider("ffi_catalog", MyCatalogProvider())
86+
session_id = ctx.session_id()
87+
alias = ctx.with_python_udf_inlining(enabled=True)
88+
if keep_returned:
89+
ctx = alias.enable_url_table()
90+
else:
91+
alias.enable_url_table()
92+
del alias
93+
gc.collect()
94+
95+
for _ in range(2):
96+
# Force filter pushdown through a real foreign catalog before planning.
97+
batches = ctx.sql(
98+
"SELECT units FROM ffi_catalog.my_schema.my_table WHERE units > 5"
99+
).collect()
100+
assert sorted(v for b in batches for v in b.column(0).to_pylist()) == [
101+
7,
102+
10,
103+
20,
104+
30,
105+
]
106+
assert logical_codec.table_provider_decode_calls() > 0
107+
assert physical_codec.execution_plan_decode_calls() > 0
108+
assert logical_codec.last_task_context_session_id() == session_id
109+
assert physical_codec.last_task_context_session_id() == session_id
110+
assert logical_codec.task_context_udf_resolutions() > 0
111+
assert physical_codec.task_context_udf_resolutions() > 0
112+
ctx = ctx.enable_url_table()
113+
gc.collect()
114+
115+
79116
def test_logical_codec_resolves_a_host_registered_udf():
80117
"""``try_decode_table_provider`` sees the host session's registry.
81118

python/datafusion/context.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -594,10 +594,26 @@ def global_ctx(cls) -> SessionContext:
594594
return wrapper
595595

596596
def enable_url_table(self) -> SessionContext:
597-
"""Control if local files can be queried as tables.
597+
"""Enable querying local files as tables on the shared session.
598+
599+
The receiver and all handles sharing its session gain URL table support,
600+
even if the returned handle is discarded. Repeated calls have no effect.
601+
Registered catalogs, functions, configuration, and session identity are
602+
preserved, as are FFI providers bound to the session.
598603
599604
Returns:
600-
A new :py:class:`SessionContext` object with url table enabled.
605+
A new :py:class:`SessionContext` handle wrapping the same session.
606+
607+
Examples:
608+
>>> ctx = SessionContext()
609+
>>> enabled = ctx.enable_url_table()
610+
>>> enabled.session_id() == ctx.session_id()
611+
True
612+
>>> ctx.sql("SET datafusion.execution.batch_size = 111").collect()
613+
[]
614+
>>> batches = enabled.sql("SHOW datafusion.execution.batch_size").collect()
615+
>>> batches[0].column(1).to_pylist()
616+
['111']
601617
"""
602618
klass = self.__class__
603619
obj = klass.__new__(klass)

python/tests/test_context.py

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import gzip
2020
import pathlib
2121
import shutil
22+
from concurrent.futures import ThreadPoolExecutor
2223

2324
import pyarrow as pa
2425
import pyarrow.dataset as ds
@@ -41,6 +42,91 @@ def test_create_context_no_args():
4142
SessionContext()
4243

4344

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

0 commit comments

Comments
 (0)