Skip to content

Commit 1d30a71

Browse files
timsaucerclaude
andcommitted
test: port codec Rust tests to pytest
The Rust tests added for the composable codec work never ran: CI invokes `cargo fmt` and `cargo clippy --all-targets` but no `cargo test`, so the tests compiled and were never executed. Rather than add a `cargo test` job — which would also require feature-gating `pyo3/extension-module`, since the test binary cannot link on Linux while it is unconditional — move the coverage to pytest, matching this repository's practice of treating the user-facing Python surface as the first line of defense. Remove both `#[cfg(test)]` modules from crates/core/src/codec.rs and replace them as follows: - Four wire-header round-trip tests and the Python-minor-mismatch test were already covered by existing cases in test_pickle_expr.py. - `strip_errors_on_too_old_version` asserted nothing: it returns early because WIRE_VERSION_MIN_SUPPORTED equals WIRE_VERSION_CURRENT. - The unsupported-wire-version and Python-major-mismatch cases move to test_pickle_expr.py, patching the header in place inside the encoded protobuf. The patches preserve length so the outer message stays parseable and the bytes reach the codec. - The three truncated-header cases are dropped. Truncation changes the payload length and breaks the protobuf framing, so they fail before reaching the header check and cannot be expressed from Python. - The codec-chain tests move to the FFI example suite, which exercises the same chain through the real FFI boundary. MyLogicalExtensionCodec gains an optional token overriding the byte prefix it stamps on encoded table providers. Two instances with distinct tokens own disjoint slices of the wire format, which is what makes chain ordering and fall-through observable from Python. The ported inlining test asserts encode and decode behavior rather than the `python_udf_inlining()` getter the Rust test checked. This is a stronger assertion: the getter is preserved even when a composed Python-aware codec re-inlines a UDF that the outer strict codec declined to inline, so the original test could not have caught that path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0672411 commit 1d30a71

5 files changed

Lines changed: 194 additions & 316 deletions

File tree

crates/core/src/codec.rs

