Skip to content

Commit f7f13a8

Browse files
committed
feat(client): expose result-cache metadata, multi-branch, and time travel
Three more Client gaps closed the same way as the splits fix: the wire protocol already carried what's needed, only the Python client hadn't caught up. All additive, no protocol/wire version change (see each item below). - table_function(batch_metadata_callback=...): AnnotatedBatch.custom_metadata was discarded unconditionally in _table_function_parallel (only the bare RecordBatch was forwarded), so a worker's vgi.cache.* cacheability advertisement was structurally unreachable through the public generator. The queue now carries the full AnnotatedBatch; an optional callback mirroring bind_result_callback fires once per batch with its custom_metadata before it's yielded. - Client.table_scan_branches_get(...): no wrapper for catalog_table_scan_branches_get existed at all, so a multi-branch table was invisible to any non-DuckDB caller. Deserializes the wire's list[bytes] into typed ScanBranch objects (mirrors table_function_plan's ScanSplit deserialization) and falls back to table_scan_function_get wrapped as a one-branch result when the worker raises MethodNotImplementedError for the additive RPC — the same fallback the C++ extension itself uses. - table_get(at_unit=..., at_value=...) / table_function(at_unit=..., at_value=...): both wire RPCs (catalog_table_get, BindRequest.at_unit/ .at_value) already carried these fields; table_scan_function_get was the only Client method that exposed them. Two worker patterns proven working: a pre-resolved bind-argument style (data.versioned_data) and a style that reads at_unit/at_value directly off the init request (data.tt_pushdown_fn/tt_pushdown_scan) — the latter is only reachable through table_function's own new parameters, not by pre-resolving args. TableInfo.supports_time_travel (per-table discovery, vs. the existing catalog-wide CatalogAttachResult.supports_time_travel) is a real wire-schema addition needing a protocol version bump + C++ codegen regen in the sibling vgi repo — intentionally out of scope here, left as a smaller follow-up. tests/client/test_batch_metadata_callback.py (9 tests), tests/client/test_scan_branches_client.py (6 tests, both the real RPC and the legacy fallback), tests/client/test_time_travel_client.py (12 tests, both worker patterns) — all 3 client transports where applicable. Zero regressions in the existing suite (2140 passed both before and after; the 9 pre-existing failures are unrelated missing optional extras and pre-existing generated-header drift, confirmed via git stash against a clean checkout).
1 parent 724b19c commit f7f13a8

5 files changed

Lines changed: 365 additions & 6 deletions

