enable_url_table returns a handle on a different session, unlike every other with_* derivation
还没有人认领这个 Issue。
评估
- 难度
- 4/5
- 预计耗时
- 3-5 天
- 新手友好度
- 55/100
- Issue 类型
- 缺陷
- 描述清晰度
- 描述清楚
- 活跃度
- 活跃
- 领域
- api, documentation
调研方向
从 crates/core/src/context.rs:425-428 开始,将 enable_url_table 与 set_session_query_planner (1729-1740) 以及 with_python_udf_inlining (1571-1588) 进行比较。运行 issue 的 Python reproduction,并检查 examples/create-context.py:41 以及列出的 FFI 和 Upgrade Guide 部分。完成的标准是:重复调用是安全的,handles 共享同一个 session 和 state,绑定到 FFI 的组件保持有效,并且所引用的文档和 docstring 描述了共享行为。
由索引模型根据 Issue 内容生成。
描述
Describe the bug
SessionContext.enable_url_table is the only derivation in the binding that returns a handle on a different session than the one it was called on, and it is the only one that leaves two live handles reporting the same session_id with divergent state. Every other with_* method returns a handle wrapping the same underlying session.
The root cause is one line. Upstream's SessionContext::enable_url_table consumes self (datafusion/core/src/execution/context/mod.rs:414):
pub fn enable_url_table(self) -> Self {
let current_catalog_list = Arc::clone(self.state.read().catalog_list());
let factory = Arc::new(DynamicListTableFactory::new(SessionStore::new()));
let catalog_list = Arc::new(DynamicFileCatalog::new(
current_catalog_list,
Arc::clone(&factory) as Arc<dyn UrlTableFactory>,
));
let session_id = self.session_id.clone();
let ctx: SessionContext = self
.into_state_builder()
.with_session_id(session_id)
.with_catalog_list(catalog_list)
.build()
.into();
factory.session_store().with_state(ctx.state_weak_ref());
ctx
}
Taking self by value means the old and new contexts are never meant to coexist, which is exactly why carrying session_id over is correct there. The binding at crates/core/src/context.rs:425 takes &self and clones instead:
ctx: Arc::new(self.ctx.as_ref().clone().enable_url_table()),
That manufactures the coexistence upstream's signature prevents, and everything else is downstream of it: the must-not-outlive caveat at crates/core/src/context.rs:428, the "one exception" paragraph at docs/source/contributor-guide/ffi.md:475-477, and the state divergence below.
To Reproduce
from datafusion import SessionContext
a = SessionContext()
b = a.enable_url_table()
print("same session_id:", a.session_id() == b.session_id())
a.sql("SET datafusion.execution.batch_size = 111").collect()
print("a", a.sql("SHOW datafusion.execution.batch_size").collect()[0].column(1).to_pylist())
print("b", b.sql("SHOW datafusion.execution.batch_size").collect()[0].column(1).to_pylist())
same session_id: True
a ['111']
b ['8192']
Two live sessions with independent SessionState, both reporting one id.
The second half of the problem is not reproducible without a built FFI extension, but it is what the existing caveat comments are about: any FFI codec, query planner, or task-context provider handed out by the receiver holds an FFI_TaskContextProvider bound weakly to the receiver's Arc<SessionContext>. The returned context is a different allocation, so ctx = ctx.enable_url_table() drops the last strong reference and those components fail at query time with TaskContextProvider went out of scope over FFI boundary. examples/create-context.py:41 is written as ctx = ctx.enable_url_table() — exactly the pattern the caveat forbids. Harmless in that example because it installs no FFI components, but it is the shape users copy.
Expected behavior
enable_url_table should behave like with_logical_extension_codec, with_physical_extension_codec, and with_python_udf_inlining: return a handle on the same underlying session, so session_id stays unique to a session, state cannot diverge, and FFI components bound to any handle in the lineage stay valid.
The pattern already exists in the same file. set_session_query_planner (crates/core/src/context.rs:1729-1740) mutates through state_ref() rather than deriving a new context, including the load-bearing with_session_id carry-over:
let state_ref = self.ctx.state_ref();
let factory = Arc::new(DynamicListTableFactory::new(SessionStore::new()));
let catalog_list = Arc::new(DynamicFileCatalog::new(
Arc::clone(state_ref.read().catalog_list()),
Arc::clone(&factory) as Arc<dyn UrlTableFactory>,
));
{
let mut guard = state_ref.write();
*guard = SessionStateBuilder::new_from_existing(guard.clone())
.with_session_id(guard.session_id().to_string())
.with_catalog_list(catalog_list)
.build();
}
factory.session_store().with_state(Arc::downgrade(&state_ref));
The returned handle then shares Arc::clone(&self.ctx) like every sibling.
Two details for whoever picks this up:
- Needs an idempotence guard.
DynamicFileCatalog::newwraps whatever catalog list is current, so callingenable_url_tabletwice nests wrappers. Upstream has the same wart, but mutating in place makes it easier to hit.with_python_udf_inlining(crates/core/src/context.rs:1571-1588) already establishes the pattern of skipping the state rebuild when the call would change nothing. - This is an
api change.ctx = ctx.enable_url_table()keeps working, but the original handle also gains url tables afterwards. That is the same tradewith_logical_extension_codecalready documents — it takes effect on the shared session even if the returned context is discarded — so this is bringingenable_url_tablein line rather than introducing a new surprise. Needs a section indocs/source/user-guide/upgrade-guides.md, plus updates to the docstring atpython/datafusion/context.py:589, the "one exception" paragraph atdocs/source/contributor-guide/ffi.md:475-477, and the shared-derivation list atdocs/source/contributor-guide/ffi.md:481-486.
Additional context
Found while reviewing #1679. That PR's _derive_for_extensions currently forks state the same way, so the "one method that mints a second Arc<SessionContext>" comment at crates/core/src/context.rs:428 is temporarily inaccurate. If #1679 lands with that fork removed, enable_url_table is once again the only site and that comment becomes true again.
The two are independent in both directions and should not be chained: #1679 does not need this fixed, and this does not need #1679.
- 主要语言
- Python
- 星标
- 605
- 派生
- 176
- 平均合并
- 1 天 23 小时
- 30 天内合并 PR
- 8
贡献指南
这个仓库没有索引到贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
apache/datafusion-python 的其他 Issue
-
documentation
难度 2/5 1-3 小时 新手友好度 72/100
apache/datafusion-python#1726 ·
-
难度 2/5 半天 新手友好度 88/100
apache/datafusion-python#1691 ·
-
bug
难度 2/5 1-3 小时 新手友好度 78/100
apache/datafusion-python#1644 ·
-
enhancement
难度 5/5 一周以上 新手友好度 30/100
apache/datafusion-python#1737 ·
-
难度 3/5 1-2 天 新手友好度 76/100
apache/datafusion-python#1735 · 1 条评论 ·
查看 apache/datafusion-python 的全部 Issue
相似的 Issue
-
bug
难度 2/5 1-3 小时 新手友好度 90/100
learningequality/ricecooker#747 ·
-
难度 2/5 1-3 小时 新手友好度 68/100
BSData/horus-heresy-3rd-edition#3171 ·
-
enhancement
难度 2/5 1-3 小时 新手友好度 72/100
-
难度 2/5 1-3 小时 新手友好度 76/100
run-llama/llama_index#23199 ·
-
难度 2/5 1-3 小时 新手友好度 84/100
KhronosGroup/glTF-Blender-IO#2769 ·