timsaucer opened a new pull request, #1740: URL: https://github.com/apache/datafusion-python/pull/1740
# Which issue does this PR close? Part 3 of 4 toward #1676. Stacked on #1739, which is stacked on #1738. Review those first; this PR's diff against #1739 is the third commit only. # Rationale for this change #1738 and #1739 covered every component whose capsule getter takes no argument. Tables and table functions are the other kind — their getters take the session — and that difference is the whole of this PR. It is also the second demonstrated gap from #1721: `build_session` registers tables as a separate step 4, after `with_extensions`. # What changes are included in this PR? **`udtfs` and `table_providers`, both as `(name, value)` pairs.** Neither carries a name of its own the way a scalar function's capsule does, so the name is given alongside the value. `table_providers` accepts anything `register_table` does; names may be qualified. **The host resolves them, and the bundle must not.** `__datafusion_table_function__` and `__datafusion_table_provider__` are handed the session, and `__datafusion_table_function__` pulls the host's logical codec straight off it. The context a components hook receives has none of the call's codecs installed yet, so a bundle that wrapped its own `TableFunction` would capture a chain missing every library in the call — including its own — and nothing would go wrong until a decode in another process. So bundles hand over the unwrapped value and the host wraps it against the finished handle, in the resolve step. That is a claim worth asserting rather than describing, so `RecordingTableFunction` in the example crate records the codec ids of whatever session it is resolved against, and `test_a_declared_table_function_sees_the_finished_codec_chain` installs it alongside a *second* bundle that contributes a codec. The recorded ids match the finished chain and are non-empty; resolving against the hook's context would record `[]`. **Tables do not shadow.** DataFusion refuses a duplicate table registration rather than replacing it, so unlike a function, a declared table name that is already on the session is an error — not only one that two bundles both claim. Both cases are caught during resolution, along with resolving the destination schema, so a bad or qualified-but-unknown name costs nothing and the rest of the call is left unwritten. **`_resolve_extension_tables` / `_install_extension_tables`**, following the split #1739 established, with one exception I want to be explicit about rather than paper over: the insert goes through a `SchemaProvider`, and a *foreign* one can still refuse a registration it reported as available. That is the one place in `with_extensions` that can leave a call part-applied. Tables are therefore committed **first**, so when it happens no planner has been bound and no function registered behind it. `extension-guide/bundles.md` states this as an exception to the infallible-commit rule rather than claiming a guarantee that does not hold. **Docs.** `bundles.md` gains an `extension_bundles_binding` section splitting components by what their getter asks for, which is the rule the rest of the stack is built on; the transaction section gains the table exception; the collision section gains the no-shadowing rule. `table-providers.md` and `functions.md` each gain the bundle form and a pointer to why the value is handed over unwrapped. `user-guide/extensions.md` no longer claims tables always register directly. # Are there any user-facing changes? Two new optional fields on `SessionExtensionComponents`, defaulting to `()`. No existing behaviour changes: `register_table` and `register_udtf` are untouched, and the new resolution path is only reachable through `with_extensions`. No upgrade-guide entry and no `api change` label. # Review notes **The pair shape is worth arguing about.** `udfs` accepts either a wrapper or a raw exportable; `udtfs` accepts only the raw value plus a name. That is not an inconsistency for its own sake — a `TableFunction` can only be built by calling the getter with *some* session, and a bundle does not have the right one. Accepting a pre-built one would mean accepting one bound to the wrong chains. If you would rather it accepted both and documented the hazard, say so. **`table_exist` then insert is a check-then-act.** Under the GIL, within a single `with_extensions` call, nothing else is registering concurrently. The alternative is to drop the check and let the duplicate surface from the insert, which would report the same problem after the call had already written the tables ahead of it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