Lines changed: 0 additions & 312 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,315 +1182,3 @@ fn decode_python_udaf(py: Python<'_>, payload: &[u8]) -> PyResult<PythonFunction
11821182
volatility,
11831183
))
11841184
}
1185-
1186-
#[cfg(test)]
1187-
mod wire_header_tests {
1188-
use super::*;
1189-
1190-
const TEST_PY: (u8, u8) = (3, 12);
1191-
1192-
#[test]
1193-
fn strip_returns_none_when_family_absent() {
1194-
let buf = b"OTHER_PAYLOAD";
1195-
assert!(matches!(
1196-
strip_wire_header(buf, PY_SCALAR_UDF_FAMILY, "scalar UDF", TEST_PY),
1197-
Ok(None)
1198-
));
1199-
}
1200-
1201-
#[test]
1202-
fn strip_errors_on_truncated_version_byte() {
1203-
let buf = PY_SCALAR_UDF_FAMILY;
1204-
let err = strip_wire_header(buf, PY_SCALAR_UDF_FAMILY, "scalar UDF", TEST_PY).unwrap_err();
1205-
assert!(format!("{err}").contains("missing wire-format version byte"));
1206-
}
1207-
1208-
#[test]
1209-
fn strip_errors_on_too_new_version() {
1210-
let mut buf = PY_SCALAR_UDF_FAMILY.to_vec();
1211-
buf.push(WIRE_VERSION_CURRENT.saturating_add(1));
1212-
buf.push(TEST_PY.0);
1213-
buf.push(TEST_PY.1);
1214-
buf.extend_from_slice(b"payload");
1215-
let err = strip_wire_header(&buf, PY_SCALAR_UDF_FAMILY, "scalar UDF", TEST_PY).unwrap_err();
1216-
let msg = format!("{err}");
1217-
assert!(msg.contains("wire-format version v"));
1218-
assert!(msg.contains("supports"));
1219-
assert!(msg.contains("Align datafusion-python versions"));
1220-
}
1221-
1222-
#[test]
1223-
fn strip_errors_on_too_old_version() {
1224-
if WIRE_VERSION_MIN_SUPPORTED == 0 {
1225-
return;
1226-
}
1227-
let mut buf = PY_SCALAR_UDF_FAMILY.to_vec();
1228-
buf.push(WIRE_VERSION_MIN_SUPPORTED - 1);
1229-
buf.push(TEST_PY.0);
1230-
buf.push(TEST_PY.1);
1231-
buf.extend_from_slice(b"payload");
1232-
assert!(strip_wire_header(&buf, PY_SCALAR_UDF_FAMILY, "scalar UDF", TEST_PY).is_err());
1233-
}
1234-
1235-
#[test]
1236-
fn strip_errors_on_truncated_py_major() {
1237-
let mut buf = PY_SCALAR_UDF_FAMILY.to_vec();
1238-
buf.push(WIRE_VERSION_CURRENT);
1239-
let err = strip_wire_header(&buf, PY_SCALAR_UDF_FAMILY, "scalar UDF", TEST_PY).unwrap_err();
1240-
assert!(format!("{err}").contains("missing Python major version byte"));
1241-
}
1242-
1243-
#[test]
1244-
fn strip_errors_on_truncated_py_minor() {
1245-
let mut buf = PY_SCALAR_UDF_FAMILY.to_vec();
1246-
buf.push(WIRE_VERSION_CURRENT);
1247-
buf.push(TEST_PY.0);
1248-
let err = strip_wire_header(&buf, PY_SCALAR_UDF_FAMILY, "scalar UDF", TEST_PY).unwrap_err();
1249-
assert!(format!("{err}").contains("missing Python minor version byte"));
1250-
}
1251-
1252-
#[test]
1253-
fn strip_errors_on_py_minor_mismatch() {
1254-
let mut buf = Vec::new();
1255-
write_wire_header(&mut buf, PY_SCALAR_UDF_FAMILY, (3, 11));
1256-
buf.extend_from_slice(b"payload");
1257-
let err = strip_wire_header(&buf, PY_SCALAR_UDF_FAMILY, "scalar UDF", (3, 12)).unwrap_err();
1258-
let msg = format!("{err}");
1259-
assert!(msg.contains("Python 3.11"));
1260-
assert!(msg.contains("Python 3.12"));
1261-
assert!(msg.contains("not portable across Python minor versions"));
1262-
}
1263-
1264-
#[test]
1265-
fn strip_errors_on_py_major_mismatch() {
1266-
let mut buf = Vec::new();
1267-
write_wire_header(&mut buf, PY_SCALAR_UDF_FAMILY, (3, 12));
1268-
buf.extend_from_slice(b"payload");
1269-
assert!(strip_wire_header(&buf, PY_SCALAR_UDF_FAMILY, "scalar UDF", (4, 0)).is_err());
1270-
}
1271-
1272-
#[test]
1273-
fn write_then_strip_round_trips_scalar_payload() {
1274-
let mut buf = Vec::new();
1275-
write_wire_header(&mut buf, PY_SCALAR_UDF_FAMILY, TEST_PY);
1276-
buf.extend_from_slice(b"scalar-payload");
1277-
1278-
let payload = strip_wire_header(&buf, PY_SCALAR_UDF_FAMILY, "scalar UDF", TEST_PY)
1279-
.unwrap()
1280-
.unwrap();
1281-
assert_eq!(payload, b"scalar-payload");
1282-
}
1283-
1284-
#[test]
1285-
fn write_then_strip_round_trips_agg_payload() {
1286-
let mut buf = Vec::new();
1287-
write_wire_header(&mut buf, PY_AGG_UDF_FAMILY, TEST_PY);
1288-
buf.extend_from_slice(b"agg-payload");
1289-
1290-
let payload = strip_wire_header(&buf, PY_AGG_UDF_FAMILY, "aggregate UDF", TEST_PY)
1291-
.unwrap()
1292-
.unwrap();
1293-
assert_eq!(payload, b"agg-payload");
1294-
}
1295-
1296-
#[test]
1297-
fn write_then_strip_round_trips_window_payload() {
1298-
let mut buf = Vec::new();
1299-
write_wire_header(&mut buf, PY_WINDOW_UDF_FAMILY, TEST_PY);
1300-
buf.extend_from_slice(b"window-payload");
1301-
1302-
let payload = strip_wire_header(&buf, PY_WINDOW_UDF_FAMILY, "window UDF", TEST_PY)
1303-
.unwrap()
1304-
.unwrap();
1305-
assert_eq!(payload, b"window-payload");
1306-
}
1307-
1308-
#[test]
1309-
fn strip_does_not_match_a_different_family() {
1310-
let mut buf = Vec::new();
1311-
write_wire_header(&mut buf, PY_SCALAR_UDF_FAMILY, TEST_PY);
1312-
buf.extend_from_slice(b"payload");
1313-
assert!(matches!(
1314-
strip_wire_header(&buf, PY_WINDOW_UDF_FAMILY, "window UDF", TEST_PY),
1315-
Ok(None)
1316-
));
1317-
}
1318-
}
1319-
1320-
#[cfg(test)]
1321-
mod codec_chain_tests {
1322-
use std::sync::atomic::{AtomicUsize, Ordering};
1323-
1324-
use datafusion::catalog::MemTable;
1325-
use datafusion::common::exec_err;
1326-
1327-
use super::*;
1328-
1329-
/// Codec that owns a single byte token for table providers and
1330-
/// errors on everything else, mirroring the family-prefix
1331-
/// discipline expected of downstream FFI codecs.
1332-
#[derive(Debug)]
1333-
struct TokenCodec {
1334-
token: &'static [u8],
1335-
/// Return `Ok` from `try_encode_table_provider` without
1336-
/// writing bytes, imitating a "no opinion" codec.
1337-
encode_by_name: bool,
1338-
decode_hits: AtomicUsize,
1339-
encode_hits: AtomicUsize,
1340-
}
1341-
1342-
impl TokenCodec {
1343-
fn new(token: &'static [u8]) -> Arc<Self> {
1344-
Arc::new(Self {
1345-
token,
1346-
encode_by_name: false,
1347-
decode_hits: AtomicUsize::new(0),
1348-
encode_hits: AtomicUsize::new(0),
1349-
})
1350-
}
1351-
1352-
fn new_by_name(token: &'static [u8]) -> Arc<Self> {
1353-
Arc::new(Self {
1354-
token,
1355-
encode_by_name: true,
1356-
decode_hits: AtomicUsize::new(0),
1357-
encode_hits: AtomicUsize::new(0),
1358-
})
1359-
}
1360-
}
1361-
1362-
impl LogicalExtensionCodec for TokenCodec {
1363-
fn try_decode(
1364-
&self,
1365-
_buf: &[u8],
1366-
_inputs: &[LogicalPlan],
1367-
_ctx: &TaskContext,
1368-
) -> Result<Extension> {
1369-
exec_err!("TokenCodec does not decode extension nodes")
1370-
}
1371-
1372-
fn try_encode(&self, _node: &Extension, _buf: &mut Vec<u8>) -> Result<()> {
1373-
exec_err!("TokenCodec does not encode extension nodes")
1374-
}
1375-
1376-
fn try_decode_table_provider(
1377-
&self,
1378-
buf: &[u8],
1379-
_table_ref: &TableReference,
1380-
schema: SchemaRef,
1381-
_ctx: &TaskContext,
1382-
) -> Result<Arc<dyn TableProvider>> {
1383-
if buf != self.token {
1384-
return exec_err!("Unknown table provider token for TokenCodec");
1385-
}
1386-
self.decode_hits.fetch_add(1, Ordering::SeqCst);
1387-
Ok(Arc::new(MemTable::try_new(schema, vec![vec![]])?))
1388-
}
1389-
1390-
fn try_encode_table_provider(
1391-
&self,
1392-
_table_ref: &TableReference,
1393-
_node: Arc<dyn TableProvider>,
1394-
buf: &mut Vec<u8>,
1395-
) -> Result<()> {
1396-
self.encode_hits.fetch_add(1, Ordering::SeqCst);
1397-
if !self.encode_by_name {
1398-
buf.extend_from_slice(self.token);
1399-
}
1400-
Ok(())
1401-
}
1402-
}
1403-
1404-
fn mem_table() -> Arc<dyn TableProvider> {
1405-
Arc::new(MemTable::try_new(Arc::new(Schema::empty()), vec![vec![]]).unwrap())
1406-
}
1407-
1408-
fn table_ref() -> TableReference {
1409-
TableReference::bare("t")
1410-
}
1411-
1412-
#[test]
1413-
fn decode_falls_through_to_earlier_installed_codec() {
1414-
let first = TokenCodec::new(b"AAAA");
1415-
let second = TokenCodec::new(b"BBBB");
1416-
let codec = PythonLogicalCodec::default()
1417-
.with_additional_codec(first.clone())
1418-
.with_additional_codec(second.clone());
1419-
1420-
let ctx = TaskContext::default();
1421-
codec
1422-
.try_decode_table_provider(b"AAAA", &table_ref(), Arc::new(Schema::empty()), &ctx)
1423-
.unwrap();
1424-
1425-
assert_eq!(first.decode_hits.load(Ordering::SeqCst), 1);
1426-
assert_eq!(second.decode_hits.load(Ordering::SeqCst), 0);
1427-
}
1428-
1429-
#[test]
1430-
fn most_recently_installed_codec_encodes_first() {
1431-
let first = TokenCodec::new(b"AAAA");
1432-
let second = TokenCodec::new(b"BBBB");
1433-
let codec = PythonLogicalCodec::default()
1434-
.with_additional_codec(first.clone())
1435-
.with_additional_codec(second.clone());
1436-
1437-
let mut buf = Vec::new();
1438-
codec
1439-
.try_encode_table_provider(&table_ref(), mem_table(), &mut buf)
1440-
.unwrap();
1441-
1442-
assert_eq!(buf, b"BBBB");
1443-
assert_eq!(first.encode_hits.load(Ordering::SeqCst), 0);
1444-
}
1445-
1446-
#[test]
1447-
fn empty_ok_encode_lets_later_codec_write_payload() {
1448-
let writer = TokenCodec::new(b"AAAA");
1449-
let by_name = TokenCodec::new_by_name(b"BBBB");
1450-
let codec = PythonLogicalCodec::default()
1451-
.with_additional_codec(writer.clone())
1452-
.with_additional_codec(by_name.clone());
1453-
1454-
let mut buf = Vec::new();
1455-
codec
1456-
.try_encode_table_provider(&table_ref(), mem_table(), &mut buf)
1457-
.unwrap();
1458-
1459-
assert_eq!(buf, b"AAAA");
1460-
assert_eq!(by_name.encode_hits.load(Ordering::SeqCst), 1);
1461-
assert_eq!(writer.encode_hits.load(Ordering::SeqCst), 1);
1462-
}
1463-
1464-
#[test]
1465-
fn decode_failure_aggregates_every_codec_error() {
1466-
let codec = PythonLogicalCodec::default()
1467-
.with_additional_codec(TokenCodec::new(b"AAAA"))
1468-
.with_additional_codec(TokenCodec::new(b"BBBB"));
1469-
1470-
let ctx = TaskContext::default();
1471-
let err = codec
1472-
.try_decode_table_provider(b"????", &table_ref(), Arc::new(Schema::empty()), &ctx)
1473-
.unwrap_err();
1474-
1475-
let msg = err.to_string();
1476-
assert!(msg.contains("None of the 3 composed extension codecs"));
1477-
assert!(msg.contains("Unknown table provider token"));
1478-
}
1479-
1480-
#[test]
1481-
fn single_codec_chain_error_is_returned_verbatim() {
1482-
let codec = PythonLogicalCodec::default();
1483-
let ctx = TaskContext::default();
1484-
let err = codec
1485-
.try_decode_table_provider(b"????", &table_ref(), Arc::new(Schema::empty()), &ctx)
1486-
.unwrap_err();
1487-
assert!(!err.to_string().contains("composed extension codecs"));
1488-
}
1489-
1490-
#[test]
1491-
fn with_additional_codec_preserves_udf_inlining_setting() {
1492-
let strict = PythonLogicalCodec::default().with_python_udf_inlining(false);
1493-
let extended = strict.with_additional_codec(TokenCodec::new(b"AAAA"));
1494-
assert!(!extended.python_udf_inlining());
1495-
}
1496-
}

examples/datafusion-ffi-example/README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ The example codecs do not inspect the callback `TaskContext`. A production codec
3737

3838
Extension codecs compose: each `with_logical_extension_codec` / `with_physical_extension_codec` call prepends the codec to the session's codec chain, with the most recently installed codec consulted first and DataFusion's default codec as the terminal fallback. A codec signals "not mine" by returning an error, so several independent plugin libraries can install codecs on the same session as long as each only answers for payloads it owns (frame them with a distinct byte prefix). In this example the provider library is the only codec owner; the planner uses built-in physical nodes and receives the provider codecs from the host.
3939

40+
`MyLogicalExtensionCodec` takes an optional token argument (`MyLogicalExtensionCodec("TOKENAAA")`) that overrides the byte prefix it stamps on encoded table providers. It exists so the tests can install two instances that own disjoint slices of the wire format, which is what makes chain ordering and fall-through observable from Python. Real plugin libraries should hard-code a prefix unique to the library rather than accept one from the caller.
41+
4042
Register both provider codecs before installing the planner:
4143

4244
```python

0 commit comments

Comments
 (0)