Skip to content

Commit 4c5c3b1

Browse files
authored
feat: define VGI 2.0 write result modes (#75)
1 parent 1bb086f commit 4c5c3b1

17 files changed

Lines changed: 451 additions & 149 deletions

File tree

‎docs/protocol/vgi-protocol-proposed-changes.md‎

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,34 @@ Triggers are different. Supporting a trigger in a virtual catalog would require
453453

454454
The same principle applies to secure views, composable COPY, remote-plan pushdown, coordinate systems, and any catalog object VGI does not currently virtualize: they are not protocol requirements simply because DuckDB implements them. They should be gated through explicit catalog capabilities and fail early.
455455

456-
### 6.1 New catalog kinds in DuckDB 2.0
456+
### 6.1 Writable result modes and OLD/NEW images
457+
458+
VGI 2.0 replaces the four `TableInfo` booleans `supports_insert`,
459+
`supports_update`, `supports_delete`, and `supports_returning` with one required,
460+
non-null Arrow map:
461+
462+
```text
463+
write_result_modes: map<utf8, utf8>
464+
```
465+
466+
The only operation keys are `insert`, `update`, and `delete`; absence means the
467+
operation is unsupported. Each value is the maximum supported mode in the
468+
ordered lattice `count < rows < changes`, and therefore promises every lower
469+
mode. Duplicate keys and unknown operations or modes are protocol errors.
470+
471+
The serialized `write_options` batch replaces `return_chunks: bool` with the
472+
required `result_mode: utf8`. `count` returns `(count int64 not null)`. `rows`
473+
returns NEW rows for INSERT/UPDATE and OLD rows for DELETE. `changes` returns
474+
two nullable structs, `old` and `new`, each shaped exactly like the table:
475+
INSERT populates only `new`, DELETE only `old`, and UPDATE both. The operation
476+
is known from the invoked write function and is not repeated in each result row.
477+
478+
The DuckDB 1.5 adapter requests `count` for ordinary DML and `rows` for DML with
479+
RETURNING. It validates and preserves `changes` capability metadata but never
480+
requests that mode. A DuckDB 2.0 adapter can request `changes` when trigger
481+
planning needs OLD/NEW images without another protocol revision.
482+
483+
### 6.2 New catalog kinds in DuckDB 2.0
457484

458485
The `CatalogType` enum has exactly two additions relative to DuckDB 1.5.5:
459486

@@ -464,7 +491,7 @@ The `CatalogType` enum has exactly two additions relative to DuckDB 1.5.5:
464491

465492
Nested schemas are recursively contained `SCHEMA_ENTRY` objects, not another kind. Secure views remain `VIEW_ENTRY`; changes to aggregate-state representation belong to the logical type system rather than the catalog enum.
466493

467-
### 6.2 How native DuckDB stores triggers
494+
### 6.3 How native DuckDB stores triggers
468495

469496
Triggers are first-class `TriggerCatalogEntry` objects, but they do not live in the schema's ordinary catalog set. Each `DuckTableEntry` owns a separate transactional `CatalogSet`:
470497

@@ -496,7 +523,7 @@ The dependency manager has special trigger identity/lookup handling because sche
496523

497524
At bind time DuckDB scans the table's triggers and expands them into the DML plan. Statement triggers form a materialized CTE chain (BEFORE actions, base DML, AFTER actions), including materialized OLD/NEW transition tables when requested. Supported row triggers use `LogicalTrigger` and correlated `NEW.col`/`OLD.col` bindings. The current catalog set yields triggers in case-insensitive alphabetical name order.
498525

499-
### 6.3 DuckDB changes needed by custom table implementations
526+
### 6.4 DuckDB changes needed by custom table implementations
500527

501528
DuckDB 2.0 already exposes virtual `TableCatalogEntry::CreateTrigger`, `ScanTriggers`, and `GetTrigger` hooks, and its trigger binder consumes the generic scan hook. Two remaining native-table assumptions should be fixed upstream:
502529

@@ -509,7 +536,7 @@ Upstream PR: [duckdb/duckdb#25539 — Support triggers on custom table catalog e
509536

510537
The `ScanTriggers` contract also implies that returned catalog entries remain alive beyond the callback: both trigger selection and introspection retain references. VGI should materialize trigger entries in a table-owned metadata cache, invalidated through the existing catalog-version mechanism, rather than returning callback-local objects.
511538

512-
### 6.4 Proposed VGI trigger protocol
539+
### 6.5 Proposed VGI trigger protocol
513540

514541
The remote VGI catalog should be authoritative for persistent trigger definitions. Keeping definitions only in a local extension cache would lose them on detach and would give different clients inconsistent catalogs.
515542

@@ -529,13 +556,22 @@ TriggerInfo
529556
for_each: statement | row
530557
referencing_new_table: optional<string>
531558
referencing_old_table: optional<string>
532-
action_sql: string
559+
definitions: map<string, string>
533560
dependencies: list<TriggerDependency>
534561
comment: optional<string>
535562
tags: map<string, string>
536563
```
537564

538-
Use structured fields plus DuckDB-dialect action SQL. Do not put DuckDB's binary `QueryNode` serialization on the VGI wire: it is an internal, version-coupled representation. The local extension can parse the action and create cached `TriggerCatalogEntry` objects. Each resolved dependency should carry its qualified object reference plus ownership, drop-blocking, and alter-blocking flags so the remote catalog can enforce DDL integrity across clients.
565+
Use structured fields plus engine-specific textual definitions keyed by stable
566+
lowercase engine identifiers such as `duckdb`, `datafusion`, `spark`, and
567+
`sqlite`. An engine executes only its matching definition; a missing entry means
568+
that trigger is unavailable on that engine, with no implicit SQL-dialect
569+
fallback. Do not put DuckDB's binary `QueryNode` serialization on the VGI wire:
570+
it is an internal, version-coupled representation. The DuckDB extension parses
571+
the `duckdb` definition and creates cached `TriggerCatalogEntry` objects. Each
572+
resolved dependency should carry its qualified object reference plus ownership,
573+
drop-blocking, and alter-blocking flags so the remote catalog can enforce DDL
574+
integrity across clients.
539575

540576
Suggested methods:
541577

@@ -545,7 +581,10 @@ Suggested methods:
545581
- `catalog_table_trigger_drop`
546582
- optional trigger comment/tag alteration
547583

548-
Add explicit catalog/table capability metadata such as `supports_triggers`. A trigger-capable table must also support exact affected-row RETURNING, because DuckDB's statement-trigger expansion internally materializes base DML with `RETURNING *` even when the user did not write a RETURNING clause. UPDATE must additionally expose captured OLD columns before OLD transition tables can be enabled.
584+
Add explicit catalog/table capability metadata such as `supports_triggers`. A
585+
DuckDB-managed trigger-capable table must advertise `changes` for every event it
586+
uses, because DuckDB's statement-trigger expansion needs exact OLD/NEW images
587+
even when the user did not write a RETURNING clause.
549588

550589
All base and trigger-body writes must use the same VGI transaction token and roll back atomically. Stable statement/sub-operation identifiers are recommended so transport retries cannot apply a trigger side effect twice. Trigger support should be rejected when the VGI catalog cannot provide transactional atomicity.
551590

@@ -673,4 +712,11 @@ The first protocol-breaking phase is now implemented in Python:
673712

674713
`CatalogAttachResult.default_schema` intentionally remains a scalar root-schema identifier for the reason described in section 2.
675714

676-
This phase does **not** implement the other proposed protocol areas: `vgi.filters.v2`, logical-type identity annotations, trigger RPCs, or standalone window-function catalog entries. The sibling C++, C#, Go, Java, Rust, and TypeScript schema-path migrations were completed afterward. The vendored browser `vgi-client.js` remains generated from the TypeScript SDK and must never be edited manually in the Python repository.
715+
The writable-result phase replaces the legacy write booleans and
716+
`return_chunks` option with the `write_result_modes`/`result_mode` contract from
717+
section 6.1. Python implements and directly tests all three modes. The DuckDB
718+
1.5 extension consumes only `count` and `rows`; `changes` is reserved for the
719+
future DuckDB 2.0 trigger adapter. Trigger catalog metadata and RPCs remain
720+
unimplemented.
721+
722+
This phase does **not** implement the other proposed protocol areas: logical-type identity annotations, trigger RPCs, or standalone window-function catalog entries. The sibling C++, C#, Go, Java, Rust, and TypeScript schema migrations are maintained in their corresponding repositories. The vendored browser `vgi-client.js` remains generated from the TypeScript SDK and must never be edited manually in the Python repository.

‎tests/catalog/test_writable_table.py‎

Lines changed: 47 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -129,11 +129,11 @@ def test_delete_requires_scan_function(self) -> None:
129129
)
130130

131131

132-
class TestTableInfoWriteFlags:
133-
"""TableInfo serialization includes write support flags."""
132+
class TestTableInfoWriteResultModes:
133+
"""TableInfo serializes per-operation maximum result modes."""
134134

135-
def test_write_flags_set_when_functions_present(self) -> None:
136-
"""Write flags are True when corresponding functions are set."""
135+
def test_count_modes_inferred_when_functions_present(self) -> None:
136+
"""Configured functions default to the count result mode."""
137137
table = Table(
138138
name="t",
139139
function=WritableTableScan,
@@ -142,35 +142,63 @@ def test_write_flags_set_when_functions_present(self) -> None:
142142
delete_function=WritableTableDelete,
143143
)
144144
info = table.to_table_info(["main"])
145-
assert info.supports_insert is True
146-
assert info.supports_update is True
147-
assert info.supports_delete is True
145+
assert info.write_result_modes == {"insert": "count", "update": "count", "delete": "count"}
148146

149-
def test_write_flags_false_by_default(self) -> None:
150-
"""Write flags are False when no write functions are set."""
147+
def test_read_only_table_has_empty_modes(self) -> None:
148+
"""An empty map describes a read-only table."""
151149
table = Table(
152150
name="t",
153151
function=WritableTableScan,
154152
)
155153
info = table.to_table_info(["main"])
156-
assert info.supports_insert is False
157-
assert info.supports_update is False
158-
assert info.supports_delete is False
154+
assert info.write_result_modes == {}
159155

160-
def test_partial_write_flags(self) -> None:
161-
"""Only the flags for defined write functions are True."""
156+
def test_partial_write_modes(self) -> None:
157+
"""Only operations with functions appear in the map."""
162158
table = Table(
163159
name="t",
164160
function=WritableTableScan,
165161
insert_function=WritableTableInsert,
166162
)
167163
info = table.to_table_info(["main"])
168-
assert info.supports_insert is True
169-
assert info.supports_update is False
170-
assert info.supports_delete is False
164+
assert info.write_result_modes == {"insert": "count"}
165+
166+
def test_explicit_maximum_modes(self) -> None:
167+
"""Authors can opt each configured operation into rows or changes."""
168+
table = Table(
169+
name="t",
170+
function=WritableTableScan,
171+
insert_function=WritableTableInsert,
172+
update_function=WritableTableUpdate,
173+
write_result_modes={"insert": "rows", "update": "changes"},
174+
)
175+
assert table.to_table_info(["main"]).write_result_modes == {
176+
"insert": "rows",
177+
"update": "changes",
178+
}
179+
180+
@pytest.mark.parametrize("modes", [{"merge": "count"}, {"insert": "invalid"}])
181+
def test_invalid_modes_rejected(self, modes: dict[str, str]) -> None:
182+
"""Unknown operations and modes fail at descriptor construction."""
183+
with pytest.raises(ValueError):
184+
Table(
185+
name="t",
186+
columns=pa.schema([("id", pa.int64())]),
187+
insert_function=WritableTableInsert,
188+
write_result_modes=modes,
189+
)
190+
191+
def test_mode_without_function_rejected(self) -> None:
192+
"""Capability metadata cannot claim a missing implementation."""
193+
with pytest.raises(ValueError, match="without a corresponding update_function"):
194+
Table(
195+
name="t",
196+
columns=pa.schema([("id", pa.int64())]),
197+
write_result_modes={"update": "rows"},
198+
)
171199

172200
def test_table_info_serialization_roundtrip(self) -> None:
173-
"""Write flags survive Arrow serialization roundtrip."""
201+
"""Write modes survive Arrow serialization roundtrip."""
174202
table = Table(
175203
name="t",
176204
function=WritableTableScan,
@@ -181,9 +209,7 @@ def test_table_info_serialization_roundtrip(self) -> None:
181209
info = table.to_table_info(["main"])
182210
data = info.serialize_to_bytes()
183211
restored = TableInfo.deserialize_from_bytes(data)
184-
assert restored.supports_insert is True
185-
assert restored.supports_update is True
186-
assert restored.supports_delete is True
212+
assert restored.write_result_modes == {"insert": "count", "update": "count", "delete": "count"}
187213

188214

189215
class TestReadOnlyCatalogInterfaceWriteMethods:

‎tests/test_write_results.py‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# Copyright 2025, 2026 Query Farm LLC - https://query.farm
2+
3+
"""Tests for the VGI 2.0 writable result-mode contract."""
4+
5+
import pyarrow as pa
6+
import pytest
7+
8+
from vgi.schema_utils import schema
9+
from vgi.write_results import supports_write_result_mode, write_changes_batch, write_result_schema
10+
11+
12+
def test_ordered_result_modes() -> None:
13+
"""A maximum mode promises itself and every lower mode."""
14+
assert supports_write_result_mode("count", "count")
15+
assert not supports_write_result_mode("count", "rows")
16+
assert supports_write_result_mode("rows", "count")
17+
assert supports_write_result_mode("changes", "rows")
18+
assert supports_write_result_mode("changes", "changes")
19+
20+
21+
def test_result_schemas() -> None:
22+
"""Every mode has the exact canonical Arrow schema."""
23+
table = schema(id=pa.int64(), name=pa.string())
24+
assert write_result_schema("count", table) == pa.schema([pa.field("count", pa.int64(), nullable=False)])
25+
assert write_result_schema("rows", table) == table
26+
changes = write_result_schema("changes", table)
27+
assert changes.names == ["old", "new"]
28+
assert changes.field("old").nullable
29+
assert changes.field("old").type == pa.struct(list(table))
30+
31+
32+
@pytest.mark.parametrize(
33+
("old", "new"),
34+
[
35+
(None, {"id": 1, "name": "inserted"}),
36+
({"id": 1, "name": "old"}, {"id": 1, "name": "new"}),
37+
({"id": 1, "name": "deleted"}, None),
38+
],
39+
)
40+
def test_change_images(old: dict[str, object] | None, new: dict[str, object] | None) -> None:
41+
"""INSERT, UPDATE, and DELETE preserve the required OLD/NEW nullability."""
42+
table = schema(id=pa.int64(), name=pa.string())
43+
batch = write_changes_batch(table, [old], [new])
44+
assert batch.to_pylist() == [{"old": old, "new": new}]
45+
46+
47+
def test_change_image_lengths_must_match() -> None:
48+
"""Every output row must contain one OLD/NEW pair."""
49+
with pytest.raises(ValueError, match="equal length"):
50+
write_changes_batch(pa.schema([("id", pa.int64())]), [], [{"id": 1}])

‎tests/transactor/test_transactor.py‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
# Helpers
2525
# ============================================================================
2626

27-
_COUNT_SCHEMA = schema(count=pa.int64())
27+
_COUNT_SCHEMA = pa.schema([pa.field("count", pa.int64(), nullable=False)])
2828
_FILTER_METADATA = {b"vgi_filter_version": b"1"}
2929

3030

@@ -66,7 +66,7 @@ def _insert_into(
6666
t: TransactorImpl, tx_id: bytes, table_name: str, rows: list[tuple[int, str]], *, returning: bool = False
6767
) -> pa.RecordBatch:
6868
"""Insert rows into an arbitrary table and return the response batch."""
69-
stream = t.insert(tx_id=tx_id, table_name=table_name, returning=returning)
69+
stream = t.insert(tx_id=tx_id, table_name=table_name, result_mode="rows" if returning else "count")
7070
batch = pa.record_batch(
7171
{"id": [r[0] for r in rows], "name": [r[1] for r in rows]},
7272
schema=schema(id=pa.int64(), name=pa.string()),
@@ -79,7 +79,7 @@ def _insert_into(
7979

8080
def _insert(t: TransactorImpl, tx_id: bytes, rows: list[tuple[int, str]], *, returning: bool = False) -> pa.RecordBatch:
8181
"""Insert rows via the transactor and return the response batch."""
82-
stream = t.insert(tx_id=tx_id, table_name="data", returning=returning)
82+
stream = t.insert(tx_id=tx_id, table_name="data", result_mode="rows" if returning else "count")
8383
batch = pa.record_batch(
8484
{"id": [r[0] for r in rows], "name": [r[1] for r in rows]},
8585
schema=schema(id=pa.int64(), name=pa.string()),
@@ -119,7 +119,7 @@ def _scan(
119119

120120
def _delete(t: TransactorImpl, tx_id: bytes, rowids: list[int], *, returning: bool = False) -> pa.RecordBatch:
121121
"""Delete rows by rowid, return the response batch."""
122-
stream = t.delete(tx_id=tx_id, table_name="data", returning=returning)
122+
stream = t.delete(tx_id=tx_id, table_name="data", result_mode="rows" if returning else "count")
123123
batch = pa.record_batch({"rowid": rowids}, schema=schema(rowid=pa.int64()))
124124
out = OutputCollector(stream.output_schema, producer_mode=False)
125125
assert isinstance(stream.state, ExchangeState)
@@ -136,7 +136,12 @@ def _update(
136136
t: TransactorImpl, tx_id: bytes, rowids: list[int], names: list[str], *, returning: bool = False
137137
) -> pa.RecordBatch:
138138
"""Update name column by rowid, return the response batch."""
139-
stream = t.update(tx_id=tx_id, table_name="data", columns=["name"], returning=returning)
139+
stream = t.update(
140+
tx_id=tx_id,
141+
table_name="data",
142+
columns=["name"],
143+
result_mode="rows" if returning else "count",
144+
)
140145
batch = pa.record_batch(
141146
{"name": names, "rowid": rowids},
142147
schema=schema(name=pa.string(), rowid=pa.int64()),

‎vgi/__init__.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,15 @@
129129
TableInOutGenerator,
130130
)
131131
from vgi.worker import Worker
132+
from vgi.write_results import WriteOperation, WriteResultMode, write_changes_batch, write_result_schema
132133

133134
__all__ = [
134135
"AggregateBindParams",
135136
"AggregateFunction",
137+
"WriteOperation",
138+
"WriteResultMode",
139+
"write_result_schema",
140+
"write_changes_batch",
136141
"AnyArrow",
137142
"AnyArrowValue",
138143
"Arg",

‎vgi/_test_fixtures/schema_reconcile/worker.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -528,9 +528,7 @@ def _table_info(self, spec: TableSpec) -> TableInfo:
528528
not_null_constraints=[],
529529
unique_constraints=[],
530530
check_constraints=[],
531-
supports_insert=True,
532-
supports_update=True,
533-
supports_delete=True,
531+
write_result_modes={"insert": "changes", "update": "changes", "delete": "changes"},
534532
)
535533

536534
def schemas(

0 commit comments

Comments
 (0)