File tree

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
# Copyright 2026 Query Farm LLC - https://query.farm
2+
3+
# ruff: noqa: S101, D101, D102, D103
4+
"""`Client.table_function(batch_metadata_callback=...)`.
5+
6+
`_table_function_parallel` previously discarded `AnnotatedBatch.custom_metadata`
7+
unconditionally (`if output.batch.num_rows > 0: output_queue.put(output.batch)`)
8+
and only forwarded the bare `pa.RecordBatch` — so a worker's `vgi.cache.*`
9+
cacheability advertisement (`vgi/cache_control.py`), which rides
10+
`custom_metadata`, was structurally unreachable through the public
11+
`table_function()` generator. `batch_metadata_callback` mirrors the existing
12+
`bind_result_callback` pattern to fix that additively, with no change to the
13+
generator's yield type. Exercised against `cacheable_numbers`
14+
(`vgi/_test_fixtures/table/cache.py`), which advertises `vgi.cache.ttl` on its
15+
first batch.
16+
"""
17+
18+
from __future__ import annotations
19+
20+
from typing import Any
21+
22+
import pyarrow as pa
23+
import pytest
24+
25+
from vgi.arguments import Arguments
26+
27+
DATA = "data"
28+
29+
30+
@pytest.fixture
31+
def client(client_transport: Any) -> Any:
32+
with client_transport() as c:
33+
c.catalog_attach(name="example", data_version_spec=None, implementation_version=None)
34+
yield c
35+
36+
37+
def test_callback_receives_cache_control_metadata(client: Any) -> None:
38+
metas: list[pa.KeyValueMetadata | None] = []
39+
batches = list(
40+
client.table_function(
41+
function_name="cacheable_numbers",
42+
schema_name=DATA,
43+
arguments=Arguments(named={"n": pa.scalar(5)}),
44+
batch_metadata_callback=metas.append,
45+
)
46+
)
47+
assert sum(b.num_rows for b in batches) == 5
48+
assert len(metas) == len(batches)
49+
assert metas[0] is not None
50+
assert dict(metas[0])[b"vgi.cache.ttl"] == b"300"
51+
52+
53+
def test_callback_is_optional_and_defaults_to_none(client: Any) -> None:
54+
"""No `batch_metadata_callback` -> unchanged behavior, no crash."""
55+
batches = list(
56+
client.table_function(
57+
function_name="cacheable_numbers",
58+
schema_name=DATA,
59+
arguments=Arguments(named={"n": pa.scalar(3)}),
60+
)
61+
)
62+
assert sum(b.num_rows for b in batches) == 3
63+
64+
65+
def test_callback_sees_none_for_a_batch_without_custom_metadata(client: Any) -> None:
66+
"""A function that never advertises cache control still drives the callback once per batch, with `None`."""
67+
metas: list[pa.KeyValueMetadata | None] = []
68+
list(
69+
client.table_function(
70+
function_name="sequence",
71+
schema_name="main",
72+
arguments=Arguments(positional=[pa.scalar(4)]),
73+
batch_metadata_callback=metas.append,
74+
)
75+
)
76+
assert metas
77+
assert all(m is None for m in metas)
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
# Copyright 2026 Query Farm LLC - https://query.farm
2+
3+
# ruff: noqa: S101, D101, D102, D103
4+
"""`Client.table_scan_branches_get`.
5+
6+
`Client` had no wrapper for `catalog_table_scan_branches_get` at all —
7+
grepped the whole `vgi/client/` package for "branch" and the only hit was an
8+
unrelated code comment ("skip the subprocess branch"). A multi-branch table
9+
was therefore invisible to any non-DuckDB caller, not just unsupported by a
10+
higher-level connector. Exercised against `data.multi_branch_numbers` (two
11+
`sequence(50)` arms — `vgi/_test_fixtures/worker.py`) and, for the legacy
12+
fallback, `data.numbers` (an ordinary single-branch table whose worker only
13+
implements the older `table_scan_function_get`).
14+
"""
15+
16+
from __future__ import annotations
17+
18+
from typing import Any
19+
20+
import pytest
21+
22+
from vgi.catalog import ScanBranch, ScanBranchesResult
23+
24+
DATA = "data"
25+
26+
27+
@pytest.fixture
28+
def client(client_transport: Any) -> Any:
29+
with client_transport() as c:
30+
c.catalog_attach(name="example", data_version_spec=None, implementation_version=None)
31+
yield c
32+
33+
34+
def _attach_opaque(client: Any) -> Any:
35+
# catalog_attach() was already called by the fixture above (its return
36+
# value discarded) — re-attach here to get the opaque data this method
37+
# needs, mirroring how CatalogClientMixin methods are used standalone.
38+
return client.catalog_attach(name="example", data_version_spec=None, implementation_version=None).attach_opaque_data
39+
40+
41+
def test_multi_branch_table_returns_all_branches(client: Any) -> None:
42+
result = client.table_scan_branches_get(
43+
attach_opaque_data=_attach_opaque(client),
44+
schema_name=DATA,
45+
name="multi_branch_numbers",
46+
)
47+
assert isinstance(result, ScanBranchesResult)
48+
assert len(result.branches) == 2
49+
for branch in result.branches:
50+
assert isinstance(branch, ScanBranch)
51+
assert branch.function_name == "sequence"
52+
assert branch.source_table is None # function branch, not catalog-table
53+
54+
55+
def test_single_branch_table_falls_back_to_legacy_rpc(client: Any) -> None:
56+
"""`data.numbers` only implements `table_scan_function_get` — the fallback wraps it as one branch."""
57+
result = client.table_scan_branches_get(
58+
attach_opaque_data=_attach_opaque(client),
59+
schema_name=DATA,
60+
name="numbers",
61+
)
62+
assert len(result.branches) == 1
63+
assert result.branches[0].function_name == client.table_scan_function_get(
64+
attach_opaque_data=_attach_opaque(client),
65+
schema_name=DATA,
66+
name="numbers",
67+
).function_name
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
# Copyright 2026 Query Farm LLC - https://query.farm
2+
3+
# ruff: noqa: S101, D101, D102, D103
4+
"""`Client.table_get`/`table_function(at_unit=..., at_value=...)`.
5+
6+
Both methods previously dropped `at_unit`/`at_value` on the floor even though
7+
the underlying wire RPCs (`catalog_table_get`, `BindRequest.at_unit`/
8+
`.at_value`) already carried them — `table_scan_function_get` was the only
9+
`Client` method that exposed time travel at all. Two distinct worker
10+
patterns are exercised, both real fixtures, proving this isn't just plumbing
11+
that happens to compile:
12+
13+
* `data.versioned_data` reads its version through a pre-resolved bind
14+
argument (`table_scan_function_get(at_unit=...)` bakes it into the
15+
returned `ScanFunctionResult.arguments`) — `table_get(at_unit=...)` is
16+
what needed fixing here, to see the schema at that version at all.
17+
* `data.tt_pushdown_fn` (`tt_pushdown_scan`) reads `at_unit`/`at_value`
18+
**directly off the init request** (`ProcessParams.at_unit`/`.at_value`,
19+
see `vgi/_test_fixtures/table/tt_pushdown.py`) — this is the pattern that
20+
needed `table_function(at_unit=...)` specifically; no amount of
21+
pre-resolved arguments would reach it.
22+
"""
23+
24+
from __future__ import annotations
25+
26+
from typing import Any
27+
28+
import pyarrow as pa
29+
import pytest
30+
31+
from vgi.arguments import Arguments
32+
33+
DATA = "data"
34+
35+
36+
@pytest.fixture
37+
def client(client_transport: Any) -> Any:
38+
with client_transport() as c:
39+
c.catalog_attach(name="example", data_version_spec=None, implementation_version=None)
40+
yield c
41+
42+
43+
def _attach_opaque(client: Any) -> Any:
44+
result = client.catalog_attach(name="example", data_version_spec=None, implementation_version=None)
45+
return result.attach_opaque_data
46+
47+
48+
def test_table_get_at_unit_returns_the_versioned_schema(client: Any) -> None:
49+
opaque = _attach_opaque(client)
50+
v1 = client.table_get(
51+
attach_opaque_data=opaque, schema_name=DATA, name="versioned_data", at_unit="VERSION", at_value="1"
52+
)
53+
live = client.table_get(attach_opaque_data=opaque, schema_name=DATA, name="versioned_data")
54+
55+
v1_schema = pa.ipc.read_schema(pa.py_buffer(v1.columns))
56+
live_schema = pa.ipc.read_schema(pa.py_buffer(live.columns))
57+
assert v1_schema.names == ["id"] # version 1: (id int64) only
58+
assert live_schema.names == ["id", "score"] # current: version 3
59+
60+
61+
def test_table_get_with_no_at_clause_is_unaffected(client: Any) -> None:
62+
"""The new `at_unit`/`at_value` params default to `None` — unchanged behavior without them."""
63+
info = client.table_get(attach_opaque_data=_attach_opaque(client), schema_name=DATA, name="numbers")
64+
assert info is not None
65+
66+
67+
def test_table_function_at_unit_reaches_a_worker_that_reads_it_from_init(client: Any) -> None:
68+
"""`tt_pushdown_scan` reads `at_unit`/`at_value` off the init request, not a resolved bind arg."""
69+
v1_batches = list(
70+
client.table_function(function_name="tt_pushdown_scan", schema_name=DATA, at_unit="VERSION", at_value="1")
71+
)
72+
live_batches = list(client.table_function(function_name="tt_pushdown_scan", schema_name=DATA))
73+
74+
assert sum(b.num_rows for b in v1_batches) == 5
75+
assert sum(b.num_rows for b in live_batches) == 10
76+
77+
78+
def test_table_function_with_no_at_clause_is_unaffected(client: Any) -> None:
79+
"""The new `at_unit`/`at_value` params default to `None` — unchanged behavior without them."""
80+
batches = list(
81+
client.table_function(
82+
function_name="sequence", schema_name="main", arguments=Arguments(positional=[pa.scalar(5)])
83+
)
84+
)
85+
assert sum(b.num_rows for b in batches) == 5

