Skip to content

Commit ba4cb30

Browse files
authored
Merge branch 'main' into fix/1577-ffi-typing-protocols
2 parents 7e7fd4f + 41eadc4 commit ba4cb30

61 files changed

Lines changed: 5755 additions & 1052 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

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

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,33 @@ library reaches things only the session has.
6262
session satisfies the protocol too — `ctx.__datafusion_query_planner__()` and
6363
`ctx.__datafusion_query_planner__(ctx)` are both valid.
6464

65+
`__datafusion_session_planner__(ctx, fallback)` is the exception to the shape
66+
above: it takes a second argument, the planner assembled so far. A session has
67+
one planner slot, so planners compose by nesting rather than by chaining, and
68+
the host hands each bundle the previous layer instead of letting it capture one.
69+
Wrap `fallback` and delegate to it; returning a planner that ignores it discards
70+
every layer beneath, including one the session already had. It runs after every
71+
bundle's codecs are installed, so `ctx` carries the final chains.
72+
73+
That is also the only hook where it does. `__datafusion_session_components__`
74+
runs before anything is installed, so its `ctx` still carries the chains the
75+
receiver had — the same session, and the same task-context provider, but not
76+
this call's codecs, not even your own. Read the host's codec chains in the
77+
planner hook, never in the extension hook.
78+
79+
A *codec* must always be handed over as an object implementing its getter, never
80+
as the bare capsule the getter returns; `with_extensions` refuses a capsule.
81+
A codec's wire id — the string a payload names on decode, which has to mean the
82+
same thing in whichever process decodes — is read off the object it arrives as,
83+
and a capsule has no type to read one from. Deriving the id from the bundle that
84+
contributed the capsule is not the fix: the bundle is whatever object the caller
85+
passed, so an application packaging your library inside a bundle of its own
86+
would re-tag your payloads and they would stop decoding where they are read. If
87+
the object's class name is not the identity you want on the wire, declare
88+
`__datafusion_codec_id__` on it. `BundledLogicalCodec` in
89+
`examples/datafusion-ffi-query-planner-example/src/extension.rs` is the shape.
90+
This applies only to codecs — a query planner carries no wire id.
91+
6592
## Rule 3 — never construct a `SessionContext` in an extension library
6693

6794
The FFI constructors ask for things a library does not have:
@@ -154,8 +181,19 @@ guards this. Its `WHERE` clause is load-bearing: filter pushdown upgrades the
154181
weak handle during logical optimization, before plan serialization could fail
155182
first for an unrelated reason.
156183

