Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
52 changes: 52 additions & 0 deletions src/appender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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::<PyResourceWarning>(), &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.
Expand Down
11 changes: 11 additions & 0 deletions src/columns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,17 @@ fn kind_of(name: &str, row: usize, value: &Value) -> Result<Kind, Snag> {
)));
}
},
// 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(_) => {
Expand Down
7 changes: 7 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
};
};
Expand Down
9 changes: 9 additions & 0 deletions src/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,15 @@ pub fn to_py<'py>(py: Python<'py>, value: &Value, names: &Names) -> PyResult<Bou
.into_any()
}
Value::Temporal(t) => 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
Expand Down
Loading
Loading