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
21 changes: 21 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,27 @@
it, fails with 40001 instead of deleting the new creator's vector.

`ALTER EXTENSION pg_warren UPDATE` creates the five functions.
- **Quotas on readers' vectors** (DESIGN.md §5.7, §14 items 46 and 47;
docs/semantics.md, "Quotas on readers' vectors"). A graph's owner may
bound the persisted vectors each other role stores on the graph, counted
as their elements' bytes across all versions:
`warren.set_vector_quota(name, bytes, role DEFAULT NULL)` sets the
graph's default (`info`'s new `reader_vector_quota`) or a role's own
quota, and `warren.vector_usage(name)` shows each role's vectors, bytes
and quota (a reader sees its own row). A persisted write that would take
the writer past its quota, and above what it stored, is refused with
53400; replacing a vector with one no larger always works. Members of
the owner role are exempt, and transient vectors are not counted. One
role's writes to one graph take a transaction-scoped lock on (graph,
role), shown in `pg_locks` as `advisory` with `objsubid` 22353.
`reassign_owned` counts the vectors it moves against the new role's
quotas. `drop_owned` and the `DROP OWNED` trigger remove a role's quotas
only on graphs whose owner the caller is a member of; `orphans()` and
`drop_orphans()` handle those of dropped roles. A quota bounds a role's
vectors on other owners' graphs, not the catalog's total storage. `ALTER EXTENSION pg_warren UPDATE` adds
the column, the table `warren.vector_quotas` (dumped with the
extension), an index on `warren.vectors (graph_id, created_by)` and the
two functions; existing graphs have no quota.

### Fixed

Expand Down
12 changes: 8 additions & 4 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,16 @@ before changing code. Commands, toolchain setup and environment gotchas are in
`version/` (diff, compaction across versions).
- `pg_warren/src/`: `extension` (schema + `warren.version()`, SQL
errors), `access/` (identity switches, privilege checks), `source/`
(node/edge query columns and read-only cursors), `catalog/`,
(node/edge query columns and read-only cursors), `catalog/` (and
`quota`, the lock and decision shared by vector writes and ownership
moves),
`segment/` (chunked persistence), `cache/` (loader; `shared`, the shared cache across backends, and `shared::introspection`, `warren.shared_cache()`), `build/`,
`version_sql/` (compact, diff), `interrupts` (the main-thread poll),
`read_only` (single-row SPI reads that assign no transaction id),
`parallel_worker` (what a parallel worker records and raises),
`external_ids/` (id arguments ↔ dense ids), `selection/` (direction and
relations), `traversal_sql/`, `vector/` (persistence and cache of named
vectors), `compute_sql/`, `spectral_sql/` (eigen, distance),
vectors; `quota`, the bound on readers' vectors), `compute_sql/`, `spectral_sql/` (eigen, distance),
`community_sql/` (contract, project, lift, modularity, local moving,
and the Louvain/Leiden library).
Feature dependencies form a DAG: `access`, `source` → `catalog` →
Expand Down Expand Up @@ -132,8 +134,10 @@ before changing code. Commands, toolchain setup and environment gotchas are in
with `Spi::connect_mut` + `update`. Clippy's `disallowed-methods`
(`clippy.toml`) enforces this; test modules allow it on the `mod` item.
- **Parallel labels** (DESIGN.md §9.3): writers and anything that may persist
are `PARALLEL UNSAFE`; anything that reads a graph or vector is `PARALLEL
RESTRICTED`; only functions that read no graph are `PARALLEL SAFE`.
are `PARALLEL UNSAFE`; anything that reads a vector, or a graph in a way
§9.3 does not list as safe, is `PARALLEL RESTRICTED`; graph reads that use
no vector, catalog-only reads and functions that read no graph are
`PARALLEL SAFE`.
`parallel_labels`'s test declares every function's label; add each new
function there (and its read to `READS` if it is not unsafe).
- **Every SQL entry point uses the guard.** The body of every
Expand Down
51 changes: 36 additions & 15 deletions DESIGN.md

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -290,10 +290,10 @@ See [CHANGELOG.md](CHANGELOG.md). This is pre-1.0 software: the SQL surface
may still change between minor versions, with an upgrade script each time.