184+
`SessionContext.with_extensions` is where this rule is easiest to get wrong,
185+
because "bind the components to the context you are about to return" reads like
186+
an instruction to derive one first. It is not: the factories are handed the
187+
receiver, and the returned handle shares its allocation. There is nothing to
188+
keep alive separately and nothing to garbage-collect out from under a provider.
189+
157190
`SessionContext.enable_url_table` is the one method that mints a second
158-
allocation for a session. Its result must not outlive the receiver.
191+
allocation for a session. Its result must not outlive the receiver, and it also
192+
forks the session's `SessionState` while keeping its id, so two handles report
193+
one `session_id()` with divergent configuration. That is a bug rather than a
194+
design — tracked in
195+
[apache/datafusion-python#1708](https://github.com/apache/datafusion-python/issues/1708)
196+
— so do not cite it as precedent for deriving a replacement context.
159197

160198
## Rule 7 — installing a planner mutates the session, and says so
161199

@@ -185,7 +223,12 @@ pins that; changing it should be deliberate.
185223

186224
## Where the truth is
187225

188-
- `docs/source/contributor-guide/ffi.md` — the protocol, the fork caveat.
226+
- `docs/source/extension-guide/` — the protocol, for the library author.
227+
`capsule-protocol.md` has the hook convention and what the getter argument
228+
actually is; `codecs.md`, `bundles.md`, and `query-planners.md` have the
229+
per-component rules; `index.md` lists all 18 hooks.
230+
- `docs/source/contributor-guide/ffi-internals.md` — why the framing is shaped
231+
this way, including the weak-`Arc` scheme and the one-level rebind.
189232
- `docs/source/user-guide/upgrade-guides.md` — every past migration.
190233
- `crates/core/src/codec.rs` — the codec chain: the envelope, identity dispatch,
191234
and the two unframed cases from Rule 8.

‎.github/workflows/build.yml‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ jobs:
4545
lint-rust:
4646
runs-on: ubuntu-latest
4747
steps:
48-
- uses: actions/checkout@v6
48+
- uses: actions/checkout@v7
4949

5050
- name: Setup Rust
5151
uses: dtolnay/rust-toolchain@4cda84d5c5c54efe2404f9d843567869ab1699d4
@@ -62,14 +62,14 @@ jobs:
6262
lint-python:
6363
runs-on: ubuntu-latest
6464
steps:
65-
- uses: actions/checkout@v6
65+
- uses: actions/checkout@v7
6666

6767
- name: Install Python
68-
uses: actions/setup-python@v5
68+
uses: actions/setup-python@v7
6969
with:
7070
python-version: "3.12"
7171

72-
- uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39
72+
- uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d
7373
with:
7474
enable-cache: true
7575

@@ -88,10 +88,10 @@ jobs:
8888
lint-toml:
8989
runs-on: ubuntu-latest
9090
steps:
91-
- uses: actions/checkout@v6
91+
- uses: actions/checkout@v7
9292

9393
- name: Install taplo
94-
uses: taiki-e/install-action@v2
94+
uses: taiki-e/install-action@v2.85.5
9595
with:
9696
tool: taplo-cli
9797

@@ -103,22 +103,22 @@ jobs:
103103
if: inputs.build_mode == 'release' && startsWith(github.ref, 'refs/tags/')
104104
runs-on: ubuntu-latest
105105
steps:
106-
- uses: actions/checkout@v6
106+
- uses: actions/checkout@v7
107107

108108
- name: Ensure [patch.crates-io] is empty
109109
run: python3 dev/check_crates_patch.py
110110

111111
generate-license:
112112
runs-on: ubuntu-latest
113113
steps:
114-
- uses: actions/checkout@v6
114+
- uses: actions/checkout@v7
115115

116-
- uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39
116+
- uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d
117117
with:
118118
enable-cache: true
119119

120120
- name: Install cargo-license
121-
uses: taiki-e/install-action@v2
121+
uses: taiki-e/install-action@v2.85.5
122122
with:
123123
tool: cargo-license
124124

@@ -142,7 +142,7 @@ jobs:
142142
matrix:
143143
python-tag: ["abi3", "3.14t"]
144144
steps:
145-
- uses: actions/checkout@v6
145+
- uses: actions/checkout@v7
146146

147147
- run: rm LICENSE.txt
148148
- name: Download LICENSE.txt
@@ -159,7 +159,7 @@ jobs:
159159
with:
160160
key: ${{ inputs.build_mode }}-${{ matrix.python-tag }}
161161

162-
- uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39
162+
- uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d
163163
with:
164164
enable-cache: true
165165

@@ -233,7 +233,7 @@ jobs:
233233
matrix:
234234
python-tag: ["abi3", "3.14t"]
235235
steps:
236-
- uses: actions/checkout@v6
236+
- uses: actions/checkout@v7
237237

238238
- run: rm LICENSE.txt
239239
- name: Download LICENSE.txt
@@ -250,7 +250,7 @@ jobs:
250250
with:
251251
key: ${{ inputs.build_mode }}-${{ matrix.python-tag }}
252252

253-
- uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39
253+
- uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d
254254
with:
255255
enable-cache: true
256256

@@ -296,7 +296,7 @@ jobs:
296296
os: [macos-latest, windows-latest]
297297
python-tag: ["abi3", "3.14t"]
298298
steps:
299-
- uses: actions/checkout@v6
299+
- uses: actions/checkout@v7
300300

301301
- name: Setup Rust
302302
uses: dtolnay/rust-toolchain@4cda84d5c5c54efe2404f9d843567869ab1699d4
@@ -315,12 +315,12 @@ jobs:
315315

316316
- name: Setup Python (free-threaded)
317317
if: matrix.python-tag != 'abi3'
318-
uses: actions/setup-python@v6
318+
uses: actions/setup-python@v7
319319
with:
320320
python-version: ${{ matrix.python-tag }}
321321
freethreaded: true
322322

323-
- uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39
323+
- uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d
324324
with:
325325
enable-cache: true
326326

@@ -379,7 +379,7 @@ jobs:
379379
matrix:
380380
python-tag: ["abi3", "3.14t"]
381381
steps:
382-
- uses: actions/checkout@v6
382+
- uses: actions/checkout@v7
383383

384384
- name: Setup Rust
385385
uses: dtolnay/rust-toolchain@4cda84d5c5c54efe2404f9d843567869ab1699d4
@@ -398,12 +398,12 @@ jobs:
398398

399399
- name: Setup Python (free-threaded)
400400
if: matrix.python-tag != 'abi3'
401-
uses: actions/setup-python@v6
401+
uses: actions/setup-python@v7
402402
with:
403403
python-version: ${{ matrix.python-tag }}
404404
freethreaded: true
405405

406-
- uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39
406+
- uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d
407407
with:
408408
enable-cache: true
409409

@@ -444,7 +444,7 @@ jobs:
444444
if: inputs.build_mode == 'release'
445445
runs-on: ubuntu-latest
446446
steps:
447-
- uses: actions/checkout@v6
447+
- uses: actions/checkout@v7
448448
- run: rm LICENSE.txt
449449
- name: Download LICENSE.txt
450450
uses: actions/download-artifact@v8
@@ -516,23 +516,23 @@ jobs:
516516
fi
517517
518518
- name: Checkout docs sources
519-
uses: actions/checkout@v6
519+
uses: actions/checkout@v7
520520

521521
- name: Checkout docs target branch
522522
if: github.event_name == 'push' && (github.ref == 'refs/heads/main' || github.ref_type == 'tag')
523-
uses: actions/checkout@v6
523+
uses: actions/checkout@v7
524524
with:
525525
fetch-depth: 0
526526
ref: ${{ steps.target-branch.outputs.value }}
527527
path: docs-target
528528

529529
- name: Setup Python
530-
uses: actions/setup-python@v6
530+
uses: actions/setup-python@v7
531531
with:
532532
python-version: "3.10"
533533

534534
- name: Install dependencies
535-
uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39
535+
uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d
536536
with:
537537
enable-cache: true
538538

‎.github/workflows/codeql.yml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,16 @@ jobs:
3939

4040
steps:
4141
- name: Checkout repository
42-
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
42+
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
4343
with:
4444
persist-credentials: false
4545

4646
- name: Initialize CodeQL
47-
uses: github/codeql-action/init@8aad20d150bbac5944a9f9d289da16a4b0d87c1e # v4
47+
uses: github/codeql-action/init@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4
4848
with:
4949
languages: actions
5050

5151
- name: Perform CodeQL Analysis
52-
uses: github/codeql-action/analyze@8aad20d150bbac5944a9f9d289da16a4b0d87c1e # v4
52+
uses: github/codeql-action/analyze@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4
5353
with:
5454
category: "/language:actions"

‎.github/workflows/dev.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ jobs:
3030
runs-on: ubuntu-latest
3131
steps:
3232
- name: Checkout
33-
uses: actions/checkout@v6
33+
uses: actions/checkout@v7
3434
- name: Setup Python
35-
uses: actions/setup-python@v6
35+
uses: actions/setup-python@v7
3636
with:
3737
python-version: "3.14"
3838
- name: Audit licenses

‎.github/workflows/test.yml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,23 +46,23 @@ jobs:
4646
# Free-threaded builds — one wheel per interpreter.
4747
- { python-version: "3.14t", wheel-tag: "3.14t", freethreaded: true }
4848
steps:
49-
- uses: actions/checkout@v6
49+
- uses: actions/checkout@v7
5050

5151
- name: Setup Python
5252
id: setup-python
53-
uses: actions/setup-python@v6
53+
uses: actions/setup-python@v7
5454
with:
5555
python-version: ${{ matrix.python-version }}
5656
freethreaded: ${{ matrix.freethreaded }}
5757

5858
- name: Cache Cargo
59-
uses: actions/cache@v5
59+
uses: actions/cache@v6
6060
with:
6161
path: ~/.cargo
6262
key: cargo-cache-stable-${{ hashFiles('Cargo.lock') }}
6363

6464
- name: Install dependencies
65-
uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39
65+
uses: astral-sh/setup-uv@20cfd1bf945f4377ade1205e4dbc17946fc9a30d
6666
with:
6767
enable-cache: true
6868

‎.github/workflows/verify-release-candidate.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ jobs:
6565
runs-on: ${{ matrix.runner }}
6666
steps:
6767
- name: Checkout repository
68-
uses: actions/checkout@v6
68+
uses: actions/checkout@v7
6969

7070
- name: Set up protoc
7171
uses: arduino/setup-protoc@v3

‎AGENTS.md‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,47 @@ Every Python function must include a docstring with usage examples.
133133
`array_sort`) only need a one-line description and a `See Also` reference to the
134134
primary function. They do not need their own examples.
135135

