From 32486479dc3c01584adb32c946fddcf6c48b7c98 Mon Sep 17 00:00:00 2001 From: Tam Nguyen Duc <1218621+tamnd@users.noreply.github.com> Date: Tue, 18 Aug 2026 16:07:51 +0700 Subject: [PATCH] twenty-three wrong programs and what each is told DX2 asks for a misuse suite in both clients, and this is the Python half of it, against the same list the engine answers in crates/zu/tests/misuse.rs. Clear is the hard word in "a clear error for every deliberately wrong program", so the suite spells it out as three things a message has to do: name the thing the caller named, say what was expected instead, and be the engine's own sentence rather than a syscall's, because "failed to fill whole buffer" is a true statement about a read that tells nobody which file was not a database. Clear also means the right class, since a Python caller catches classes and reads messages afterwards. A file that is there and is not a database now raises ConnectionError rather than InternalError: it is the same typo as a file that is not there, landing on a real file, and telling somebody to file a bug about their own path would be the wrong answer twice. No leak is checked twice from outside the call that would cause one. Every case is followed by a read on the connection it was aimed at, and the failing connects are repeated five hundred times, which is past the descriptor limit a process starts with, then a database is opened and written to prove the descriptors are there and gc is asked whether any connection outlived its test. An appender the collector took while it still held rows is the one mistake that cannot be reported where it happens, so it warns: ResourceWarning naming the table and the rows it discarded. Flushing from a destructor is the other answer and is not available to this client, because a collector runs whenever it likes, including while another thread is inside a statement on the same connection. Going quietly is the failure mode worth a warning, since a loop that appended a million rows and never closed otherwise leaves an empty database and no complaint about it. The last test is the half of a misuse suite that is usually missing: the programs that look wrong and are not. A parameter no statement reads, a label nothing carries, a comparison across types, a result read twice, a second close, and a stop that arrives with nothing running are each a decision, and a decision nobody wrote down is a decision somebody reverses by accident. The interrupt case is where the two clients differ on purpose, and the test says why: this one clears before every statement, because the thread that asked cannot know whether the statement it meant to stop had already finished. The engine pin moves to the commit carrying the Rust half, which also names the file in every failure the operating system raised. Value now has Graph and BindingTable variants, so a reference goes to Python as the string that names it, the way the shell and the ABI's JSON already send it, and Arrow refuses it by name because Arrow has no type for it. --- Cargo.lock | 20 +- Cargo.toml | 4 +- README.md | 8 + src/appender.rs | 52 +++++ src/columns.rs | 11 ++ src/error.rs | 7 + src/value.rs | 9 + tests/test_misuse.py | 439 +++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 538 insertions(+), 12 deletions(-) create mode 100644 tests/test_misuse.py diff --git a/Cargo.lock b/Cargo.lock index 5ed99c6..18f8988 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1647,7 +1647,7 @@ dependencies = [ [[package]] name = "zu" version = "0.0.1" -source = "git+https://github.com/tamnd/zu?rev=6ceb6d50abe20cfbef97c3d0d033d051c0649521#6ceb6d50abe20cfbef97c3d0d033d051c0649521" +source = "git+https://github.com/tamnd/zu?rev=8009a961f463d6b576509e0f752b60f44071cd33#8009a961f463d6b576509e0f752b60f44071cd33" dependencies = [ "zu-common", "zu-encoding", @@ -1663,7 +1663,7 @@ dependencies = [ [[package]] name = "zu-common" version = "0.0.1" -source = "git+https://github.com/tamnd/zu?rev=6ceb6d50abe20cfbef97c3d0d033d051c0649521#6ceb6d50abe20cfbef97c3d0d033d051c0649521" +source = "git+https://github.com/tamnd/zu?rev=8009a961f463d6b576509e0f752b60f44071cd33#8009a961f463d6b576509e0f752b60f44071cd33" dependencies = [ "thiserror", ] @@ -1671,7 +1671,7 @@ dependencies = [ [[package]] name = "zu-encoding" version = "0.0.1" -source = "git+https://github.com/tamnd/zu?rev=6ceb6d50abe20cfbef97c3d0d033d051c0649521#6ceb6d50abe20cfbef97c3d0d033d051c0649521" +source = "git+https://github.com/tamnd/zu?rev=8009a961f463d6b576509e0f752b60f44071cd33#8009a961f463d6b576509e0f752b60f44071cd33" dependencies = [ "ruzstd", "zu-common", @@ -1680,7 +1680,7 @@ dependencies = [ [[package]] name = "zu-exec" version = "0.0.1" -source = "git+https://github.com/tamnd/zu?rev=6ceb6d50abe20cfbef97c3d0d033d051c0649521#6ceb6d50abe20cfbef97c3d0d033d051c0649521" +source = "git+https://github.com/tamnd/zu?rev=8009a961f463d6b576509e0f752b60f44071cd33#8009a961f463d6b576509e0f752b60f44071cd33" dependencies = [ "zu-common", "zu-query", @@ -1690,7 +1690,7 @@ dependencies = [ [[package]] name = "zu-query" version = "0.0.1" -source = "git+https://github.com/tamnd/zu?rev=6ceb6d50abe20cfbef97c3d0d033d051c0649521#6ceb6d50abe20cfbef97c3d0d033d051c0649521" +source = "git+https://github.com/tamnd/zu?rev=8009a961f463d6b576509e0f752b60f44071cd33#8009a961f463d6b576509e0f752b60f44071cd33" dependencies = [ "crossbeam-deque", "zu-common", @@ -1701,7 +1701,7 @@ dependencies = [ [[package]] name = "zu-s3" version = "0.0.1" -source = "git+https://github.com/tamnd/zu?rev=6ceb6d50abe20cfbef97c3d0d033d051c0649521#6ceb6d50abe20cfbef97c3d0d033d051c0649521" +source = "git+https://github.com/tamnd/zu?rev=8009a961f463d6b576509e0f752b60f44071cd33#8009a961f463d6b576509e0f752b60f44071cd33" dependencies = [ "crc32c", "object_store", @@ -1712,7 +1712,7 @@ dependencies = [ [[package]] name = "zu-sqlite" version = "0.0.1" -source = "git+https://github.com/tamnd/zu?rev=6ceb6d50abe20cfbef97c3d0d033d051c0649521#6ceb6d50abe20cfbef97c3d0d033d051c0649521" +source = "git+https://github.com/tamnd/zu?rev=8009a961f463d6b576509e0f752b60f44071cd33#8009a961f463d6b576509e0f752b60f44071cd33" dependencies = [ "rusqlite", "zu-common", @@ -1722,7 +1722,7 @@ dependencies = [ [[package]] name = "zu-storage" version = "0.0.1" -source = "git+https://github.com/tamnd/zu?rev=6ceb6d50abe20cfbef97c3d0d033d051c0649521#6ceb6d50abe20cfbef97c3d0d033d051c0649521" +source = "git+https://github.com/tamnd/zu?rev=8009a961f463d6b576509e0f752b60f44071cd33#8009a961f463d6b576509e0f752b60f44071cd33" dependencies = [ "zu-common", "zu-encoding", @@ -1731,7 +1731,7 @@ dependencies = [ [[package]] name = "zu-vector" version = "0.0.1" -source = "git+https://github.com/tamnd/zu?rev=6ceb6d50abe20cfbef97c3d0d033d051c0649521#6ceb6d50abe20cfbef97c3d0d033d051c0649521" +source = "git+https://github.com/tamnd/zu?rev=8009a961f463d6b576509e0f752b60f44071cd33#8009a961f463d6b576509e0f752b60f44071cd33" dependencies = [ "zu-common", ] @@ -1739,7 +1739,7 @@ dependencies = [ [[package]] name = "zu-zu1" version = "0.0.1" -source = "git+https://github.com/tamnd/zu?rev=6ceb6d50abe20cfbef97c3d0d033d051c0649521#6ceb6d50abe20cfbef97c3d0d033d051c0649521" +source = "git+https://github.com/tamnd/zu?rev=8009a961f463d6b576509e0f752b60f44071cd33#8009a961f463d6b576509e0f752b60f44071cd33" dependencies = [ "crc32c", "loom", diff --git a/Cargo.toml b/Cargo.toml index fa13847..bc41f04 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,8 +18,8 @@ crate-type = ["cdylib"] # with (ADR 0002), so a revision is the honest way to say which one. # A local checkout is used instead with a `paths` override in # `.cargo/config.toml`, which is untracked on purpose. -zudb = { package = "zu", git = "https://github.com/tamnd/zu", rev = "6ceb6d50abe20cfbef97c3d0d033d051c0649521" } -zu-common = { git = "https://github.com/tamnd/zu", rev = "6ceb6d50abe20cfbef97c3d0d033d051c0649521" } +zudb = { package = "zu", git = "https://github.com/tamnd/zu", rev = "8009a961f463d6b576509e0f752b60f44071cd33" } +zu-common = { git = "https://github.com/tamnd/zu", rev = "8009a961f463d6b576509e0f752b60f44071cd33" } # `extension-module` is asked for by maturin, in pyproject.toml, and # not here. Only the build backend knows how an extension is linked on # the platform it is building for, and a crate that turns the feature diff --git a/README.md b/README.md index c7f4332..5c21c06 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,8 @@ A row is every column of the table, in the order the table declares them, and th The `with` block closes, and closing flushes, including on the way out of a block that raised: a load that stopped partway is better served by its rows arriving than by them vanishing, and `discard()` is there for the caller who wants the other answer. A flush that fails keeps its rows, so what did not go in is still there to look at. +An appender nobody closed is the one mistake that cannot be reported where it happens, and it raises a `ResourceWarning` naming the table and the rows when the collector takes it. Flushing from there is the other answer and is not available, because a collector runs whenever it likes, including while another thread is inside a statement on the same connection. The warning is the whole point: a loop that appended a million rows and never closed leaves a database with nothing in it, and going quietly about that is worse than a line on stderr. + On this machine 200,000 rows of an integer and a string take 11 ms to buffer through `append_rows` and 93 ms to flush, which is 1.9 million rows a second including the commit. The same rows one call at a time cost 32 ms of buffering, since a call is a call. Against `INSERT`, 2,000 rows take 32 seconds a row at a time and 28 ms through an appender, and the gap widens with the table because every `INSERT` is a commit and a fold. ## Reading a result as columns @@ -101,6 +103,12 @@ conn.rows_read # how far the statement running now has got `interrupt()` is the one a program calls, from a thread that is not the one inside `execute`, and it raises `zudb.Interrupted` there. It is one of the three calls that may be made on a connection while a statement is running, with `rows_read` and `closed`, and none of the three waits for it: a progress bar drawn from `rows_read` is a poll of an atomic, not a queue behind the executor. +## When a program is wrong + +Every condition arrives as an exception class carrying its GQLSTATUS code, its position and a link to what the standard says about it, and the class is the class a Python caller would have written: a mistake the program made is a `zudb.ProgrammingError`, a value Python has and zu does not is a `TypeError`, a value of the right type and the wrong shape is a `ValueError`, and a file that is not a database is a `zudb.ConnectionError`, the same as a file that is not there. Both of those are a path that does not lead to a database, and telling a caller who mistyped one to file a bug would be the wrong answer twice. + +`tests/test_misuse.py` is twenty-three deliberately wrong programs and what each of them is told, run against the same list the engine runs in `crates/zu/tests/misuse.rs`. A message has to name the thing the caller named, say what was expected instead, and be the engine's own sentence rather than a syscall's, because "failed to fill whole buffer" is a true statement about a read that tells nobody which file was not a database. The suite checks the other two words too: nothing crashes, and nothing leaks, which is five hundred failing connects followed by a database that still opens and no connection left alive behind the collector's back. Half of it is the programs that look wrong and are not, since a parameter nothing reads, a label nothing carries and a second `close()` are all decisions somebody would otherwise reverse by accident. + ## Types The wheel carries `py.typed` and a stub for the compiled module, so mypy, pyright and an editor's completion all work with nothing else installed. `zudb.Value` is the union a row holds and a parameter takes, for code that passes rows around and wants to say so. diff --git a/src/appender.rs b/src/appender.rs index d63e10f..3b9c41f 100644 --- a/src/appender.rs +++ b/src/appender.rs @@ -31,6 +31,7 @@ use std::sync::{Mutex, MutexGuard}; +use pyo3::exceptions::PyResourceWarning; use pyo3::prelude::*; use pyo3::types::PyTuple; use zudb::zu1::catalog::Catalog; @@ -298,6 +299,57 @@ impl Appender { } } +/// An appender the collector took while it still held rows says so. +/// +/// The rows are gone by then, and going quietly is the failure mode +/// worth a warning: the caller wrote a loop that appended a million +/// rows, never closed the appender, and got a database with nothing in +/// it and no complaint about it. Flushing here instead is what the +/// Rust appender does when it is dropped, and it is not available to +/// this one: a collector runs whenever it likes, including while +/// another thread is inside a statement on the same connection, and a +/// commit from there would be a write nobody asked for at a moment +/// nobody chose. +/// +/// `ResourceWarning` is the class Python already uses for a file that +/// was never closed, which is the same mistake with the same cure, and +/// it is silent by default and loud under `-W error` and under pytest. +impl Drop for Appender { + fn drop(&mut self) { + let Ok(state) = self.state.lock() else { return }; + if !state.open || state.buffered == 0 { + return; + } + let Ok(message) = std::ffi::CString::new(format!( + "appender on '{}' was collected with {} row{} buffered and never closed, \ + so they were discarded: close it, or hold it in a `with` block", + self.table, + state.buffered, + if state.buffered == 1 { "" } else { "s" }, + )) else { + return; + }; + Python::attach(|py| { + // A collector runs wherever it likes, including in the + // middle of raising something else, and warning while an + // exception is set is an error in itself. The one being + // raised is put aside and put back, so the warning is an + // aside rather than a thing that replaces the failure the + // caller is about to see. + let raising = PyErr::take(py); + // A warning turned into an error by the caller's filters + // has nowhere to go from a destructor, so it is written to + // stderr the way Python writes any exception raised in one. + if let Err(raised) = PyErr::warn(py, &py.get_type::(), &message, 1) { + raised.write_unraisable(py, None); + } + if let Some(raising) = raising { + raising.restore(py); + } + }); + } +} + impl Appender { /// Opens an appender on `table`, which is a node table or a rel /// table of the graph this connection reads. diff --git a/src/columns.rs b/src/columns.rs index cae9a8e..df71e1d 100644 --- a/src/columns.rs +++ b/src/columns.rs @@ -337,6 +337,17 @@ fn kind_of(name: &str, row: usize, value: &Value) -> Result { ))); } }, + // GV60 and GV61. A handle is a reference, and a column of + // references is a column of nothing a frame can hold: the + // graph is in the file and the binding table is behind the + // handle. A caller who wants one in a frame reads the rows, + // where it arrives as the string that names it, or projects + // the columns of the table instead of the table. + Value::Graph(_) | Value::BindingTable(_) => { + return Err(Snag::Type(format!( + "row {row} of column '{name}' is a reference to a graph or a binding table, which Arrow has no type for" + ))); + } // Never in a result: the executor settles a chain into its // edges before the rows leave the pipeline. Value::Chain(_) => { diff --git a/src/error.rs b/src/error.rs index 6268b55..8738fa7 100644 --- a/src/error.rs +++ b/src/error.rs @@ -95,6 +95,13 @@ fn class_for(err: &ZuError) -> &'static str { // missing path as an internal error would tell the caller // to file a bug about their own typo. ZuError::Io(_) => "ConnectionError", + // And so is a file that is there and is not a database, + // which is the same typo landing on a real file. Every + // corruption a client can meet is a file it opened, so + // this is the connection failing rather than the engine, + // and the message says which file and what was wrong with + // it. + ZuError::Corrupt { .. } => "ConnectionError", _ => "InternalError", }; }; diff --git a/src/value.rs b/src/value.rs index 43a8ff1..d268cfa 100644 --- a/src/value.rs +++ b/src/value.rs @@ -336,6 +336,15 @@ pub fn to_py<'py>(py: Python<'py>, value: &Value, names: &Names) -> PyResult temporal_to_py(py, *t)?, + // GV60 and GV61. A reference goes across as the string that + // names it, `GRAPH /social` or `BINDING TABLE #3 (2 columns, 7 + // rows)`, which is what the shell prints and what the ABI's + // JSON carries. A handle is a reference on purpose: the graph + // is in the file and the table is behind the handle, so + // building a Python object that held either would copy the + // thing the value was passed by reference to avoid. + Value::Graph(handle) => handle.label().into_pyobject(py)?.into_any(), + Value::BindingTable(table) => table.label().into_pyobject(py)?.into_any(), // The executor settles a chain into its edge list before any // value leaves the pipeline, so a result never holds one and // seeing one here is a bug in the engine rather than something diff --git a/tests/test_misuse.py b/tests/test_misuse.py new file mode 100644 index 0000000..d8ada2e --- /dev/null +++ b/tests/test_misuse.py @@ -0,0 +1,439 @@ +"""Deliberately wrong programs, and what each of them is told. + +DX2 asks for a misuse suite in both clients: no crash, no leak, and a +clear error for every program that is wrong on purpose. Clear is the +hard word of the three, so it is spelled out here as three things a +message has to do. It names the thing the caller named, being the file +they opened, the column they appended to, the parameter they passed. +It says what was expected instead wherever there is something to say. +And it is the engine's own sentence rather than a syscall's, because +"failed to fill whole buffer" is a true statement about a read that +tells nobody which file was not a database. + +Clear also means the right class, since a Python caller catches classes +and reads messages. A mistake the program made is a `ProgrammingError`, +a value Python has and zu does not is a `TypeError`, a value of the +right type and the wrong shape is a `ValueError`, and a file that is +not a database is a `ConnectionError`, the same as a file that is not +there: both are a path that does not lead to a database, and telling a +caller who mistyped one to file a bug would be the wrong answer twice. + +No crash is the suite running at all. No leak is checked from outside +the call that would cause one, twice: every case is followed by a read +on the connection it was aimed at, and the failing connects are +repeated five hundred times, which is past the descriptor limit a +process starts with. + +The last two tests are the half of a misuse suite that is usually +missing. The programs that look wrong and are not, each of which is a +decision somebody would otherwise reverse by accident, and the one +mistake that cannot be reported where it happens: an appender the +collector took while it still held rows, which warns instead. +""" + +from __future__ import annotations + +import gc +import warnings +from collections.abc import Callable +from dataclasses import dataclass +from pathlib import Path + +import pytest +import zudb + +READ = "MATCH (p:person) RETURN p.uid AS uid" + + +@dataclass(frozen=True) +class Misuse: + """One deliberately wrong program. + + `run` gets a connection to a database with three people in it and a + directory to make a mess in. Returning normally fails the test: + every program in the table is wrong. + """ + + what: str + run: Callable[[zudb.Connection, Path], object] + raises: type[BaseException] + says: tuple[str, ...] + + +def junk(where: Path, name: str, contents: bytes) -> Path: + """A file that is not a database, written where a caller would have + a file that is not a database.""" + path = where / name + path.write_bytes(contents) + return path + + +def read_only(where: Path) -> zudb.Connection: + """A database of its own, written, closed and opened read-only. + + Written and closed first, because a read-only open reads the base + file as the last fold left it and a database still being written + has a fold outstanding. + """ + path = where / "reader.zu1" + if not path.exists(): + with zudb.connect(path) as conn: + conn.execute("INSERT (p:person {uid: 1, name: 'ada'})") + return zudb.connect(path, read_only=True) + + +MISUSES: tuple[Misuse, ...] = ( + Misuse( + "connects to a file too small to be a database", + lambda conn, tmp: zudb.connect(junk(tmp, "small.zu1", b"not a database at all")), + zudb.ConnectionError, + ("small.zu1", "21 bytes", "too short to be a zu1 database"), + ), + Misuse( + "connects to a file the right size and the wrong kind", + lambda conn, tmp: zudb.connect(junk(tmp, "big.zu1", b"x" * 40960)), + zudb.ConnectionError, + ("big.zu1", "not a zu1 file"), + ), + Misuse( + "connects read-only to a database that is not there", + # Read-only, because `connect` on a path with nothing at it + # makes a database and that is the documented answer. Asking + # for read-only is asking for one that already exists. + lambda conn, tmp: zudb.connect(tmp / "nowhere.zu1", read_only=True), + zudb.ConnectionError, + ("nowhere.zu1",), + ), + Misuse( + "writes through a connection it opened read-only", + lambda conn, tmp: read_only(tmp).execute("INSERT (p:person {uid: 4, name: 'zoe'})"), + zudb.ProgrammingError, + ("reader.zu1", "read-only"), + ), + Misuse( + "opens an appender on a connection it opened read-only", + lambda conn, tmp: read_only(tmp).appender("person"), + zudb.ProgrammingError, + ("appender writes", "read-only"), + ), + Misuse( + "runs text that will not parse", + lambda conn, tmp: conn.execute("MATCH (p:person) RETRUN p.uid"), + zudb.SyntaxError, + ("42001",), + ), + Misuse( + "leaves out a parameter the statement reads", + lambda conn, tmp: conn.execute("MATCH (p:person) WHERE p.uid = $uid RETURN p.uid AS uid"), + zudb.SyntaxError, + ("42002", "missing parameter $uid"), + ), + Misuse( + "passes a parameter of a type zu has no value for", + lambda conn, tmp: conn.execute("RETURN $x AS x", {"x": object()}), + TypeError, + ("a parameter cannot be a object", "zu holds"), + ), + Misuse( + "passes the parameters as a list", + lambda conn, tmp: conn.execute("RETURN $x AS x", [1, 2]), + TypeError, + ("dict",), + ), + Misuse( + "passes a statement that is not a string", + lambda conn, tmp: conn.execute(42), + TypeError, + ("str",), + ), + Misuse( + "opens an appender on a table that is not there", + lambda conn, tmp: conn.appender("nope"), + zudb.ProgrammingError, + ("no node table or rel table 'nope'",), + ), + Misuse( + "appends a row shorter than the table", + lambda conn, tmp: conn.appender("person").append_row([4]), + ValueError, + ("this row carries 1 values and 'person' takes 3", "uid, name, score"), + ), + Misuse( + "appends a row with a value the column does not take", + lambda conn, tmp: conn.appender("person").append_row(["four", "zoe", 1.0]), + TypeError, + ("value 0 of this row", "column 'uid' of 'person' holds integers"), + ), + Misuse( + "appends a row that is not a row", + lambda conn, tmp: conn.appender("person").append_row(4), + TypeError, + ("not iterable",), + ), + Misuse( + "appends to an appender it closed", + lambda conn, tmp: closed_appender(conn).append_row([4, "zoe", 1.0]), + zudb.ProgrammingError, + ("this appender is closed",), + ), + Misuse( + "runs a statement on a connection it closed", + lambda conn, tmp: closed_connection(tmp).execute(READ), + zudb.ProgrammingError, + ("this connection is closed",), + ), + Misuse( + "flushes an appender whose connection is closed", + lambda conn, tmp: orphaned_appender(tmp).flush(), + zudb.ProgrammingError, + ("the connection this appender writes through is closed",), + ), + Misuse( + "loads columns of different lengths", + lambda conn, tmp: zudb.load( + tmp / "ragged.zu1", + nodes="person", + columns={"uid": [1, 2], "name": ["ada"]}, + ), + ValueError, + ("column 'name' holds 1 values and column 'uid' holds 2",), + ), + Misuse( + "loads a column that holds two types", + lambda conn, tmp: zudb.load( + tmp / "mixed.zu1", + nodes="person", + columns={"uid": [1, "two"]}, + ), + TypeError, + ("column 'uid' holds integers and row 1 is of type 'str'",), + ), + Misuse( + "loads an edge to a row that is not there", + lambda conn, tmp: zudb.load( + tmp / "dangling.zu1", + nodes="person", + rels="knows", + columns={"uid": [1, 2]}, + edges=[(0, 9)], + ), + ValueError, + ("edge 0 joins row 9 of a table with 2 rows in it",), + ), + Misuse( + "loads an edge that is not a pair of rows", + lambda conn, tmp: zudb.load( + tmp / "single.zu1", + nodes="person", + rels="knows", + columns={"uid": [1, 2]}, + edges=[(0,)], + ), + TypeError, + ("edge 0 is not a pair of row numbers",), + ), + Misuse( + "loads over a database that is already there", + lambda conn, tmp: zudb.load(conn.path, nodes="person", columns={"uid": [1]}), + zudb.ConnectionError, + ("social.zu1",), + ), + Misuse( + "loads nothing at all", + lambda conn, tmp: zudb.load(tmp / "nothing.zu1", nodes="person"), + ValueError, + ("a load with no columns has no rows to count",), + ), +) + + +def closed_appender(conn: zudb.Connection) -> zudb.Appender: + """An appender that has been closed, which is the state a `with` + block leaves one in.""" + rows = conn.appender("person") + rows.close() + return rows + + +def closed_connection(where: Path) -> zudb.Connection: + """A connection of its own, closed. Its own, because the case that + uses it would otherwise close the connection every other case is + checked against afterwards.""" + conn = zudb.connect(where / "closed.zu1") + conn.close() + return conn + + +def orphaned_appender(where: Path) -> zudb.Appender: + """An appender holding a row, whose connection was closed under it.""" + conn = zudb.connect(where / "orphan.zu1") + conn.execute("INSERT (p:person {uid: 1, name: 'ada'})") + rows = conn.appender("person") + rows.append_row([2, "grace"]) + conn.close() + return rows + + +@pytest.mark.parametrize("case", [pytest.param(case, id=case.what) for case in MISUSES]) +def test_a_wrong_program_is_told_what_is_wrong( + case: Misuse, social: zudb.Connection, tmp_path: Path +) -> None: + with pytest.raises(case.raises) as raised: + case.run(social, tmp_path) + message = str(raised.value) + for phrase in case.says: + assert phrase in message, f"the message is missing '{phrase}': {message}" + # The engine's sentence, not the read that noticed. + assert "failed to fill whole buffer" not in message + # And the connection it was aimed at is still a connection: the + # failure took nothing with it. + assert len(social.execute(READ).fetchall()) == 3 + + +def test_every_condition_is_catchable_as_the_class_a_caller_writes() -> None: + """The classes the table above uses, as a caller sees them. + + A caller writes `except zudb.Error` around a database call and + `except (TypeError, ValueError)` around the values they built, so + which of the two a mistake lands in is part of the contract rather + than an implementation detail. + """ + for case in MISUSES: + if issubclass(case.raises, zudb.Error): + continue + assert case.raises in (TypeError, ValueError), case.what + assert not issubclass(case.raises, zudb.Error), case.what + + +def test_five_hundred_failed_connections_leave_nothing_open(tmp_path: Path) -> None: + """A failure that keeps the file open is a failure a program can + only make a few hundred times, and the few hundredth is where it is + found, in production, as a limit nobody thought was near.""" + missing = tmp_path / "nowhere.zu1" + small = junk(tmp_path, "small.zu1", b"not a database at all") + + for _ in range(500): + with pytest.raises(zudb.ConnectionError): + zudb.connect(missing, read_only=True) + with pytest.raises(zudb.ConnectionError): + zudb.connect(small) + + # A descriptor per failure would have run out long ago, and a + # database made now is readable and writable. + with zudb.connect(tmp_path / "after.zu1") as conn: + conn.execute("INSERT (p:person {uid: 1, name: 'ada'})") + assert conn.execute(READ).fetchall() == [(1,)] + + # And nothing is holding a connection open behind the collector's + # back, which is the other way a client leaks: the objects are gone + # and the files they held went with them. + gc.collect() + alive = [obj for obj in gc.get_objects() if isinstance(obj, zudb.Connection)] + assert alive == [] + + +def test_a_statement_that_failed_wrote_nothing_and_left_the_connection_alone( + social: zudb.Connection, +) -> None: + """A statement that fails partway is where "no crash" is not + enough: the connection has to be where it was, and so does the + data.""" + with pytest.raises(zudb.DataError): + social.execute("INSERT (p:person {uid: 1 / 0, name: 'zoe', score: 1.0})") + assert len(social.execute(READ).fetchall()) == 3 + + # The same for an appender: a value the column does not take is + # refused by the call that appended it, and the rows already + # buffered are still there to flush. + with social.appender("person") as rows: + rows.append_row([40, "zoe", 1.0]) + with pytest.raises(TypeError): + rows.append_row(["fifty", "yvonne", 2.0]) + assert len(social.execute(READ).fetchall()) == 4 + + +def test_the_programs_that_look_like_misuse_and_are_not( + social: zudb.Connection, tmp_path: Path +) -> None: + """Each of these is a decision, and a decision nobody wrote down is + a decision somebody reverses by accident.""" + # A parameter the statement does not read is not an error. A caller + # that passes one dict to several statements is doing something + # reasonable, and refusing it would make the dict the union of what + # every statement wants. + assert len(social.execute(READ, {"unread": 1}).fetchall()) == 3 + + # A label nothing carries matches nothing. A pattern with no answer + # is the ordinary answer to a question about a graph, and the other + # reading gives a query that fails on the day the last row of a + # label is deleted. + assert social.execute("MATCH (p:nobody) RETURN p.uid AS uid").fetchall() == [] + + # A value of the wrong type against a column is a comparison that + # is false rather than a failure, which is what GQL says about + # comparing across types. + assert ( + social.execute( + f"{READ.rsplit(' RETURN', 1)[0]} WHERE p.uid = $uid RETURN p.uid AS uid", {"uid": "ada"} + ).fetchall() + == [] + ) + + # A result is rows that were already read, so reading it twice + # gives the same rows twice rather than an empty second answer, + # which is the mistake every cursor API teaches people to expect. + result = social.execute(READ) + assert result.fetchall() == result.fetchall() + assert len(list(result)) == 3 + + # Closing twice is not an error. The second close has nothing to + # do, and a caller cleaning up in a loop is allowed to ask. + conn = zudb.connect(tmp_path / "twice.zu1") + conn.close() + conn.close() + + # A stop with nothing running is dropped rather than kept. This + # client clears the flag before every statement, because the thread + # that asked cannot know whether the statement it meant to stop had + # already finished, and a `Ctrl-C` pressed a moment too late would + # otherwise end the next thing the notebook runs. + social.interrupt() + assert len(social.execute(READ).fetchall()) == 3 + + +def test_an_appender_the_collector_took_with_rows_in_it_says_so( + social: zudb.Connection, +) -> None: + """The one mistake that cannot be reported where it happens. + + Rows buffered in an appender nobody closed are discarded when the + collector takes it, and going quietly is the failure mode worth a + warning: a loop that appended a million rows, no `close`, a + database with nothing in it and no complaint about it. Flushing + from a destructor is the other answer and is not available, since a + collector runs whenever it likes, including while another thread is + inside a statement on the same connection. + """ + rows = social.appender("person") + rows.append_row([40, "zoe", 1.0]) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + del rows + gc.collect() + assert len(caught) == 1, [str(warning.message) for warning in caught] + assert caught[0].category is ResourceWarning + message = str(caught[0].message) + assert "appender on 'person'" in message + assert "1 row buffered" in message + assert "`with` block" in message + # The rows went nowhere, which is what the warning is about. + assert len(social.execute(READ).fetchall()) == 3 + + # An appender with nothing in it is not a mistake and says nothing: + # a warning nobody can act on is a warning people learn to filter. + empty = social.appender("person") + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + del empty + gc.collect() + assert caught == []