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 == []