Not there yet: the rest of the road to 1.0 (DESIGN.md §13) is text ids,
quotas and observability (M10; ownership transfer, `warren.set_owner`, and
cleanup after a dropped role, `warren.reassign_owned`, `warren.drop_owned`,
the optional `DROP OWNED` trigger and `warren.drop_orphans`, are built for
0.6.0);
and observability (M10; ownership transfer, `warren.set_owner`, cleanup
after a dropped role, `warren.reassign_owned`, `warren.drop_owned`, the
optional `DROP OWNED` trigger and `warren.drop_orphans`, and quotas on
readers' vectors, `warren.set_vector_quota`, are built for 0.6.0);
benchmarks and robustness on real graphs and deployments, and packages
(M11); and a reviewed, frozen SQL surface (M12, 1.0.0). Incremental
maintenance, general sparse matrix algebra (contraction's `PᵀAP` is the
Expand Down
6 changes: 5 additions & 1 deletion crates/pg_warren/src/access/guard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ mod tests {
("revoke", "warren.revoke('victim', 'eve')"),
("s", "warren.s(NULL)"),
("set_owner", "warren.set_owner('victim', 'eve')"),
("set_vector_quota", "warren.set_vector_quota('victim', 1000000, 'eve')"),
("shared_cache", "warren.shared_cache()"),
("shortest_path", "warren.shortest_path('victim', 1::bigint, 2::bigint)"),
("similar_pairs", "warren.similar_pairs('victim', NULL::bigint)"),
Expand All @@ -320,6 +321,7 @@ mod tests {
("vector_reduce", "warren.vector_reduce('victim', 'c', 'sum')"),
("vector_stats", "warren.vector_stats('victim', 'c')"),
("vector_topk", "warren.vector_topk('victim', 'c', 3, NULL::bigint)"),
("vector_usage", "warren.vector_usage('victim')"),
("vector_values", "warren.vector_values('victim', 'c', NULL::bigint)"),
("version", "warren.version()"),
("walk", "warren.walk('victim', ARRAY[1::bigint], ARRAY[warren.s(NULL)])"),
Expand Down Expand Up @@ -369,10 +371,12 @@ mod tests {
// Label vector `c` and quotient `victim_q`, which eve reads, so
// that contract, project and lift run all the way as eve; and
// `eve_minion`, whose privileges eve has, so that drop_owned and
// reassign_owned run all the way too (it owns nothing).
// reassign_owned run all the way too (it owns nothing); and a
// quota on readers' vectors, so that eve's writes are counted.
Spi::run(
"CREATE ROLE eve_minion; GRANT eve_minion TO eve;
SELECT warren.grant('victim', 'eve');
SELECT warren.set_vector_quota('victim', 1000000);
SELECT warren.vector_const('victim', 'c', 1);
SELECT warren.contract('victim', 'c', 'victim_q');
SELECT warren.vector_const('victim_q', 'c', 1);
Expand Down
60 changes: 55 additions & 5 deletions crates/pg_warren/src/catalog/bootstrap.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! The catalog tables, created by the extension script (DESIGN.md §5.1):
//! `warren.graphs`, `versions`, `segments`, `vectors` and `vector_chunks`,
//! revoked from `PUBLIC` and registered with `pg_extension_config_dump`.
//! `warren.graphs`, `versions`, `segments`, `vectors`, `vector_chunks`
//! and `vector_quotas`, revoked from `PUBLIC` and registered with
//! `pg_extension_config_dump`.

// Emitted at the end of the script (`finalize`), after `CREATE SCHEMA warren`.
// Functions reach these tables only at run time, through SPI.
Expand All @@ -25,7 +26,8 @@ CREATE TABLE warren.graphs (
current_version bigint,
store_merged boolean NOT NULL DEFAULT true,
node_order text NOT NULL DEFAULT 'id'
CHECK (node_order IN ('id'::pg_catalog.text, 'locality'::pg_catalog.text))
CHECK (node_order IN ('id'::pg_catalog.text, 'locality'::pg_catalog.text)),
reader_vector_quota bigint CHECK (reader_vector_quota >= 0)
);

CREATE TABLE warren.versions (
Expand Down Expand Up @@ -79,6 +81,17 @@ CREATE TABLE warren.vector_chunks (
);
-- IEEE floats rarely compress, so skip pglz's attempt on every write.
ALTER TABLE warren.vector_chunks ALTER COLUMN data SET STORAGE EXTERNAL;
-- A quota sums one role's vectors on one graph (DESIGN.md §5.7).
CREATE INDEX vectors_created_by ON warren.vectors (graph_id, created_by);

-- Per-role quotas on readers' persisted vectors, overriding
-- graphs.reader_vector_quota (DESIGN.md §5.7, §14 item 46).
CREATE TABLE warren.vector_quotas (
graph_id int NOT NULL REFERENCES warren.graphs ON DELETE CASCADE,
role regrole NOT NULL,
bytes bigint NOT NULL CHECK (bytes >= 0),
PRIMARY KEY (graph_id, role)
);

-- graphs.current_version is not a foreign key: with versions.graph_id it
-- would make a cycle, and pg_dump cannot order the data of tables in a
Expand Down Expand Up @@ -164,6 +177,7 @@ SELECT pg_catalog.pg_extension_config_dump('warren.segments', '');
SELECT pg_catalog.pg_extension_config_dump('warren.vectors', '');
SELECT pg_catalog.pg_extension_config_dump('warren.vectors_vector_id_seq', '');
SELECT pg_catalog.pg_extension_config_dump('warren.vector_chunks', '');
SELECT pg_catalog.pg_extension_config_dump('warren.vector_quotas', '');
"#,
name = "catalog",
finalize,
Expand Down Expand Up @@ -231,6 +245,37 @@ mod tests {
Ok(())
}

/// A quota sums one role's vectors on one graph (DESIGN.md §5.7).
#[pg_test]
fn vectors_are_indexed_by_graph_and_creator() -> Result<(), Error> {
let index = Spi::get_one::<String>(
"SELECT pg_get_indexdef('warren.vectors_created_by'::regclass)",
)?;
assert_eq!(
index.as_deref(),
Some("CREATE INDEX vectors_created_by ON warren.vectors USING btree (graph_id, created_by)")
);
Ok(())
}

#[pg_test]
fn quotas_are_bytes_not_negative() -> Result<(), Error> {
ring("quota_check", 3, 2)?;
assert_eq!(
sqlstate("UPDATE warren.graphs SET reader_vector_quota = -1")?.as_deref(),
Some("23514")
);
assert_eq!(
sqlstate(
"INSERT INTO warren.vector_quotas SELECT graph_id, 'quota_check_owner', -1
FROM warren.graphs"
)?
.as_deref(),
Some("23514")
);
Ok(())
}

#[pg_test]
fn versions_are_indexed_by_parent_and_label_vector() -> Result<(), Error> {
let indexes = Spi::get_one::<String>(
Expand Down Expand Up @@ -293,7 +338,7 @@ mod tests {
build_reverse boolean not null, weighted boolean not null, \
has_relations boolean not null, retain_versions integer not null, \
current_version bigint, store_merged boolean not null, \
node_order text not null"
node_order text not null, reader_vector_quota bigint"
)
);
Ok(())
Expand Down Expand Up @@ -338,6 +383,10 @@ mod tests {
columns("vector_chunks")?.as_deref(),
Some("vector_id bigint not null, chunk_no integer not null, data bytea not null")
);
assert_eq!(
columns("vector_quotas")?.as_deref(),
Some("graph_id integer not null, role regrole not null, bytes bigint not null")
);
Ok(())
}

Expand Down Expand Up @@ -374,7 +423,8 @@ mod tests {
dumped.as_deref(),
Some(
"warren.graphs, warren.graphs_graph_id_seq, warren.segments, \
warren.vector_chunks, warren.vectors, warren.vectors_vector_id_seq, \
warren.vector_chunks, warren.vector_quotas, warren.vectors, \
warren.vectors_vector_id_seq, \
warren.versions, warren.versions_version_id_seq"
)
);
Expand Down
38 changes: 32 additions & 6 deletions crates/pg_warren/src/catalog/dropped_roles/drop_owned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@
use pgrx::prelude::*;

use super::super::CatalogError;
use super::clean_up_after;
use super::{clean_up_after, Cleaner};
use crate::access::{as_catalog_owner, entry, refuse_bootstrap_superuser, require_privileges_of};

/// `warren.drop_owned(role regrole)`: drops every graph `role` owns (with
/// its versions and everyone's vectors, as `drop_graph` does), deletes the
/// vectors it wrote on other graphs and removes it from every graph's
/// readers. The caller needs the privileges of `role`. Returns the number
/// of graphs dropped.
/// readers, and its quotas on readers' vectors on graphs whose owner the
/// caller is a member of. The caller needs the privileges of `role`.
/// Returns the number of graphs dropped.
#[pg_extern(
schema = "warren",
sql = r#"
Expand All @@ -26,6 +27,7 @@ fn drop_owned(role: pg_sys::Oid) -> i64 {
fn drop_everything_of(role: pg_sys::Oid) -> Result<i64, CatalogError> {
require_privileges_of(role, "drop its graphs and vectors")?;
refuse_bootstrap_superuser(role, "drop")?;
let by = Cleaner::current()?;
let dropped = as_catalog_owner(|| {
Spi::connect_mut(|client| {
// Deleting a graph's row takes its lock, waiting for a running
Expand All @@ -38,7 +40,7 @@ fn drop_everything_of(role: pg_sys::Oid) -> Result<i64, CatalogError> {
&[role.into()],
)?
.len();
clean_up_after(client, role)?;
clean_up_after(client, role, by)?;
Ok::<_, CatalogError>(dropped)
})
})?;
Expand All @@ -52,14 +54,15 @@ mod tests {
use pgrx::prelude::*;
use pgrx::spi::Error;

use crate::fixture::{failure, failure_as, neighbouring_graphs, ownership};
use crate::fixture::{failure, failure_as, neighbouring_graphs, ownership, quotas};

#[pg_test]
fn drop_owned_drops_the_roles_graphs_its_vectors_and_its_reader_entries() -> Result<(), Error> {
fn drop_owned_drops_the_roles_graphs_vectors_reader_entries_and_quotas() -> Result<(), Error> {
neighbouring_graphs()?;
let dropped = Spi::get_one::<i64>("SELECT warren.drop_owned('dr_owner')")?;
assert_eq!(dropped, Some(1));
assert_eq!(ownership()?.as_deref(), Some("dk:dk_owner:{} / dk.theirs:dk_owner"));
assert_eq!(quotas()?.as_deref(), Some(""));
Ok(())
}

Expand Down Expand Up @@ -116,4 +119,27 @@ mod tests {
);
Ok(())
}

/// The security review of PR #59: a reader that drops what it owns
/// keeps the quota another graph's owner set on it; the sweep removes
/// it once the role is dropped.
#[pg_test]
fn a_role_that_drops_what_it_owns_keeps_its_quotas_on_others_graphs() -> Result<(), Error> {
neighbouring_graphs()?;
assert_eq!(failure_as("dr_owner", "warren.drop_owned('dr_owner')")?, None);
assert_eq!(quotas()?.as_deref(), Some("dk:dr_owner:4096"));
assert_eq!(ownership()?.as_deref(), Some("dk:dk_owner:{} / dk.theirs:dk_owner"));
Ok(())
}

/// The owner of the graph a quota is on may remove it with the role's
/// other entries.
#[pg_test]
fn the_graphs_owner_removes_a_roles_quota_with_drop_owned() -> Result<(), Error> {
neighbouring_graphs()?;
Spi::run("GRANT dr_owner TO dk_owner")?;
assert_eq!(failure_as("dk_owner", "warren.drop_owned('dr_owner')")?, None);
assert_eq!(quotas()?.as_deref(), Some(""));
Ok(())
}
}
30 changes: 26 additions & 4 deletions crates/pg_warren/src/catalog/dropped_roles/event_trigger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use pgrx::prelude::*;
use pgrx::PgList;

use super::super::CatalogError;
use super::clean_up_after;
use super::{clean_up_after, Cleaner};
use crate::access::{as_catalog_owner, entry, has_privileges_of, role_name};
use crate::read_only::read_one;

Expand Down Expand Up @@ -77,10 +77,11 @@ fn refuse_or_clean_up(roles: &[pg_sys::Oid]) -> Result<(), CatalogError> {
if let Some(message) = refusal(&owned) {
return Err(CatalogError::OwnsGraphs(message));
}
let by = Cleaner::current()?;
as_catalog_owner(|| {
Spi::connect_mut(|client| {
for &role in roles {
clean_up_after(client, role)?;
clean_up_after(client, role, by)?;
}
Ok::<_, CatalogError>(())
})
Expand Down Expand Up @@ -131,7 +132,7 @@ mod tests {
use pgrx::prelude::*;
use pgrx::spi::Error;

use crate::fixture::{failure, neighbouring_graphs, ownership, statement_failure_as};
use crate::fixture::{failure, neighbouring_graphs, ownership, quotas, statement_failure_as};

/// The trigger as `docs/semantics.md` tells a superuser to create it.
fn install() -> Result<(), Error> {
Expand Down Expand Up @@ -187,7 +188,8 @@ mod tests {
}

#[pg_test]
fn after_reassign_owned_drop_owned_removes_the_roles_reader_entries() -> Result<(), Error> {
fn after_reassign_owned_drop_owned_removes_the_roles_reader_entries_and_quotas(
) -> Result<(), Error> {
neighbouring_graphs()?;
install()?;
Spi::run(
Expand All @@ -199,6 +201,7 @@ mod tests {
ownership()?.as_deref(),
Some("dk:dk_owner:{} dr:dr_heir:{} / dk.mine:dr_heir dk.theirs:dk_owner")
);
assert_eq!(quotas()?.as_deref(), Some(""));
Ok(())
}

Expand All @@ -219,6 +222,25 @@ mod tests {
Ok(())
}

/// The security review of PR #59: a role's own `DROP OWNED` leaves the
/// quota a graph's owner set on it.
#[pg_test]
fn a_roles_own_drop_owned_keeps_its_quotas() -> Result<(), Error> {
neighbouring_graphs()?;
install()?;
Spi::run(
"CREATE ROLE dk_reader; SELECT warren.grant('dk', 'dk_reader');
SELECT warren.set_vector_quota('dk', 0, 'dk_reader')",
)?;
assert_eq!(statement_failure_as("dk_reader", "DROP OWNED BY dk_reader")?, None);
assert_eq!(quotas()?.as_deref(), Some("dk:dk_reader:0 dk:dr_owner:4096"));
assert_eq!(
ownership()?.as_deref(),
Some("dk:dk_owner:{dr_owner} dr:dr_owner:{} / dk.mine:dr_owner dk.theirs:dk_owner")
);
Ok(())
}

/// `DROP OWNED` checks privileges after `ddl_command_start`, so the
/// trigger leaves a role the caller lacks the privileges of, unread, to
/// `DROP OWNED`'s own refusal; and a role that does not exist to its
Expand Down
Loading
Loading