‎vgi/client/catalog_mixin.py‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ class CatalogEnabledClient(CatalogClientMixin, Client):
5959
MacroInfo,
6060
MacroType,
6161
OnConflict,
62+
ScanBranch,
63+
ScanBranchesResult,
6264
ScanFunctionResult,
6365
SchemaInfo,
6466
SchemaObjectType,
@@ -603,6 +605,8 @@ def table_get(
603605
transaction_opaque_data: TransactionOpaqueData | None = None,
604606
schema_name: str,
605607
name: str,
608+
at_unit: str | None = None,
609+
at_value: str | None = None,
606610
) -> TableInfo | None:
607611
"""Get information about a table.
608612
@@ -611,6 +615,10 @@ def table_get(
611615
transaction_opaque_data: Optional transaction ID for transactional reads.
612616
schema_name: The schema containing the table.
613617
name: The table name.
618+
at_unit: Optional time travel unit (e.g. 'timestamp', 'version') —
619+
the schema at a past point may differ from the live one.
620+
`None` for the live schema.
621+
at_value: Optional time travel value, paired with `at_unit`.
614622
615623
Returns:
616624
[`TableInfo`][] for the table, or None if not found.
@@ -621,6 +629,8 @@ def table_get(
621629
attach_opaque_data=attach_opaque_data,
622630
schema_name=schema_name,
623631
name=name,
632+
at_unit=at_unit,
633+
at_value=at_value,
624634
transaction_opaque_data=transaction_opaque_data,
625635
).to_optional()
626636

@@ -774,6 +784,82 @@ def table_scan_function_get(
774784
batch, _ = deserialize_record_batch(result_bytes)
775785
return ScanFunctionResult.deserialize(batch)
776786

787+
def table_scan_branches_get(
788+
self,
789+
*,
790+
attach_opaque_data: AttachOpaqueData,
791+
transaction_opaque_data: TransactionOpaqueData | None = None,
792+
schema_name: str,
793+
name: str,
794+
at_unit: str | None = None,
795+
at_value: str | None = None,
796+
) -> ScanBranchesResult:
797+
"""Get the list of scan branches for a (possibly multi-source) table.
798+
799+
Multi-branch tables compose a logical scan from N physical sources
800+
(canonical case: Kafka hot tier + Iceberg cold tier). The VGI DuckDB
801+
extension's optimizer rewrites the placeholder scan into
802+
``LogicalSetOperation(UNION_ALL, ...)``, one arm per branch — a
803+
caller of this method is expected to do the equivalent (e.g.
804+
`pl.concat` over one scan per branch in vgi-polars).
805+
806+
Falls back to :meth:`table_scan_function_get`, wrapped as a
807+
single-branch `ScanBranchesResult`, when the worker raises
808+
`MethodNotImplementedError` for the additive `catalog_table_scan_
809+
branches_get` RPC — mirroring exactly how the C++ extension reads
810+
that same fallback, so a worker written before this RPC existed
811+
still works, just as a one-branch table.
812+
813+
Args:
814+
attach_opaque_data: The attachment ID from catalog_attach.
815+
transaction_opaque_data: Optional transaction ID for transactional reads.
816+
schema_name: The schema containing the table.
817+
name: The table name.
818+
at_unit: Optional time travel unit (e.g., 'timestamp', 'version').
819+
The C++ extension refuses `AT(...)` on tables with more than
820+
one branch at bind time — a worker returning multiple
821+
branches should expect this to always be `None`.
822+
at_value: Optional time travel value.
823+
824+
Returns:
825+
`ScanBranchesResult` with one or more `ScanBranch` entries plus
826+
the union of required DuckDB extensions across all branches.
827+
828+
"""
829+
with self._catalog_connect() as proxy:
830+
try:
831+
result_bytes = proxy.catalog_table_scan_branches_get(
832+
attach_opaque_data=attach_opaque_data,
833+
schema_name=schema_name,
834+
name=name,
835+
at_unit=at_unit,
836+
at_value=at_value,
837+
transaction_opaque_data=transaction_opaque_data,
838+
)
839+
except RpcError as e:
840+
if "MethodNotImplemented" not in type(e).__name__ and "not implemented" not in str(e).lower():
841+
raise
842+
scan_fn = self.table_scan_function_get(
843+
attach_opaque_data=attach_opaque_data,
844+
transaction_opaque_data=transaction_opaque_data,
845+
schema_name=schema_name,
846+
name=name,
847+
at_unit=at_unit,
848+
at_value=at_value,
849+
)
850+
return ScanBranchesResult(
851+
branches=[
852+
ScanBranch(
853+
function_name=scan_fn.function_name,
854+
positional_arguments=list(scan_fn.positional_arguments),
855+
named_arguments=dict(scan_fn.named_arguments),
856+
)
857+
],
858+
required_extensions=list(scan_fn.required_extensions),
859+
)
860+
batch, _ = deserialize_record_batch(result_bytes)
861+
return ScanBranchesResult.deserialize(batch)
862+
777863
def table_comment_set(
778864
self,
779865
*,

0 commit comments

Comments
 (0)