136+
### One canonical home per claim
137+
138+
A claim lives where the reader already is when they need it — **exactly once**.
139+
140+
- A property of one callable's arguments, return value, or errors → that
141+
callable's docstring.
142+
- A property of a *type* that several callables share → the class docstring.
143+
(Session sharing and context lifetime live on `SessionContext`, not on each
144+
of the five `with_*` methods.)
145+
- **Why** the API is shaped this way, a multi-library recipe, Rust-side code, a
146+
trade-off, or a limitation with an upstream issue → the narrative guide under
147+
`docs/source/`.
148+
149+
Everywhere else: one sentence plus one Sphinx role. A docstring may *state* a
150+
claim the guide also makes; it must not *argue* it — no "because", no "the
151+
reason is", no counter-argument.
152+
153+
**The test:** if a paragraph would survive being moved into the guide unchanged,
154+
move it. `python/tests/test_docstrings.py` enforces a length ceiling, which is
155+
the symptom this rule treats.
156+
157+
When you point at the guide, use a real `:ref:` to a specific label. Prose
158+
saying "see the extensions guide" with no role is a dead end in the rendered
159+
HTML.
160+
161+
### Examples that need a compiled extension
162+
163+
Some APIs cannot be demonstrated without a built FFI extension library, which
164+
this package does not ship. The convention is:
165+
166+
1. A **runnable** example block first, using only the wheel. A `SessionContext`
167+
satisfies the capsule-getter protocols, so its own exported capsule stands in
168+
for a real library's in a doctest.
169+
2. Then a `# doctest: +SKIP` block showing real usage, at most a few lines, with
170+
one line of prose naming the test that runs it for real.
171+
3. Every `+SKIP` block needs that mirror. See
172+
`test_with_extensions_docstring_example_still_runs` in
173+
`examples/datafusion-ffi-query-planner-example`, which parses the live
174+
docstring, drops the skip, and executes it — so a renamed method or a wrong
175+
expected output fails there.
176+
136177
## Aggregate and Window Function Documentation
137178

138179
When adding or updating an aggregate or window function, ensure the corresponding

0 commit comments

Comments
 (0)