timsaucer commented on code in PR #1678: URL: https://github.com/apache/datafusion-python/pull/1678#discussion_r3935993010
########## docs/source/contributor-guide/ffi.md: ########## @@ -248,10 +248,96 @@ foreign planner. This lets the planner decode provider-owned objects and lets process-local tokens to demonstrate ownership; production codecs should serialize durable metadata instead. -The current Python API has one external logical codec and one external physical codec. -Installing another codec replaces the prior codec rather than composing a registry. -The example therefore has one external codec owner, and the planner uses built-in -physical nodes. Install the provider codecs before the planner where possible. +### Composable codecs + +Extension codecs compose. Each call to `with_logical_extension_codec` or +`with_physical_extension_codec` appends the codec to the session's codec chain +rather than replacing prior codecs. + +**Nothing is asked of the codec itself.** Implement `LogicalExtensionCodec` or +`PhysicalExtensionCodec` exactly as you would for a session that installs only +yours. When your codec writes bytes into a serialized plan, datafusion-python +records which codec wrote them, and strips that record off again before handing the +bytes back. So your codec receives, byte for byte, the payload it wrote, and is +never offered a payload another codec wrote. + +A codec that also ships to hosts which dispatch differently may still want its own +guard against foreign payloads. Keeping one is fine; it is simply not needed for the +datafusion-python path. + +That record is the codec's **id**: a short string stored inside the plan, naming the +codec that wrote each payload. Because plans are decoded in another process — or +another program — the id has to name the same codec there as it did where the plan +was written. + +Ids are assigned for you. A codec's id is normally its exporting class's import +path, such as `my_library.Codec`, which is what you will see in +`logical_extension_codec_ids()` and in decode errors. You choose one yourself in +three cases: + +- **Two instances of one class.** Both get the same id, so the second install + raises `ValueError`. Pass `codec_id=` to tell them apart. +- **A bare `PyCapsule`.** A capsule has no class to take a name from, so it gets an + id private to the session that installed it. Plans it encodes fail with a clear + error on any other session, rather than being decoded by the wrong codec. Pass + `codec_id=` if those plans have to cross sessions. +- **A class you intend to rename.** The id follows the class name, so renaming stops + older plans from decoding. Declare `__datafusion_codec_id__` on the exporting + object to pin an id that survives the rename. + +`SessionContext.logical_extension_codec_ids()` and its physical counterpart list the +ids installed on a session, which is also what a decode failure names. + +Installing one context's codec stack on another session composes the two sessions +rather than copying codecs out of one: the imported codecs resolve their task context +against the original and stop working when it is dropped — see +[One session, one `Arc<SessionContext>`](#one-session-one-arcsessioncontext). Pass +the context itself rather than the capsule it exports, so its codecs get an id that +other sessions can decode. + +Because decoding keys off the id rather than install position, registration order +between independent libraries does not affect decoding at all. It is visible only +on encoding, where codecs are consulted in install order and the first to claim an +object wins — so installing a library can claim objects nothing else claimed, but +never takes over an object an earlier codec was already encoding. Two libraries +that each own tables, functions, and a planner register like this: + +```python +ctx = SessionContext(config) + +# Codecs from both libraries. Order between libraries does not matter. Review Comment: I don't think it's possible because any object decoded by a foreign codec will show up as a `ForeignXYZ` when attempting a downcast so the type ID should never match. -- 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]
