Skip to content

Commit 89b5feb

Browse files
committed
docs: rewrite 'Encode metadata, not a handle to a live object' section
Closes #1726
1 parent ceb2d8d commit 89b5feb

2 files changed

Lines changed: 24 additions & 12 deletions

File tree

‎docs/source/extension-guide/checklist.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,9 @@ publish. Each links to the page that explains it.
9191
derived from it is in use. This is the rule most likely to arrive as a
9292
bug report against your library. → {ref}`extension_sessions`
9393
- [ ] **Your production codec serializes durable metadata**, not a
94-
process-local token. The examples in this repository use tokens to make
95-
ownership observable; that is a demonstration, not a pattern.
94+
process-local token. While one arm of one codec in this repository uses a
95+
token to make ownership observable, that is a marked workaround for an
96+
upstream defect, not a pattern.
9697
→ {ref}`extension_codec_durable_metadata`
9798
- [ ] **You have integration tests across a real FFI boundary.** The two
9899
example crates in this repository are the pattern: build the cdylib,

‎docs/source/extension-guide/codecs.md‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,24 @@ Your payload has to be enough to rebuild the object somewhere your process is
5858
not. Write the metadata a fresh instance can be constructed from — a path, a
5959
connection string, a schema, the options the object was created with.
6060

61-
The example codecs in this repository do not do this, and it is worth knowing
62-
before copying them. They keep a process-local `HashMap` of live providers and
63-
encode an integer token into it: encoding inserts, decoding removes. That makes
64-
Rust type identity observable across three separately loaded libraries in one
65-
test, which is what the examples exist to show. It also means a decode consumes
66-
its token, so the same bytes cannot be decoded twice, one encoded plan cannot
67-
fan out to several readers, and a plan that never reaches a decoder keeps its
68-
provider alive for the life of the process. A real codec has none of those
69-
properties because it does not park the object anywhere.
61+
For working examples, see the logical codec in [`datafusion-ffi-example`] and
62+
the codecs in `examples/distributed/storage-library`.
63+
64+
You may see a codec that uses a process-local `HashMap` of live objects and
65+
encodes an integer token into it: encoding inserts, decoding removes. Do not copy
66+
this pattern. It means a decode consumes its token, so the same bytes cannot be
67+
decoded twice, one encoded plan cannot fan out to several readers, and a plan
68+
that never reaches a decoder keeps its object alive for the life of the process.
69+
A real codec has none of those properties because it does not park the object
70+
anywhere.
71+
72+
A token registry is a consequence, not a choice. A codec that downcasts to its
73+
own concrete types is never handed something it cannot describe. The only place
74+
this repository uses one is the `ForeignExecutionPlan` arm of the physical codec
75+
in [`datafusion-ffi-example`]. It is a marked workaround for an upstream defect
76+
([apache/datafusion#25152](https://github.com/apache/datafusion/issues/25152))
77+
and will be deleted once that is fixed. For why a codec would ever claim a foreign
78+
node it does not own in the first place, see {ref}`extension_codec_order`.
7079

7180
(extension_codec_ids)=
7281

@@ -151,7 +160,9 @@ after it. The query still succeeds. What changes is which library wrote the
151160
bytes — so a plan that has to decode in another process now needs whichever
152161
library happened to win, not the one whose node it is.
153162
`MyPhysicalExtensionCodec` in [`datafusion-ffi-example`] claims this way, and
154-
the query-planner example's test suite pins the consequence.
163+
the query-planner example's test suite pins the consequence. See
164+
{ref}`extension_codec_durable_metadata` for why that arm exists and when it will
165+
be removed.
155166

156167
Two rules of thumb:
157168

0 commit comments

Comments
 (0)