diff --git a/Cargo.lock b/Cargo.lock index 5360122ed95..1168aa9a382 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3773,6 +3773,7 @@ dependencies = [ "hash-graph-authorization", "hash-graph-embeddings", "hash-graph-migrations", + "hash-graph-postgres-store", "hash-graph-store", "hash-graph-temporal-versioning", "hash-graph-test-data", diff --git a/apps/hash-graph/docs/dependency-diagram.mmd b/apps/hash-graph/docs/dependency-diagram.mmd index c5aad87b71e..e99c111ad68 100644 --- a/apps/hash-graph/docs/dependency-diagram.mmd +++ b/apps/hash-graph/docs/dependency-diagram.mmd @@ -67,7 +67,8 @@ graph TD 7 --> 8 7 --> 34 9 --> 6 - 9 -.-> 7 + 9 --> 7 + 9 -.-> 9 9 --> 15 9 --> 33 10 --> 5 diff --git a/libs/@blockprotocol/type-system/rust/docs/dependency-diagram.mmd b/libs/@blockprotocol/type-system/rust/docs/dependency-diagram.mmd index caefa648975..f948008763b 100644 --- a/libs/@blockprotocol/type-system/rust/docs/dependency-diagram.mmd +++ b/libs/@blockprotocol/type-system/rust/docs/dependency-diagram.mmd @@ -48,6 +48,7 @@ graph TD 5 --> 1 6 --> 11 7 --> 6 + 7 -.-> 7 7 --> 12 8 --> 5 8 --> 11 diff --git a/libs/@local/codec/docs/dependency-diagram.mmd b/libs/@local/codec/docs/dependency-diagram.mmd index 046b44f0057..ccff16ca7bc 100644 --- a/libs/@local/codec/docs/dependency-diagram.mmd +++ b/libs/@local/codec/docs/dependency-diagram.mmd @@ -53,6 +53,7 @@ graph TD 5 --> 1 6 --> 11 7 --> 6 + 7 -.-> 7 7 --> 12 8 --> 5 8 --> 11 diff --git a/libs/@local/codegen/docs/dependency-diagram.mmd b/libs/@local/codegen/docs/dependency-diagram.mmd index 08c5bbadc20..079e2a37734 100644 --- a/libs/@local/codegen/docs/dependency-diagram.mmd +++ b/libs/@local/codegen/docs/dependency-diagram.mmd @@ -49,6 +49,7 @@ graph TD 5 --> 1 6 --> 11 7 --> 6 + 7 -.-> 7 7 --> 12 8 --> 5 8 --> 11 diff --git a/libs/@local/graph/api/docs/dependency-diagram.mmd b/libs/@local/graph/api/docs/dependency-diagram.mmd index b15023971bf..27e84cc7fde 100644 --- a/libs/@local/graph/api/docs/dependency-diagram.mmd +++ b/libs/@local/graph/api/docs/dependency-diagram.mmd @@ -68,7 +68,8 @@ graph TD 7 --> 8 7 --> 34 9 --> 6 - 9 -.-> 7 + 9 --> 7 + 9 -.-> 9 9 --> 15 9 --> 33 10 --> 5 diff --git a/libs/@local/graph/api/src/rest/entity/mod.rs b/libs/@local/graph/api/src/rest/entity/mod.rs index bc10f42d808..a3867e98ae6 100644 --- a/libs/@local/graph/api/src/rest/entity/mod.rs +++ b/libs/@local/graph/api/src/rest/entity/mod.rs @@ -509,7 +509,7 @@ where .await .map_err(report_to_response)?; - let store = store_pool + let mut store = store_pool .acquire(temporal_client.0) .await .map_err(report_to_response)?; diff --git a/libs/@local/graph/api/src/rest/entity/query/mod.rs b/libs/@local/graph/api/src/rest/entity/query/mod.rs index 556af5e3212..9be5deeffcb 100644 --- a/libs/@local/graph/api/src/rest/entity/query/mod.rs +++ b/libs/@local/graph/api/src/rest/entity/query/mod.rs @@ -73,7 +73,7 @@ where .attach(hash_status::StatusCode::InvalidArgument) .map_err(report_to_response)?; - let store = store_pool + let mut store = store_pool .acquire(temporal_client.0) .await .map_err(report_to_response)?; @@ -151,7 +151,7 @@ where .attach(hash_status::StatusCode::InvalidArgument) .map_err(report_to_response)?; - let store = store_pool + let mut store = store_pool .acquire(temporal_client.0) .await .map_err(report_to_response)?; diff --git a/libs/@local/graph/authorization/docs/dependency-diagram.mmd b/libs/@local/graph/authorization/docs/dependency-diagram.mmd index aa9283d70f0..555820f3fe9 100644 --- a/libs/@local/graph/authorization/docs/dependency-diagram.mmd +++ b/libs/@local/graph/authorization/docs/dependency-diagram.mmd @@ -48,6 +48,7 @@ graph TD 5 --> 1 6 --> 11 7 --> 6 + 7 -.-> 7 7 --> 12 8 --> 5 8 --> 11 diff --git a/libs/@local/graph/embeddings/docs/dependency-diagram.mmd b/libs/@local/graph/embeddings/docs/dependency-diagram.mmd index 5aa3d1e61bb..6ba835c7fe1 100644 --- a/libs/@local/graph/embeddings/docs/dependency-diagram.mmd +++ b/libs/@local/graph/embeddings/docs/dependency-diagram.mmd @@ -48,6 +48,7 @@ graph TD 6 --> 10 6 -.-> 21 7 --> 6 + 7 -.-> 7 8 --> 5 8 --> 10 8 --> 19 diff --git a/libs/@local/graph/migrations-macros/docs/dependency-diagram.mmd b/libs/@local/graph/migrations-macros/docs/dependency-diagram.mmd index 427345a63b3..39bd5f98c6a 100644 --- a/libs/@local/graph/migrations-macros/docs/dependency-diagram.mmd +++ b/libs/@local/graph/migrations-macros/docs/dependency-diagram.mmd @@ -26,7 +26,8 @@ graph TD 1 --> 7 1 --> 10 2 --> 3 - 4 -.-> 2 + 4 --> 2 + 4 -.-> 4 5 -.-> 6 6 --> 7 6 --> 10 diff --git a/libs/@local/graph/migrations/docs/dependency-diagram.mmd b/libs/@local/graph/migrations/docs/dependency-diagram.mmd index 9e314eca731..b8e6c3be379 100644 --- a/libs/@local/graph/migrations/docs/dependency-diagram.mmd +++ b/libs/@local/graph/migrations/docs/dependency-diagram.mmd @@ -29,7 +29,8 @@ graph TD 1 --> 10 2 --> 3 2 --> 11 - 4 -.-> 2 + 4 --> 2 + 4 -.-> 4 5 -.-> 6 6 --> 7 6 --> 10 diff --git a/libs/@local/graph/migrations/graph-migrations/v001__extensions/mod.rs b/libs/@local/graph/migrations/graph-migrations/v001__extensions/mod.rs index 02810f6060e..2f4ead8f514 100644 --- a/libs/@local/graph/migrations/graph-migrations/v001__extensions/mod.rs +++ b/libs/@local/graph/migrations/graph-migrations/v001__extensions/mod.rs @@ -1,5 +1,5 @@ use error_stack::Report; -use hash_graph_migrations::{Context, Migration}; +use hash_graph_migrations::{ContextTransaction, Migration}; use tokio_postgres::Client; use tracing::Instrument as _; @@ -11,7 +11,7 @@ impl Migration for Extensions { async fn up( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("up.sql")) @@ -27,7 +27,7 @@ impl Migration for Extensions { async fn down( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("down.sql")) diff --git a/libs/@local/graph/migrations/graph-migrations/v002__principals/mod.rs b/libs/@local/graph/migrations/graph-migrations/v002__principals/mod.rs index 715de5a8e6b..952fa256486 100644 --- a/libs/@local/graph/migrations/graph-migrations/v002__principals/mod.rs +++ b/libs/@local/graph/migrations/graph-migrations/v002__principals/mod.rs @@ -1,5 +1,5 @@ use error_stack::Report; -use hash_graph_migrations::{Context, Migration}; +use hash_graph_migrations::{ContextTransaction, Migration}; use tokio_postgres::Client; use tracing::Instrument as _; @@ -11,7 +11,7 @@ impl Migration for Principals { async fn up( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("up.sql")) @@ -27,7 +27,7 @@ impl Migration for Principals { async fn down( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("down.sql")) diff --git a/libs/@local/graph/migrations/graph-migrations/v003__policies/mod.rs b/libs/@local/graph/migrations/graph-migrations/v003__policies/mod.rs index fd449a3b02b..fa3c9b06fcf 100644 --- a/libs/@local/graph/migrations/graph-migrations/v003__policies/mod.rs +++ b/libs/@local/graph/migrations/graph-migrations/v003__policies/mod.rs @@ -1,5 +1,5 @@ use error_stack::Report; -use hash_graph_migrations::{Context, Migration}; +use hash_graph_migrations::{ContextTransaction, Migration}; use tokio_postgres::Client; use tracing::Instrument as _; @@ -11,7 +11,7 @@ impl Migration for Policies { async fn up( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("up.sql")) @@ -27,7 +27,7 @@ impl Migration for Policies { async fn down( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("down.sql")) diff --git a/libs/@local/graph/migrations/graph-migrations/v004__webs/mod.rs b/libs/@local/graph/migrations/graph-migrations/v004__webs/mod.rs index 7ee3bb69fc9..a5991624e77 100644 --- a/libs/@local/graph/migrations/graph-migrations/v004__webs/mod.rs +++ b/libs/@local/graph/migrations/graph-migrations/v004__webs/mod.rs @@ -1,5 +1,5 @@ use error_stack::Report; -use hash_graph_migrations::{Context, Migration}; +use hash_graph_migrations::{ContextTransaction, Migration}; use tokio_postgres::Client; pub struct Webs; @@ -10,14 +10,14 @@ impl Migration for Webs { fn up( self, - _context: &mut ::Transaction<'_>, + _context: &mut ContextTransaction<'_, Self::Context>, ) -> impl Future>> { core::future::ready(Ok(())) } fn down( self, - _context: &mut ::Transaction<'_>, + _context: &mut ContextTransaction<'_, Self::Context>, ) -> impl Future>> { core::future::ready(Ok(())) } diff --git a/libs/@local/graph/migrations/graph-migrations/v005__common_ontology/mod.rs b/libs/@local/graph/migrations/graph-migrations/v005__common_ontology/mod.rs index 5a3549eb4de..98ef3fe81d9 100644 --- a/libs/@local/graph/migrations/graph-migrations/v005__common_ontology/mod.rs +++ b/libs/@local/graph/migrations/graph-migrations/v005__common_ontology/mod.rs @@ -1,5 +1,5 @@ use error_stack::Report; -use hash_graph_migrations::{Context, Migration}; +use hash_graph_migrations::{ContextTransaction, Migration}; use tokio_postgres::Client; use tracing::Instrument as _; @@ -11,7 +11,7 @@ impl Migration for CommonOntology { async fn up( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("up.sql")) @@ -27,7 +27,7 @@ impl Migration for CommonOntology { async fn down( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("down.sql")) diff --git a/libs/@local/graph/migrations/graph-migrations/v006__data_types/mod.rs b/libs/@local/graph/migrations/graph-migrations/v006__data_types/mod.rs index ff76161a734..63b8134b899 100644 --- a/libs/@local/graph/migrations/graph-migrations/v006__data_types/mod.rs +++ b/libs/@local/graph/migrations/graph-migrations/v006__data_types/mod.rs @@ -1,5 +1,5 @@ use error_stack::Report; -use hash_graph_migrations::{Context, Migration}; +use hash_graph_migrations::{ContextTransaction, Migration}; use tokio_postgres::Client; use tracing::Instrument as _; @@ -11,7 +11,7 @@ impl Migration for DataTypes { async fn up( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("up.sql")) @@ -27,7 +27,7 @@ impl Migration for DataTypes { async fn down( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("down.sql")) diff --git a/libs/@local/graph/migrations/graph-migrations/v007__property_types/mod.rs b/libs/@local/graph/migrations/graph-migrations/v007__property_types/mod.rs index dc5cf8df990..ec5911e4edc 100644 --- a/libs/@local/graph/migrations/graph-migrations/v007__property_types/mod.rs +++ b/libs/@local/graph/migrations/graph-migrations/v007__property_types/mod.rs @@ -1,5 +1,5 @@ use error_stack::Report; -use hash_graph_migrations::{Context, Migration}; +use hash_graph_migrations::{ContextTransaction, Migration}; use tokio_postgres::Client; use tracing::Instrument as _; @@ -11,7 +11,7 @@ impl Migration for PropertyTypes { async fn up( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("up.sql")) @@ -27,7 +27,7 @@ impl Migration for PropertyTypes { async fn down( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("down.sql")) diff --git a/libs/@local/graph/migrations/graph-migrations/v008__entity_types/mod.rs b/libs/@local/graph/migrations/graph-migrations/v008__entity_types/mod.rs index 0f234ecd5db..f596eb7fcaf 100644 --- a/libs/@local/graph/migrations/graph-migrations/v008__entity_types/mod.rs +++ b/libs/@local/graph/migrations/graph-migrations/v008__entity_types/mod.rs @@ -1,5 +1,5 @@ use error_stack::Report; -use hash_graph_migrations::{Context, Migration}; +use hash_graph_migrations::{ContextTransaction, Migration}; use tokio_postgres::Client; use tracing::Instrument as _; @@ -11,7 +11,7 @@ impl Migration for EntityTypes { async fn up( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("up.sql")) @@ -27,7 +27,7 @@ impl Migration for EntityTypes { async fn down( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("down.sql")) diff --git a/libs/@local/graph/migrations/graph-migrations/v009__entities/mod.rs b/libs/@local/graph/migrations/graph-migrations/v009__entities/mod.rs index 7336e2ec00e..bc94f05d524 100644 --- a/libs/@local/graph/migrations/graph-migrations/v009__entities/mod.rs +++ b/libs/@local/graph/migrations/graph-migrations/v009__entities/mod.rs @@ -1,5 +1,5 @@ use error_stack::Report; -use hash_graph_migrations::{Context, Migration}; +use hash_graph_migrations::{ContextTransaction, Migration}; use tokio_postgres::Client; use tracing::Instrument as _; @@ -11,7 +11,7 @@ impl Migration for Entities { async fn up( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("up.sql")) @@ -27,7 +27,7 @@ impl Migration for Entities { async fn down( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("down.sql")) diff --git a/libs/@local/graph/migrations/graph-migrations/v010__query/mod.rs b/libs/@local/graph/migrations/graph-migrations/v010__query/mod.rs index ef889d4e2ba..c240e8ab751 100644 --- a/libs/@local/graph/migrations/graph-migrations/v010__query/mod.rs +++ b/libs/@local/graph/migrations/graph-migrations/v010__query/mod.rs @@ -1,5 +1,5 @@ use error_stack::Report; -use hash_graph_migrations::{Context, Migration}; +use hash_graph_migrations::{ContextTransaction, Migration}; use tokio_postgres::Client; use tracing::Instrument as _; @@ -11,7 +11,7 @@ impl Migration for Query { async fn up( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("up.sql")) @@ -27,7 +27,7 @@ impl Migration for Query { async fn down( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report> { context .simple_query(include_str!("down.sql")) diff --git a/libs/@local/graph/migrations/src/context.rs b/libs/@local/graph/migrations/src/context.rs index a7072ae1f85..2eab2afb6e9 100644 --- a/libs/@local/graph/migrations/src/context.rs +++ b/libs/@local/graph/migrations/src/context.rs @@ -1,7 +1,20 @@ use core::error::Error; use error_stack::Report; -// use tokio_postgres::{Client, Transaction}; + +/// The isolation level of a database transaction. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum IsolationLevel { + /// An individual statement in the transaction will see rows committed before it began. + ReadCommitted, + /// All statements in the transaction will see the same view of rows committed before the + /// first query in the transaction. + RepeatableRead, + /// The reads and writes in this transaction must be able to be committed as an atomic "unit" + /// with respect to reads and writes of all other concurrent serializable transactions + /// without interleaving. + Serializable, +} pub trait Transaction { type Error: Error + Send + Sync + 'static; @@ -10,13 +23,53 @@ pub trait Transaction { async fn rollback(self) -> Result<(), Report>; } +/// A configurable, in-progress request to begin a [`Transaction`]. +/// +/// The builder is created by [`Context::transaction`] and begins the transaction when awaited. +/// Options adjust the transaction to be begun, e.g. its [`IsolationLevel`] or access mode: +/// +/// ```ignore +/// let transaction = context +/// .transaction() +/// .isolation_level(IsolationLevel::RepeatableRead) +/// .read_only() +/// .await?; +/// ``` +pub trait TransactionBuilder: + IntoFuture>> +{ + type Transaction: Transaction; + type Error: Error + Send + Sync + 'static; + + /// Sets the isolation level of the transaction. + #[must_use] + fn isolation_level(self, isolation_level: IsolationLevel) -> Self; + + /// Marks the transaction as read-only. + #[must_use] + fn read_only(self) -> Self; + + /// Marks the transaction as deferrable. + /// + /// If the transaction is also serializable and read-only, beginning the transaction may + /// block, but when it completes the transaction is able to run with less overhead and a + /// guarantee that it will not be aborted due to serialization failure. + #[must_use] + fn deferrable(self) -> Self; +} + +/// The [`Transaction`] type begun by a [`Context`]'s [`TransactionBuilder`]. +pub type ContextTransaction<'c, C> = + <::TransactionBuilder<'c> as TransactionBuilder>::Transaction; + pub trait Context { - type Transaction<'c>: Transaction + type Error: Error + Send + Sync + 'static; + type TransactionBuilder<'c>: TransactionBuilder where Self: 'c; - type Error: Error + Send + Sync + 'static; - async fn transaction(&mut self) -> Result, Report>; + /// Returns a [`TransactionBuilder`] which begins the transaction when awaited. + fn transaction(&mut self) -> Self::TransactionBuilder<'_>; } /// Provides the context for a migration. diff --git a/libs/@local/graph/migrations/src/lib.rs b/libs/@local/graph/migrations/src/lib.rs index ea45685a750..004bbf92364 100644 --- a/libs/@local/graph/migrations/src/lib.rs +++ b/libs/@local/graph/migrations/src/lib.rs @@ -15,7 +15,10 @@ extern crate alloc; pub use ::hash_graph_migrations_macros::embed_migrations; pub use self::{ - context::{Context, ContextProvider, Transaction}, + context::{ + Context, ContextProvider, ContextTransaction, IsolationLevel, Transaction, + TransactionBuilder, + }, info::{Digest, InvalidMigrationFile, MigrationInfo}, list::{MigrationError, MigrationList}, migration::Migration, diff --git a/libs/@local/graph/migrations/src/migration.rs b/libs/@local/graph/migrations/src/migration.rs index 86cd0d34925..6699c9d3ffd 100644 --- a/libs/@local/graph/migrations/src/migration.rs +++ b/libs/@local/graph/migrations/src/migration.rs @@ -2,7 +2,7 @@ use core::error::Error; use error_stack::Report; -use crate::context::Context; +use crate::context::{Context, ContextTransaction}; pub trait Migration { type Context: Context; @@ -11,11 +11,11 @@ pub trait Migration { async fn up( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report>; async fn down( self, - context: &mut ::Transaction<'_>, + context: &mut ContextTransaction<'_, Self::Context>, ) -> Result<(), Report>; } diff --git a/libs/@local/graph/migrations/src/postgres.rs b/libs/@local/graph/migrations/src/postgres.rs index 8bda3faca92..725f92e19a3 100644 --- a/libs/@local/graph/migrations/src/postgres.rs +++ b/libs/@local/graph/migrations/src/postgres.rs @@ -9,9 +9,19 @@ use tracing::Instrument as _; use crate::{ Digest, MigrationInfo, MigrationState, StateStore, - context::{Context, Transaction}, + context::{Context, IsolationLevel, Transaction, TransactionBuilder}, }; +impl From for tokio_postgres::IsolationLevel { + fn from(isolation_level: IsolationLevel) -> Self { + match isolation_level { + IsolationLevel::ReadCommitted => Self::ReadCommitted, + IsolationLevel::RepeatableRead => Self::RepeatableRead, + IsolationLevel::Serializable => Self::Serializable, + } + } +} + impl Transaction for tokio_postgres::Transaction<'_> { type Error = tokio_postgres::Error; @@ -26,16 +36,58 @@ impl Transaction for tokio_postgres::Transaction<'_> { } } +/// A [`TransactionBuilder`] which delegates to [`Client::build_transaction`]. +/// +/// All configured options are compiled into the single `BEGIN` statement issued when the builder +/// is awaited, e.g. `START TRANSACTION ISOLATION LEVEL REPEATABLE READ, READ ONLY`. +pub struct PostgresTransactionBuilder<'c> { + builder: tokio_postgres::TransactionBuilder<'c>, +} + +impl<'c> IntoFuture for PostgresTransactionBuilder<'c> { + type Output = Result, Report>; + + type IntoFuture = impl Future; + + fn into_future(self) -> Self::IntoFuture { + async move { + tracing::trace!("Starting transaction"); + self.builder.start().await.map_err(Report::new) + } + } +} + +impl<'c> TransactionBuilder for PostgresTransactionBuilder<'c> { + type Error = tokio_postgres::Error; + type Transaction = tokio_postgres::Transaction<'c>; + + fn isolation_level(mut self, isolation_level: IsolationLevel) -> Self { + self.builder = self.builder.isolation_level(isolation_level.into()); + self + } + + fn read_only(mut self) -> Self { + self.builder = self.builder.read_only(true); + self + } + + fn deferrable(mut self) -> Self { + self.builder = self.builder.deferrable(true); + self + } +} + impl Context for Client { type Error = tokio_postgres::Error; - type Transaction<'c> - = tokio_postgres::Transaction<'c> + type TransactionBuilder<'c> + = PostgresTransactionBuilder<'c> where Self: 'c; - async fn transaction(&mut self) -> Result, Report> { - tracing::trace!("Starting transaction"); - self.transaction().await.map_err(Report::new) + fn transaction(&mut self) -> Self::TransactionBuilder<'_> { + PostgresTransactionBuilder { + builder: self.build_transaction(), + } } } diff --git a/libs/@local/graph/postgres-store/Cargo.toml b/libs/@local/graph/postgres-store/Cargo.toml index f48e6bc8fed..63e08389d58 100644 --- a/libs/@local/graph/postgres-store/Cargo.toml +++ b/libs/@local/graph/postgres-store/Cargo.toml @@ -10,6 +10,7 @@ description = "HASH Graph API" [dependencies] # Public workspace dependencies hash-graph-authorization = { workspace = true, public = true, features = ["postgres"] } +hash-graph-migrations = { workspace = true, public = true } hash-graph-store = { workspace = true, public = true, features = ["postgres"] } hash-graph-validation = { workspace = true, public = true } hash-temporal-client = { workspace = true, public = true } @@ -53,16 +54,22 @@ utoipa = { workspace = true, optional = true, features = ["uuid"] } uuid = { workspace = true, features = ["v4", "serde"] } [dev-dependencies] -hash-graph-migrations = { workspace = true } -hash-graph-test-data = { workspace = true } -hash-telemetry = { workspace = true } -indoc = { workspace = true } -pretty_assertions = { workspace = true } -tokio = { workspace = true, features = ["macros"] } -tracing-subscriber = { workspace = true, features = ["env-filter"] } +# The crate's own test suites wrap each test in a rollback transaction, so they need the +# `test-utils`-gated `BeginReadOnlyTransaction` impl for `InTransaction` stores. The +# self-dependency enables the feature for test builds only via feature unification. +hash-graph-postgres-store = { workspace = true, features = ["test-utils"] } +hash-graph-test-data = { workspace = true } +hash-telemetry = { workspace = true } +indoc = { workspace = true } +pretty_assertions = { workspace = true } +tokio = { workspace = true, features = ["macros"] } +tracing-subscriber = { workspace = true, features = ["env-filter"] } [features] clap = ["dep:clap"] +# Exposes test-only impls, in particular `BeginReadOnlyTransaction` for stores already inside a +# transaction. See the `TODO(BE-688)` on the trait definition. +test-utils = [] utoipa = [ "dep:utoipa", "hash-graph-store/utoipa", @@ -73,3 +80,11 @@ utoipa = [ [lints] workspace = true + + +[package.metadata.sync.turborepo] +# The self-dependency above only exists to enable the `test-utils` feature for test builds; a +# literal self-reference in `package.json` would make Turborepo reject the package graph. +ignore-dev-dependencies = [ + "hash-graph-postgres-store", +] diff --git a/libs/@local/graph/postgres-store/docs/dependency-diagram.mmd b/libs/@local/graph/postgres-store/docs/dependency-diagram.mmd index eaa2eff809a..57a7eeece8d 100644 --- a/libs/@local/graph/postgres-store/docs/dependency-diagram.mmd +++ b/libs/@local/graph/postgres-store/docs/dependency-diagram.mmd @@ -55,7 +55,8 @@ graph TD 7 --> 8 7 --> 23 9 --> 6 - 9 -.-> 7 + 9 --> 7 + 9 -.-> 9 9 --> 13 9 --> 22 10 --> 5 diff --git a/libs/@local/graph/postgres-store/package.json b/libs/@local/graph/postgres-store/package.json index 2ae76eade07..ed705c68ae7 100644 --- a/libs/@local/graph/postgres-store/package.json +++ b/libs/@local/graph/postgres-store/package.json @@ -17,6 +17,7 @@ "@rust/hash-codec": "workspace:*", "@rust/hash-graph-authorization": "workspace:*", "@rust/hash-graph-embeddings": "workspace:*", + "@rust/hash-graph-migrations": "workspace:*", "@rust/hash-graph-store": "workspace:*", "@rust/hash-graph-temporal-versioning": "workspace:*", "@rust/hash-graph-types": "workspace:*", @@ -25,7 +26,6 @@ "@rust/hash-temporal-client": "workspace:*" }, "devDependencies": { - "@rust/hash-graph-migrations": "workspace:*", "@rust/hash-graph-test-data": "workspace:*", "@rust/hash-telemetry": "workspace:*" } diff --git a/libs/@local/graph/postgres-store/src/permissions/mod.rs b/libs/@local/graph/postgres-store/src/permissions/mod.rs index b349a357f15..b2e9d346cc6 100644 --- a/libs/@local/graph/postgres-store/src/permissions/mod.rs +++ b/libs/@local/graph/postgres-store/src/permissions/mod.rs @@ -7,6 +7,7 @@ use hash_graph_authorization::policies::{ action::ActionName, store::{RoleAssignmentStatus, RoleUnassignmentStatus}, }; +use hash_graph_migrations::Transaction as _; use hash_graph_store::account::{AccountStore as _, GetActorError}; use tokio_postgres::{GenericClient as _, error::SqlState}; use tracing::Instrument as _; @@ -18,14 +19,15 @@ use type_system::principal::{ }; use uuid::Uuid; -use crate::store::{AsClient, PostgresStore}; +use crate::store::{AsClient, PostgresStore, TransactionState}; mod error; pub use self::error::{ActionError, PolicyError, PrincipalError}; -impl PostgresStore +impl PostgresStore where C: AsClient, + S: TransactionState, { async fn is_principal( &self, @@ -939,7 +941,7 @@ where /// [`StoreError`]: ActionError::StoreError pub async fn register_action(&mut self, action: ActionName) -> Result<(), Report> { let transaction = self - .transaction() + .begin_transaction() .await .change_context(ActionError::StoreError)?; diff --git a/libs/@local/graph/postgres-store/src/snapshot/action/batch.rs b/libs/@local/graph/postgres-store/src/snapshot/action/batch.rs index f61edefa2a7..df237cb3b85 100644 --- a/libs/@local/graph/postgres-store/src/snapshot/action/batch.rs +++ b/libs/@local/graph/postgres-store/src/snapshot/action/batch.rs @@ -6,7 +6,7 @@ use tracing::Instrument as _; use super::table::{ActionHierarchyRow, ActionRow}; use crate::{ snapshot::{SnapshotInsertOptions, WriteBatch, insert_rows_batch}, - store::{AsClient, PostgresStore, postgres::query::OnConflict}, + store::{AsClient, InTransaction, PostgresStore, postgres::query::OnConflict}, }; pub enum ActionRowBatch { @@ -18,7 +18,9 @@ impl WriteBatch for ActionRowBatch where C: AsClient, { - async fn begin(postgres_client: &mut PostgresStore) -> Result<(), Report> { + async fn begin( + postgres_client: &mut PostgresStore, + ) -> Result<(), Report> { postgres_client .as_client() .client() @@ -47,7 +49,7 @@ where async fn write( self, - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, ) -> Result<(), Report> { let client = postgres_client.as_client().client(); match self { @@ -84,7 +86,7 @@ where } async fn commit( - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, _ignore_validation_errors: bool, ) -> Result<(), Report> { postgres_client diff --git a/libs/@local/graph/postgres-store/src/snapshot/entity/batch.rs b/libs/@local/graph/postgres-store/src/snapshot/entity/batch.rs index 530a996402e..280facff88e 100644 --- a/libs/@local/graph/postgres-store/src/snapshot/entity/batch.rs +++ b/libs/@local/graph/postgres-store/src/snapshot/entity/batch.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use error_stack::{Report, ResultExt as _, ensure}; use futures::{StreamExt as _, TryStreamExt as _, stream}; use hash_graph_store::{ - entity::{EntityStore as _, EntityValidationReport, ValidateEntityComponents}, + entity::{EntityValidationReport, ValidateEntityComponents}, error::InsertionError, query::Read, }; @@ -23,7 +23,7 @@ use crate::{ store::{ StoreCache, StoreProvider, postgres::{ - AsClient, PostgresStore, + AsClient, InTransaction, PostgresStore, query::{ OnConflict, rows::{ @@ -49,7 +49,9 @@ impl WriteBatch for EntityRowBatch where C: AsClient, { - async fn begin(postgres_client: &mut PostgresStore) -> Result<(), Report> { + async fn begin( + postgres_client: &mut PostgresStore, + ) -> Result<(), Report> { postgres_client .as_client() .client() @@ -99,7 +101,7 @@ where #[expect(clippy::too_many_lines)] async fn write( self, - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, ) -> Result<(), Report> { let client = postgres_client.as_client().client(); match self { @@ -207,7 +209,7 @@ where #[expect(clippy::too_many_lines)] async fn commit( - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, ignore_validation_errors: bool, ) -> Result<(), Report> { postgres_client @@ -247,7 +249,7 @@ where .change_context(InsertionError)?; postgres_client - .reindex_entity_cache() + .reindex_entity_cache_impl() .await .change_context(InsertionError)?; @@ -385,7 +387,7 @@ where // values written above would otherwise leave stale label entries behind. if !edition_ids_updates.is_empty() { postgres_client - .reindex_entity_cache() + .reindex_entity_cache_impl() .await .change_context(InsertionError)?; } diff --git a/libs/@local/graph/postgres-store/src/snapshot/mod.rs b/libs/@local/graph/postgres-store/src/snapshot/mod.rs index 20baf771fdb..e09e55ff74f 100644 --- a/libs/@local/graph/postgres-store/src/snapshot/mod.rs +++ b/libs/@local/graph/postgres-store/src/snapshot/mod.rs @@ -30,6 +30,7 @@ use futures::{ Sink, SinkExt as _, Stream, StreamExt as _, TryFutureExt as _, TryStreamExt as _, channel::mpsc, stream, }; +use hash_graph_migrations::Transaction as _; use hash_graph_store::{error::InsertionError, filter::QueryRecord, pool::StorePool, query::Read}; use hash_status::StatusCode; use postgres_types::{Json, ToSql}; @@ -57,7 +58,7 @@ use uuid::Uuid; use crate::{ snapshot::{entity::EntityEmbeddingRecord, restore::SnapshotRecordBatch}, store::postgres::{ - AsClient, PolicyParts, PostgresStore, PostgresStorePool, + AsClient, InTransaction, PolicyParts, PostgresStore, PostgresStorePool, query::{OnConflict, TableName, bulk_insert, rows::PostgresRow}, }, }; @@ -255,14 +256,14 @@ impl SnapshotEntry { trait WriteBatch { fn begin( - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, ) -> impl Future>> + Send; fn write( self, - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, ) -> impl Future>> + Send; fn commit( - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, ignore_validation_errors: bool, ) -> impl Future>> + Send; } @@ -1172,7 +1173,7 @@ where let mut client = self .0 - .transaction() + .begin_transaction() .await .change_context(SnapshotRestoreError::Write)?; diff --git a/libs/@local/graph/postgres-store/src/snapshot/ontology/data_type/batch.rs b/libs/@local/graph/postgres-store/src/snapshot/ontology/data_type/batch.rs index 895508c6b61..61da7b31a04 100644 --- a/libs/@local/graph/postgres-store/src/snapshot/ontology/data_type/batch.rs +++ b/libs/@local/graph/postgres-store/src/snapshot/ontology/data_type/batch.rs @@ -6,7 +6,7 @@ use tracing::Instrument as _; use crate::{ snapshot::{SnapshotInsertOptions, WriteBatch, insert_rows_batch}, store::{ - AsClient, PostgresStore, + AsClient, InTransaction, PostgresStore, postgres::query::{ OnConflict, rows::{DataTypeConversionsRow, DataTypeEmbeddingRow, DataTypeRow}, @@ -24,7 +24,9 @@ impl WriteBatch for DataTypeRowBatch where C: AsClient, { - async fn begin(postgres_client: &mut PostgresStore) -> Result<(), Report> { + async fn begin( + postgres_client: &mut PostgresStore, + ) -> Result<(), Report> { postgres_client .as_client() .client() @@ -57,7 +59,7 @@ where async fn write( self, - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, ) -> Result<(), Report> { let client = postgres_client.as_client().client(); match self { @@ -108,7 +110,7 @@ where } async fn commit( - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, _ignore_validation_errors: bool, ) -> Result<(), Report> { postgres_client diff --git a/libs/@local/graph/postgres-store/src/snapshot/ontology/entity_type/batch.rs b/libs/@local/graph/postgres-store/src/snapshot/ontology/entity_type/batch.rs index 221198fbe09..49d1d978f43 100644 --- a/libs/@local/graph/postgres-store/src/snapshot/ontology/entity_type/batch.rs +++ b/libs/@local/graph/postgres-store/src/snapshot/ontology/entity_type/batch.rs @@ -6,7 +6,7 @@ use tracing::Instrument as _; use crate::{ snapshot::{SnapshotInsertOptions, WriteBatch, insert_rows_batch}, store::{ - AsClient, PostgresStore, + AsClient, InTransaction, PostgresStore, postgres::query::{ OnConflict, rows::{EntityTypeEmbeddingRow, EntityTypeRow}, @@ -23,7 +23,9 @@ impl WriteBatch for EntityTypeRowBatch where C: AsClient, { - async fn begin(postgres_client: &mut PostgresStore) -> Result<(), Report> { + async fn begin( + postgres_client: &mut PostgresStore, + ) -> Result<(), Report> { postgres_client .as_client() .client() @@ -52,7 +54,7 @@ where async fn write( self, - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, ) -> Result<(), Report> { let client = postgres_client.as_client().client(); match self { @@ -89,7 +91,7 @@ where } async fn commit( - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, _ignore_validation_errors: bool, ) -> Result<(), Report> { postgres_client diff --git a/libs/@local/graph/postgres-store/src/snapshot/ontology/metadata/batch.rs b/libs/@local/graph/postgres-store/src/snapshot/ontology/metadata/batch.rs index 5d6e5201e00..2885fc2e065 100644 --- a/libs/@local/graph/postgres-store/src/snapshot/ontology/metadata/batch.rs +++ b/libs/@local/graph/postgres-store/src/snapshot/ontology/metadata/batch.rs @@ -6,7 +6,7 @@ use tracing::Instrument as _; use crate::{ snapshot::{SnapshotInsertOptions, WriteBatch, insert_rows_batch}, store::{ - AsClient, PostgresStore, + AsClient, InTransaction, PostgresStore, postgres::query::{ OnConflict, rows::{ @@ -28,7 +28,9 @@ impl WriteBatch for OntologyTypeMetadataRowBatch where C: AsClient, { - async fn begin(postgres_client: &mut PostgresStore) -> Result<(), Report> { + async fn begin( + postgres_client: &mut PostgresStore, + ) -> Result<(), Report> { postgres_client .as_client() .client() @@ -65,7 +67,7 @@ where async fn write( self, - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, ) -> Result<(), Report> { let client = postgres_client.as_client().client(); match self { @@ -130,7 +132,7 @@ where } async fn commit( - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, _ignore_validation_errors: bool, ) -> Result<(), Report> { postgres_client diff --git a/libs/@local/graph/postgres-store/src/snapshot/ontology/property_type/batch.rs b/libs/@local/graph/postgres-store/src/snapshot/ontology/property_type/batch.rs index 8dbe2e22732..029a1dd9f2b 100644 --- a/libs/@local/graph/postgres-store/src/snapshot/ontology/property_type/batch.rs +++ b/libs/@local/graph/postgres-store/src/snapshot/ontology/property_type/batch.rs @@ -6,7 +6,7 @@ use tracing::Instrument as _; use crate::{ snapshot::{SnapshotInsertOptions, WriteBatch, insert_rows_batch}, store::{ - AsClient, PostgresStore, + AsClient, InTransaction, PostgresStore, postgres::query::{ OnConflict, rows::{ @@ -28,7 +28,9 @@ impl WriteBatch for PropertyTypeRowBatch where C: AsClient, { - async fn begin(postgres_client: &mut PostgresStore) -> Result<(), Report> { + async fn begin( + postgres_client: &mut PostgresStore, + ) -> Result<(), Report> { postgres_client .as_client() .client() @@ -65,7 +67,7 @@ where async fn write( self, - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, ) -> Result<(), Report> { let client = postgres_client.as_client().client(); match self { @@ -130,7 +132,7 @@ where } async fn commit( - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, _ignore_validation_errors: bool, ) -> Result<(), Report> { postgres_client diff --git a/libs/@local/graph/postgres-store/src/snapshot/policy/batch.rs b/libs/@local/graph/postgres-store/src/snapshot/policy/batch.rs index b14d95e4645..c1edcbddf39 100644 --- a/libs/@local/graph/postgres-store/src/snapshot/policy/batch.rs +++ b/libs/@local/graph/postgres-store/src/snapshot/policy/batch.rs @@ -6,7 +6,7 @@ use tracing::Instrument as _; use super::table::{PolicyActionRow, PolicyEditionRow, PolicyRow}; use crate::{ snapshot::{SnapshotInsertOptions, WriteBatch, insert_rows_batch}, - store::{AsClient, PostgresStore, postgres::query::OnConflict}, + store::{AsClient, InTransaction, PostgresStore, postgres::query::OnConflict}, }; pub enum PolicyRowBatch { @@ -19,7 +19,9 @@ impl WriteBatch for PolicyRowBatch where C: AsClient, { - async fn begin(postgres_client: &mut PostgresStore) -> Result<(), Report> { + async fn begin( + postgres_client: &mut PostgresStore, + ) -> Result<(), Report> { postgres_client .as_client() .client() @@ -52,7 +54,7 @@ where async fn write( self, - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, ) -> Result<(), Report> { let client = postgres_client.as_client().client(); match self { @@ -103,7 +105,7 @@ where } async fn commit( - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, _ignore_validation_errors: bool, ) -> Result<(), Report> { postgres_client diff --git a/libs/@local/graph/postgres-store/src/snapshot/principal/batch.rs b/libs/@local/graph/postgres-store/src/snapshot/principal/batch.rs index 86e09b670d8..a3b8470ea37 100644 --- a/libs/@local/graph/postgres-store/src/snapshot/principal/batch.rs +++ b/libs/@local/graph/postgres-store/src/snapshot/principal/batch.rs @@ -8,7 +8,7 @@ use super::table::{ }; use crate::{ snapshot::{SnapshotInsertOptions, WriteBatch, insert_rows_batch}, - store::{AsClient, PostgresStore, postgres::query::OnConflict}, + store::{AsClient, InTransaction, PostgresStore, postgres::query::OnConflict}, }; pub enum PrincipalRowBatch { @@ -25,7 +25,9 @@ impl WriteBatch for PrincipalRowBatch where C: AsClient, { - async fn begin(postgres_client: &mut PostgresStore) -> Result<(), Report> { + async fn begin( + postgres_client: &mut PostgresStore, + ) -> Result<(), Report> { postgres_client .as_client() .client() @@ -71,7 +73,7 @@ where #[expect(clippy::too_many_lines)] async fn write( self, - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, ) -> Result<(), Report> { let client = postgres_client.as_client().client(); match self { @@ -178,7 +180,7 @@ where } async fn commit( - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, _ignore_validation_errors: bool, ) -> Result<(), Report> { postgres_client diff --git a/libs/@local/graph/postgres-store/src/snapshot/restore/batch.rs b/libs/@local/graph/postgres-store/src/snapshot/restore/batch.rs index 2aa07407e5b..3d6f061e1e2 100644 --- a/libs/@local/graph/postgres-store/src/snapshot/restore/batch.rs +++ b/libs/@local/graph/postgres-store/src/snapshot/restore/batch.rs @@ -13,7 +13,7 @@ use crate::{ policy::PolicyRowBatch, principal::PrincipalRowBatch, }, - store::{AsClient, PostgresStore}, + store::{AsClient, InTransaction, PostgresStore}, }; pub enum SnapshotRecordBatch { @@ -31,7 +31,9 @@ impl WriteBatch for SnapshotRecordBatch where C: AsClient, { - async fn begin(postgres_client: &mut PostgresStore) -> Result<(), Report> { + async fn begin( + postgres_client: &mut PostgresStore, + ) -> Result<(), Report> { PrincipalRowBatch::begin(postgres_client).await?; OntologyTypeMetadataRowBatch::begin(postgres_client).await?; DataTypeRowBatch::begin(postgres_client).await?; @@ -45,7 +47,7 @@ where async fn write( self, - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, ) -> Result<(), Report> { match self { Self::Principals(principals) => principals.write(postgres_client).await, @@ -60,7 +62,7 @@ where } async fn commit( - postgres_client: &mut PostgresStore, + postgres_client: &mut PostgresStore, ignore_validation_errors: bool, ) -> Result<(), Report> { PrincipalRowBatch::commit(postgres_client, ignore_validation_errors).await?; diff --git a/libs/@local/graph/postgres-store/src/store/mod.rs b/libs/@local/graph/postgres-store/src/store/mod.rs index c7c32581581..b90ee497b15 100644 --- a/libs/@local/graph/postgres-store/src/store/mod.rs +++ b/libs/@local/graph/postgres-store/src/store/mod.rs @@ -7,6 +7,10 @@ pub mod postgres; pub use self::{ config::{DatabaseConnectionInfo, DatabasePoolConfig, DatabaseType}, - postgres::{AsClient, PostgresStore, PostgresStorePool, PostgresStoreSettings}, + postgres::{ + AsClient, BeginReadOnlyTransaction, Context, InTransaction, IsolationLevel, NoTransaction, + PostgresStore, PostgresStorePool, PostgresStoreSettings, PostgresStoreTransactionBuilder, + Transaction, TransactionBuilder, TransactionOptions, TransactionState, + }, validation::{StoreCache, StoreProvider}, }; diff --git a/libs/@local/graph/postgres-store/src/store/postgres/crud.rs b/libs/@local/graph/postgres-store/src/store/postgres/crud.rs index da3516f80f8..b976a058127 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/crud.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/crud.rs @@ -12,7 +12,7 @@ use tokio_postgres::{GenericClient as _, Row}; use tracing::Instrument as _; use crate::store::{ - AsClient, PostgresStore, + AsClient, PostgresStore, TransactionState, postgres::query::{PostgresQueryPath, PostgresRecord, PostgresSorting, SelectCompiler}, }; @@ -61,9 +61,10 @@ where } } -impl ReadPaginated for PostgresStore +impl ReadPaginated for PostgresStore where Cl: AsClient, + St: TransactionState, for<'c> R: PostgresRecord: PostgresQueryPath>, for<'s> S: PostgresSorting<'s, R> + Sync, S::Cursor: Send, @@ -134,9 +135,10 @@ where } } -impl Read for PostgresStore +impl Read for PostgresStore where Cl: AsClient, + St: TransactionState, for<'c> R: PostgresRecord: PostgresQueryPath>, { type ReadStream = impl Stream>> + Send + Sync; diff --git a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/delete.rs b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/delete.rs index bc0cbc76074..ae48d1c63ad 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/delete.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/delete.rs @@ -29,7 +29,7 @@ use type_system::{ }; use crate::store::{ - AsClient as _, PostgresStore, + AsClient as _, InTransaction, PostgresStore, postgres::query::{Distinctness, SelectCompiler}, }; @@ -81,7 +81,7 @@ enum DeletionTarget<'a> { /// Without a transaction these locks would be released immediately, defeating the purpose. /// /// [`patch_entity`]: hash_graph_store::entity::EntityStore::patch_entity -impl PostgresStore> { +impl PostgresStore, InTransaction> { /// Finds entities matching `filter` and partitions them into full vs draft-only deletions. /// /// A published match (or a match that subsumes all drafts of a draft-only entity) produces diff --git a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs index fb9b7931884..1c7213bb55c 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/mod.rs @@ -18,6 +18,7 @@ use hash_graph_authorization::policies::{ store::{PolicyCreationParams, PrincipalStore as _}, }; use hash_graph_embeddings::{Dimension, clustering::Clustering}; +use hash_graph_migrations::Transaction as _; use hash_graph_store::{ entity::{ ClusterEntitiesParams, ClusterEntitiesResponse, CreateEntityParams, DeleteEntitiesParams, @@ -98,7 +99,7 @@ use crate::store::{ AsClient, PostgresStore, error::{EntityDoesNotExist, RaceConditionOnUpdate}, postgres::{ - TraversalContext, + BeginReadOnlyTransaction, InTransaction, TransactionState, TraversalContext, crud::{QueryIndices, TypedRow}, knowledge::entity::{ read::EntityEdgeTraversalData, @@ -180,9 +181,10 @@ impl fmt::Display for JoinError { impl core::error::Error for JoinError {} -impl PostgresStore +impl PostgresStore where C: AsClient, + S: TransactionState, { /// Resolves `is-of-type` edges from entities to their entity types. /// @@ -553,7 +555,7 @@ where #[tracing::instrument(level = "info", skip_all)] #[expect(clippy::too_many_lines)] - async fn query_entities_impl( + async fn read_entities_impl( &self, params: &QueryEntitiesParams<'_>, temporal_axes: &QueryTemporalAxes, @@ -707,218 +709,677 @@ where } } -impl EntityStore for PostgresStore +/// Read implementations which run inside the snapshot-consistent read transaction begun by +/// [`BeginReadOnlyTransaction::begin_read_only_transaction`]. +impl PostgresStore where C: AsClient, { #[tracing::instrument(level = "info", skip(self, params))] - #[expect(clippy::too_many_lines)] - async fn create_entities( - &mut self, - actor_uuid: ActorEntityUuid, - params: Vec, - ) -> Result, Report> { - let transaction_time = Timestamp::::now().remove_nanosecond(); - let mut entity_edition_ids = Vec::with_capacity(params.len()); - - let mut entity_id_rows = Vec::with_capacity(params.len()); - let mut entity_draft_rows = Vec::new(); - let mut entity_edition_rows = Vec::with_capacity(params.len()); - let mut entity_temporal_metadata_rows = Vec::with_capacity(params.len()); - let mut entity_is_of_type_rows = Vec::with_capacity(params.len()); - let mut entity_edge_rows = Vec::new(); - - let mut policies = Vec::new(); - - let mut entities = Vec::with_capacity(params.len()); - // TODO: There are expected to be duplicates but we currently don't have a way to identify - // multi-type entity types. We need a way to speed this up. - let mut validation_params = Vec::with_capacity(params.len()); + async fn query_entities_impl( + &self, + actor_id: ActorEntityUuid, + mut params: QueryEntitiesParams<'_>, + ) -> Result, Report> { + let policy_components = PolicyComponents::builder(self) + .with_actor(actor_id) + .with_action(ActionName::ViewEntity, MergePolicies::Yes) + .await + .change_context(QueryError)?; - let transaction = self.transaction().await.change_context(InsertionError)?; + let provider = StoreProvider::new(self, &policy_components); - let actor_id = transaction - .determine_actor(actor_uuid) + params + .filter + .convert_parameters(&provider) .await - .change_context(InsertionError)? - .ok_or_else(|| Report::new(InsertionError).attach("Actor not found"))?; - - let mut policy_components_builder = PolicyComponents::builder(&transaction); + .change_context(QueryError)?; - let mut entity_ids = Vec::with_capacity(params.len()); + let temporal_axes = params.temporal_axes.resolve(); - // We will use the added entity type IDs to check for the instantiation permission later. - // This means that we need to make sure, that exactly the required entity types are passed - // here. - let mut entity_type_id_set = HashSet::with_capacity(params.len()); - for params in ¶ms { - let entity_id = EntityId { - web_id: params.web_id, - entity_uuid: params - .entity_uuid - .unwrap_or_else(|| EntityUuid::new(Uuid::new_v4())), - draft_id: params.draft.then(|| DraftId::new(Uuid::new_v4())), - }; - policy_components_builder.add_entity( - actor_id, - entity_id, - Cow::Owned(params.entity_type_ids.iter().cloned().collect()), - ); - entity_ids.push(entity_id); + let mut response = self + .read_entities_impl(¶ms, &temporal_axes, &policy_components) + .await?; - entity_type_id_set.extend(¶ms.entity_type_ids); + if !params.conversions.is_empty() { + for entity in &mut response.entities { + self.convert_entity(&provider, entity, ¶ms.conversions) + .await + .change_context(QueryError)?; + } } - // The policy components builder will make sure, that also parent entity types are added to - // the set of entity type IDs. These are accessible via `tracked_entity_types` method. - let policy_components = policy_components_builder - .with_actor(actor_id) - .with_entity_type_ids(entity_type_id_set) - .with_actions( - [ActionName::Instantiate, ActionName::CreateEntity], - MergePolicies::No, - ) - .with_actions( - [ - ActionName::ViewEntity, - ActionName::ViewEntityType, - ActionName::ViewPropertyType, - ActionName::ViewDataType, - ], - MergePolicies::Yes, - ) - .await - .change_context(InsertionError)?; - - let policy_set = policy_components - .build_policy_set([ActionName::Instantiate, ActionName::CreateEntity]) - .change_context(InsertionError)?; + if params.include_permissions { + let entity_ids = response + .entities + .iter() + .map(|entity| entity.metadata.record_id.entity_id) + .collect::>(); - let mut forbidden_instantiations = Vec::new(); - for entity_type_id in policy_components.tracked_entity_types() { - match policy_set - .evaluate( - &Request { - actor: policy_components.actor_id(), - action: ActionName::Instantiate, - resource: &ResourceId::EntityType(Cow::Borrowed(entity_type_id.into())), - context: RequestContext::default(), + let update_permissions = self + .has_permission_for_entities_impl( + policy_components.actor_id().into(), + HasPermissionForEntitiesParams { + action: ActionName::UpdateEntity, + entity_ids: Cow::Borrowed(&entity_ids), + temporal_axes: params.temporal_axes, + include_drafts: params.include_drafts, }, - policy_components.context(), ) - .change_context(InsertionError)? - { - Authorized::Always => {} - Authorized::Never => { - forbidden_instantiations.push(entity_type_id); - } - } - } + .await + .change_context(QueryError)?; - if !forbidden_instantiations.is_empty() { - return Err(Report::new(InsertionError) - .attach_opaque(StatusCode::PermissionDenied) - .attach( - "The actor does not have permission to instantiate one or more entity types", - ) - .attach( - forbidden_instantiations - .into_iter() - .map(ToString::to_string) - .map(Cow::Owned) - .intersperse(Cow::Borrowed(", ")) - .collect::(), - )); - } + let mut permissions: HashMap = + HashMap::with_capacity(update_permissions.len()); - let mut forbidden_entity_creations = Vec::new(); - for entity_id in &entity_ids { - match policy_set - .evaluate( - &Request { - actor: policy_components.actor_id(), - action: ActionName::CreateEntity, - resource: &ResourceId::Entity(entity_id.entity_uuid), - context: RequestContext::default(), - }, - policy_components.context(), - ) - .change_context(InsertionError)? - { - Authorized::Always => {} - Authorized::Never => { - forbidden_entity_creations.push(entity_id); - } + for (entity_id, editions) in update_permissions { + permissions.entry(entity_id).or_default().update = editions; } - } - if !forbidden_entity_creations.is_empty() { - return Err(Report::new(InsertionError) - .attach_opaque(StatusCode::PermissionDenied) - .attach("The actor does not have permission to create one or more entities") - .attach( - forbidden_entity_creations - .into_iter() - .map(ToString::to_string) - .collect::>() - .join(", "), - )); + debug_assert!( + response.permissions.is_none(), + "Should not be populated yet" + ); + response.permissions = Some(permissions); } - let validator_provider = StoreProvider::new(&transaction, &policy_components); - - let mut validation_reports = HashMap::::new(); + Ok(response) + } - debug_assert_eq!( - params.len(), - entity_ids.len(), - "Number of parameters ({}) and entity ids ({}) must be the same", - params.len(), - entity_ids.len(), - ); - for (index, (mut params, &entity_id)) in params.into_iter().zip(&entity_ids).enumerate() { - let entity_type = ClosedMultiEntityType::from_multi_type_closed_schema( - stream::iter(¶ms.entity_type_ids) - .then(|entity_type_url| async { - OntologyTypeProvider::::provide_type( - &validator_provider, - entity_type_url, - ) - .await - .map(|entity_type| ClosedEntityType::clone(&*entity_type)) - }) - .try_collect::>() - .await - .change_context(InsertionError)?, - ) - .change_context(InsertionError)?; + #[tracing::instrument(level = "info", skip(self, params))] + #[expect(clippy::too_many_lines)] + async fn query_entity_subgraph_impl( + &self, + actor_id: ActorEntityUuid, + params: QueryEntitySubgraphParams<'_>, + ) -> Result, Report> { + let actions = params.view_actions(); - let mut preprocessor = EntityPreprocessor { - components: if params.draft { - ValidateEntityComponents::draft() - } else { - ValidateEntityComponents::full() - }, - convert_values: true, - }; - preprocessor.components.link_validation = transaction.settings.validate_links; + let policy_components = PolicyComponents::builder(self) + .with_actor(actor_id) + .with_actions(actions, MergePolicies::Yes) + .await + .change_context(QueryError)?; + let actor = policy_components.actor_id(); - if let Err(property_validation) = preprocessor - .visit_object(&entity_type, &mut params.properties, &validator_provider) - .await - { - validation_reports.entry(index).or_default().properties = - property_validation.properties; - } + let provider = StoreProvider::new(self, &policy_components); - let (properties, property_metadata) = params.properties.into_parts(); + let (mut request, traversal_params) = params.into_parts(); + request + .filter + .convert_parameters(&provider) + .await + .change_context(QueryError)?; - let decision_time = params - .decision_time - .map_or_else(|| transaction_time.cast(), Timestamp::remove_nanosecond); + let temporal_axes = request.temporal_axes.resolve(); + let time_axis = temporal_axes.variable_time_axis(); - let entity_provenance = EntityProvenance { - inferred: InferredEntityProvenance { + let QueryEntitiesResponse { + entities: root_entities, + cursor, + closed_multi_entity_types: _, + definitions: _, + permissions, + } = self + .read_entities_impl(&request, &temporal_axes, &policy_components) + .await?; + + let mut subgraph = Subgraph::new(request.temporal_axes, temporal_axes); + + async move { + subgraph.roots.extend( + root_entities + .iter() + .map(|entity| entity.vertex_id(time_axis).into()), + ); + subgraph.vertices.entities = root_entities + .into_iter() + .map(|entity| (entity.vertex_id(time_axis), entity)) + .collect(); + + let mut traversal_context = TraversalContext::default(); + + // Iterate over each traversal path and call traverse_entities separately + match &traversal_params { + SubgraphTraversalParams::Paths { traversal_paths } => { + for path in traversal_paths { + let (entity_traversal_path, ontology_traversal_path) = + path.split_entity_path(); + self.traverse_entities_with_path( + &entity_traversal_path, + ontology_traversal_path, + &mut traversal_context, + &provider, + &mut subgraph, + ) + .await?; + } + } + SubgraphTraversalParams::ResolveDepths { + traversal_paths, + graph_resolve_depths, + } => { + if traversal_paths.is_empty() { + if graph_resolve_depths.is_of_type { + // If no entity traversal paths are specified, still initialize + // the traversal with ontology resolve depths to enable + // traversal of ontology edges (e.g., isOfType, inheritsFrom) + self.traverse_entities_with_resolve_depths( + &[], + *graph_resolve_depths, + &mut traversal_context, + &provider, + &mut subgraph, + ) + .await?; + } + } else { + for path in traversal_paths { + self.traverse_entities_with_resolve_depths( + &path.edges, + *graph_resolve_depths, + &mut traversal_context, + &provider, + &mut subgraph, + ) + .await?; + } + } + } + } + + traversal_context + .read_traversed_vertices( + self, + &mut subgraph, + request.include_drafts, + provider + .policy_components + .as_ref() + .expect("Policy components should be set"), + ) + .await?; + + if !request.conversions.is_empty() { + for entity in subgraph.vertices.entities.values_mut() { + self.convert_entity(&provider, entity, &request.conversions) + .await + .change_context(QueryError)?; + } + } + + Ok(QueryEntitySubgraphResponse { + closed_multi_entity_types: if request.include_entity_types.is_some() { + Some( + self.get_closed_multi_entity_types( + actor_id, + subgraph + .vertices + .entities + .values() + .map(|entity| entity.metadata.entity_type_ids.clone()), + QueryTemporalAxesUnresolved::live_only(), + None, + ) + .await? + .entity_types, + ) + } else { + None + }, + definitions: match request.include_entity_types { + Some( + IncludeEntityTypeOption::Resolved + | IncludeEntityTypeOption::ResolvedWithDataTypeChildren, + ) => { + let entity_type_uuids = subgraph + .vertices + .entities + .values() + .flat_map(|entity| { + entity + .metadata + .entity_type_ids + .iter() + .map(EntityTypeUuid::from_url) + }) + .collect::>() + .into_iter() + .collect::>(); + Some( + self.get_entity_type_resolve_definitions( + actor_id, + &entity_type_uuids, + request.include_entity_types + == Some(IncludeEntityTypeOption::ResolvedWithDataTypeChildren), + ) + .await?, + ) + } + None | Some(IncludeEntityTypeOption::Closed) => None, + }, + cursor, + entity_permissions: if request.include_permissions { + debug_assert!(permissions.is_none(), "Should not be populated yet"); + + let entity_ids = subgraph + .vertices + .entities + .keys() + .map(|vertex_id| vertex_id.base_id) + .collect::>(); + + let update_permissions = self + .has_permission_for_entities_impl( + actor.into(), + HasPermissionForEntitiesParams { + action: ActionName::UpdateEntity, + entity_ids: Cow::Borrowed(&entity_ids), + temporal_axes: request.temporal_axes, + include_drafts: request.include_drafts, + }, + ) + .await + .change_context(QueryError)?; + + let mut permissions: HashMap = + HashMap::with_capacity(update_permissions.len()); + + for (entity_id, editions) in update_permissions { + permissions.entry(entity_id).or_default().update = editions; + } + + Some(permissions) + } else { + None + }, + subgraph, + }) + } + .instrument(tracing::trace_span!("construct_subgraph")) + .await + } +} + +impl PostgresStore +where + C: AsClient, + S: TransactionState, +{ + /// Rebuilds the entity edition cache and the inherited `entity_is_of_type` rows. + /// + /// This is inherent rather than only an [`EntityStore`] method so that code paths already + /// operating inside an enclosing transaction — snapshot restore and entity-type reindexing — + /// can rebuild the cache without requiring the [`EntityStore`] impl, whose snapshot-consistent + /// reads are only available where [`BeginReadOnlyTransaction`] is implemented. + /// + /// # Errors + /// + /// - if the database rejects one of the rebuild statements + #[tracing::instrument(level = "info", skip(self))] + pub(crate) async fn reindex_entity_cache_impl(&mut self) -> Result<(), Report> { + tracing::info!("Reindexing entity cache"); + let transaction = self.begin_transaction().await.change_context(UpdateError)?; + + // We remove the data from the reference tables first + transaction + .as_client() + .simple_query( + " + DELETE FROM entity_is_of_type WHERE inheritance_depth > 0; + + INSERT INTO entity_is_of_type ( + entity_edition_id, + entity_type_ontology_id, + inheritance_depth + ) + SELECT entity_edition_id, + target_entity_type_ontology_id AS entity_type_ontology_id, + MIN(entity_type_inherits_from.depth + 1) AS inheritance_depth + FROM entity_is_of_type + JOIN entity_type_inherits_from + ON entity_type_ontology_id = source_entity_type_ontology_id + GROUP BY entity_edition_id, target_entity_type_ontology_id; + + DELETE FROM entity_edition_cache; + ", + ) + .instrument(tracing::info_span!( + "INSERT", + otel.kind = "client", + db.system = "postgresql", + peer.service = "Postgres", + )) + .await + .change_context(UpdateError)?; + + transaction + .as_client() + .query(&insert_entity_edition_cache_statement(false), &[]) + .instrument(tracing::info_span!( + "INSERT", + otel.kind = "client", + db.system = "postgresql", + peer.service = "Postgres", + )) + .await + .change_context(UpdateError)?; + + transaction.commit().await.change_context(UpdateError)?; + + Ok(()) + } + + /// Returns the entity editions among `params.entity_ids` on which `authenticated_actor` may + /// perform `params.action`. + /// + /// This is inherent rather than only an [`EntityStore`] method because the snapshot-consistent + /// read implementations invoke it on the [`InTransaction`] store, where the [`EntityStore`] + /// impl — bounded on [`BeginReadOnlyTransaction`] — is not available. + /// + /// # Errors + /// + /// - if the permission filter cannot be compiled or the underlying query fails + #[tracing::instrument(skip(self, params))] + pub(crate) async fn has_permission_for_entities_impl( + &self, + authenticated_actor: AuthenticatedActor, + params: HasPermissionForEntitiesParams<'_>, + ) -> Result>, Report> { + let temporal_axes = params.temporal_axes.resolve(); + let mut compiler = SelectCompiler::new(Some(&temporal_axes), params.include_drafts); + + let entity_uuids = params + .entity_ids + .iter() + .map(|id| id.entity_uuid) + .collect::>(); + + let entity_filter = Filter::In( + FilterExpression::Path { + path: EntityQueryPath::Uuid, + }, + FilterExpressionList::ParameterList { + parameters: ParameterList::EntityUuids(&entity_uuids), + }, + ); + compiler + .add_filter(&entity_filter) + .change_context(CheckPermissionError::CompileFilter)?; + + let policy_components = PolicyComponents::builder(self) + .with_actor(authenticated_actor) + .with_action(params.action, MergePolicies::Yes) + .await + .change_context(CheckPermissionError::BuildPolicyContext)?; + let policy_filter = Filter::::for_policies( + policy_components.extract_filter_policies(params.action), + policy_components.actor_id(), + policy_components.optimization_data(params.action), + ); + compiler + .add_filter(&policy_filter) + .change_context(CheckPermissionError::CompileFilter)?; + + let web_id_idx = compiler.add_selection_path(&EntityQueryPath::WebId); + let uuid_idx = compiler.add_selection_path(&EntityQueryPath::Uuid); + let draft_id_idx = compiler.add_selection_path(&EntityQueryPath::DraftId); + let edition_id_idx = compiler.add_distinct_selection_with_ordering( + &EntityQueryPath::EditionId, + Distinctness::Distinct, + None, + ); + + let mut permitted_ids = HashMap::>::new(); + + let (statement, parameters) = compiler.compile(); + let () = self + .as_client() + .query_raw(&statement, parameters.iter().copied()) + .instrument(tracing::info_span!( + "SELECT", + otel.kind = "client", + db.system = "postgresql", + peer.service = "Postgres", + db.query.text = statement, + )) + .await + .change_context(CheckPermissionError::StoreError)? + .map_ok(|row| { + permitted_ids + .entry(EntityId { + web_id: row.get(web_id_idx), + entity_uuid: row.get(uuid_idx), + draft_id: row.get(draft_id_idx), + }) + .or_default() + .push(row.get(edition_id_idx)); + }) + .try_collect() + .await + .change_context(CheckPermissionError::StoreError)?; + + Ok(permitted_ids) + } +} + +impl EntityStore for PostgresStore +where + C: AsClient, + S: TransactionState, + Self: BeginReadOnlyTransaction, +{ + #[tracing::instrument(level = "info", skip(self, params))] + #[expect(clippy::too_many_lines)] + async fn create_entities( + &mut self, + actor_uuid: ActorEntityUuid, + params: Vec, + ) -> Result, Report> { + let transaction_time = Timestamp::::now().remove_nanosecond(); + let mut entity_edition_ids = Vec::with_capacity(params.len()); + + let mut entity_id_rows = Vec::with_capacity(params.len()); + let mut entity_draft_rows = Vec::new(); + let mut entity_edition_rows = Vec::with_capacity(params.len()); + let mut entity_temporal_metadata_rows = Vec::with_capacity(params.len()); + let mut entity_is_of_type_rows = Vec::with_capacity(params.len()); + let mut entity_edge_rows = Vec::new(); + + let mut policies = Vec::new(); + + let mut entities = Vec::with_capacity(params.len()); + // TODO: There are expected to be duplicates but we currently don't have a way to identify + // multi-type entity types. We need a way to speed this up. + let mut validation_params = Vec::with_capacity(params.len()); + + let transaction = self + .begin_transaction() + .await + .change_context(InsertionError)?; + + let actor_id = transaction + .determine_actor(actor_uuid) + .await + .change_context(InsertionError)? + .ok_or_else(|| Report::new(InsertionError).attach("Actor not found"))?; + + let mut policy_components_builder = PolicyComponents::builder(&transaction); + + let mut entity_ids = Vec::with_capacity(params.len()); + + // We will use the added entity type IDs to check for the instantiation permission later. + // This means that we need to make sure, that exactly the required entity types are passed + // here. + let mut entity_type_id_set = HashSet::with_capacity(params.len()); + for params in ¶ms { + let entity_id = EntityId { + web_id: params.web_id, + entity_uuid: params + .entity_uuid + .unwrap_or_else(|| EntityUuid::new(Uuid::new_v4())), + draft_id: params.draft.then(|| DraftId::new(Uuid::new_v4())), + }; + policy_components_builder.add_entity( + actor_id, + entity_id, + Cow::Owned(params.entity_type_ids.iter().cloned().collect()), + ); + entity_ids.push(entity_id); + + entity_type_id_set.extend(¶ms.entity_type_ids); + } + + // The policy components builder will make sure, that also parent entity types are added to + // the set of entity type IDs. These are accessible via `tracked_entity_types` method. + let policy_components = policy_components_builder + .with_actor(actor_id) + .with_entity_type_ids(entity_type_id_set) + .with_actions( + [ActionName::Instantiate, ActionName::CreateEntity], + MergePolicies::No, + ) + .with_actions( + [ + ActionName::ViewEntity, + ActionName::ViewEntityType, + ActionName::ViewPropertyType, + ActionName::ViewDataType, + ], + MergePolicies::Yes, + ) + .await + .change_context(InsertionError)?; + + let policy_set = policy_components + .build_policy_set([ActionName::Instantiate, ActionName::CreateEntity]) + .change_context(InsertionError)?; + + let mut forbidden_instantiations = Vec::new(); + for entity_type_id in policy_components.tracked_entity_types() { + match policy_set + .evaluate( + &Request { + actor: policy_components.actor_id(), + action: ActionName::Instantiate, + resource: &ResourceId::EntityType(Cow::Borrowed(entity_type_id.into())), + context: RequestContext::default(), + }, + policy_components.context(), + ) + .change_context(InsertionError)? + { + Authorized::Always => {} + Authorized::Never => { + forbidden_instantiations.push(entity_type_id); + } + } + } + + if !forbidden_instantiations.is_empty() { + return Err(Report::new(InsertionError) + .attach_opaque(StatusCode::PermissionDenied) + .attach( + "The actor does not have permission to instantiate one or more entity types", + ) + .attach( + forbidden_instantiations + .into_iter() + .map(ToString::to_string) + .map(Cow::Owned) + .intersperse(Cow::Borrowed(", ")) + .collect::(), + )); + } + + let mut forbidden_entity_creations = Vec::new(); + for entity_id in &entity_ids { + match policy_set + .evaluate( + &Request { + actor: policy_components.actor_id(), + action: ActionName::CreateEntity, + resource: &ResourceId::Entity(entity_id.entity_uuid), + context: RequestContext::default(), + }, + policy_components.context(), + ) + .change_context(InsertionError)? + { + Authorized::Always => {} + Authorized::Never => { + forbidden_entity_creations.push(entity_id); + } + } + } + + if !forbidden_entity_creations.is_empty() { + return Err(Report::new(InsertionError) + .attach_opaque(StatusCode::PermissionDenied) + .attach("The actor does not have permission to create one or more entities") + .attach( + forbidden_entity_creations + .into_iter() + .map(ToString::to_string) + .collect::>() + .join(", "), + )); + } + + let validator_provider = StoreProvider::new(&transaction, &policy_components); + + let mut validation_reports = HashMap::::new(); + + debug_assert_eq!( + params.len(), + entity_ids.len(), + "Number of parameters ({}) and entity ids ({}) must be the same", + params.len(), + entity_ids.len(), + ); + for (index, (mut params, &entity_id)) in params.into_iter().zip(&entity_ids).enumerate() { + let entity_type = ClosedMultiEntityType::from_multi_type_closed_schema( + stream::iter(¶ms.entity_type_ids) + .then(|entity_type_url| async { + OntologyTypeProvider::::provide_type( + &validator_provider, + entity_type_url, + ) + .await + .map(|entity_type| ClosedEntityType::clone(&*entity_type)) + }) + .try_collect::>() + .await + .change_context(InsertionError)?, + ) + .change_context(InsertionError)?; + + let mut preprocessor = EntityPreprocessor { + components: if params.draft { + ValidateEntityComponents::draft() + } else { + ValidateEntityComponents::full() + }, + convert_values: true, + }; + preprocessor.components.link_validation = transaction.settings.validate_links; + + if let Err(property_validation) = preprocessor + .visit_object(&entity_type, &mut params.properties, &validator_provider) + .await + { + validation_reports.entry(index).or_default().properties = + property_validation.properties; + } + + let (properties, property_metadata) = params.properties.into_parts(); + + let decision_time = params + .decision_time + .map_or_else(|| transaction_time.cast(), Timestamp::remove_nanosecond); + + let entity_provenance = EntityProvenance { + inferred: InferredEntityProvenance { created_by_id: actor_uuid, created_at_transaction_time: transaction_time, created_at_decision_time: decision_time, @@ -1311,96 +1772,49 @@ where .await { validation_report.properties = property_validation.properties; - } - - validation_report.link = params - .link_data - .as_deref() - .validate(&schema, params.components, &validator_provider) - .await; - - if !validation_report.is_valid() { - validation_reports.insert(index, validation_report); - } - } - - Ok(validation_reports) - } - - #[tracing::instrument(level = "info", skip(self, params))] - async fn query_entities( - &self, - actor_id: ActorEntityUuid, - mut params: QueryEntitiesParams<'_>, - ) -> Result, Report> { - let policy_components = PolicyComponents::builder(self) - .with_actor(actor_id) - .with_action(ActionName::ViewEntity, MergePolicies::Yes) - .await - .change_context(QueryError)?; - - let provider = StoreProvider::new(self, &policy_components); - - params - .filter - .convert_parameters(&provider) - .await - .change_context(QueryError)?; - - let temporal_axes = params.temporal_axes.resolve(); + } - let mut response = self - .query_entities_impl(¶ms, &temporal_axes, &policy_components) - .await?; + validation_report.link = params + .link_data + .as_deref() + .validate(&schema, params.components, &validator_provider) + .await; - if !params.conversions.is_empty() { - for entity in &mut response.entities { - self.convert_entity(&provider, entity, ¶ms.conversions) - .await - .change_context(QueryError)?; + if !validation_report.is_valid() { + validation_reports.insert(index, validation_report); } } - if params.include_permissions { - let entity_ids = response - .entities - .iter() - .map(|entity| entity.metadata.record_id.entity_id) - .collect::>(); - - let update_permissions = self - .has_permission_for_entities( - policy_components.actor_id().into(), - HasPermissionForEntitiesParams { - action: ActionName::UpdateEntity, - entity_ids: Cow::Borrowed(&entity_ids), - temporal_axes: params.temporal_axes, - include_drafts: params.include_drafts, - }, - ) - .await - .change_context(QueryError)?; + Ok(validation_reports) + } - let mut permissions: HashMap = - HashMap::with_capacity(update_permissions.len()); + #[tracing::instrument(level = "info", skip(self, params))] + async fn query_entities( + &mut self, + actor_id: ActorEntityUuid, + params: QueryEntitiesParams<'_>, + ) -> Result, Report> { + // An entity query consists of multiple statements: the entity read itself, the optional + // entity-type resolution, and the permission checks on the returned entities. Under + // `READ COMMITTED` each statement uses its own MVCC snapshot, so a write committing + // mid-read can yield entities, entity types, and permissions reflecting different + // states of the store. Running the whole read in a single `REPEATABLE READ, READ ONLY` + // transaction gives all statements one shared snapshot. + let transaction = self + .begin_read_only_transaction() + .await + .change_context(QueryError)?; - for (entity_id, editions) in update_permissions { - permissions.entry(entity_id).or_default().update = editions; - } + let response = transaction.query_entities_impl(actor_id, params).await?; - debug_assert!( - response.permissions.is_none(), - "Should not be populated yet" - ); - response.permissions = Some(permissions); - } + transaction.commit().await.change_context(QueryError)?; Ok(response) } #[tracing::instrument(level = "info", skip(self, params))] async fn search_entities( - &self, + &mut self, actor_id: ActorEntityUuid, params: SearchEntitiesParams, ) -> Result> { @@ -1434,279 +1848,92 @@ where parameter: Parameter::Vector(embedding), convert: None, }, - FilterExpression::Parameter { - parameter: Parameter::Decimal(maximum_distance), - convert: None, - }, - ), - Filter::Equal( - FilterExpression::Path { - path: EntityQueryPath::Archived, - }, - FilterExpression::Parameter { - parameter: Parameter::Boolean(false), - convert: None, - }, - ), - ]; - - if !entity_type_ids.is_empty() { - filters.push(Filter::Any( - entity_type_ids - .iter() - .map(Filter::for_entity_by_type_id) - .collect(), - )); - } - if !web_ids.is_empty() { - filters.push(Filter::In( - FilterExpression::Path { - path: EntityQueryPath::WebId, - }, - FilterExpressionList::ParameterList { - parameters: ParameterList::WebIds(&web_ids), - }, - )); - } - - let response = self - .query_entities( - actor_id, - QueryEntitiesParams { - filter: Filter::All(filters), - temporal_axes: QueryTemporalAxesUnresolved::live_only(), - sorting: EntityQuerySorting { - paths: vec![], - cursor: None, - }, - conversions: Vec::new(), - limit, - include_drafts, - include_entity_types: include_entity_types - .then_some(IncludeEntityTypeOption::Closed), - include_permissions: false, - }, - ) - .await?; - - Ok(SearchEntitiesResponse { - entities: response.entities, - closed_multi_entity_types: response.closed_multi_entity_types, - }) - } - - #[tracing::instrument(level = "info", skip(self, params))] - #[expect(clippy::too_many_lines)] - async fn query_entity_subgraph( - &self, - actor_id: ActorEntityUuid, - params: QueryEntitySubgraphParams<'_>, - ) -> Result, Report> { - let actions = params.view_actions(); - - let policy_components = PolicyComponents::builder(self) - .with_actor(actor_id) - .with_actions(actions, MergePolicies::Yes) - .await - .change_context(QueryError)?; - let actor = policy_components.actor_id(); - - let provider = StoreProvider::new(self, &policy_components); - - let (mut request, traversal_params) = params.into_parts(); - request - .filter - .convert_parameters(&provider) - .await - .change_context(QueryError)?; - - let temporal_axes = request.temporal_axes.resolve(); - let time_axis = temporal_axes.variable_time_axis(); - - let QueryEntitiesResponse { - entities: root_entities, - cursor, - closed_multi_entity_types: _, - definitions: _, - permissions, - } = self - .query_entities_impl(&request, &temporal_axes, &policy_components) - .await?; - - let mut subgraph = Subgraph::new(request.temporal_axes, temporal_axes); - - async move { - subgraph.roots.extend( - root_entities - .iter() - .map(|entity| entity.vertex_id(time_axis).into()), - ); - subgraph.vertices.entities = root_entities - .into_iter() - .map(|entity| (entity.vertex_id(time_axis), entity)) - .collect(); - - let mut traversal_context = TraversalContext::default(); - - // Iterate over each traversal path and call traverse_entities separately - match &traversal_params { - SubgraphTraversalParams::Paths { traversal_paths } => { - for path in traversal_paths { - let (entity_traversal_path, ontology_traversal_path) = - path.split_entity_path(); - self.traverse_entities_with_path( - &entity_traversal_path, - ontology_traversal_path, - &mut traversal_context, - &provider, - &mut subgraph, - ) - .await?; - } - } - SubgraphTraversalParams::ResolveDepths { - traversal_paths, - graph_resolve_depths, - } => { - if traversal_paths.is_empty() { - if graph_resolve_depths.is_of_type { - // If no entity traversal paths are specified, still initialize - // the traversal with ontology resolve depths to enable - // traversal of ontology edges (e.g., isOfType, inheritsFrom) - self.traverse_entities_with_resolve_depths( - &[], - *graph_resolve_depths, - &mut traversal_context, - &provider, - &mut subgraph, - ) - .await?; - } - } else { - for path in traversal_paths { - self.traverse_entities_with_resolve_depths( - &path.edges, - *graph_resolve_depths, - &mut traversal_context, - &provider, - &mut subgraph, - ) - .await?; - } - } - } - } - - traversal_context - .read_traversed_vertices( - self, - &mut subgraph, - request.include_drafts, - provider - .policy_components - .as_ref() - .expect("Policy components should be set"), - ) - .await?; - - if !request.conversions.is_empty() { - for entity in subgraph.vertices.entities.values_mut() { - self.convert_entity(&provider, entity, &request.conversions) - .await - .change_context(QueryError)?; - } - } - - Ok(QueryEntitySubgraphResponse { - closed_multi_entity_types: if request.include_entity_types.is_some() { - Some( - self.get_closed_multi_entity_types( - actor_id, - subgraph - .vertices - .entities - .values() - .map(|entity| entity.metadata.entity_type_ids.clone()), - QueryTemporalAxesUnresolved::live_only(), - None, - ) - .await? - .entity_types, - ) - } else { - None - }, - definitions: match request.include_entity_types { - Some( - IncludeEntityTypeOption::Resolved - | IncludeEntityTypeOption::ResolvedWithDataTypeChildren, - ) => { - let entity_type_uuids = subgraph - .vertices - .entities - .values() - .flat_map(|entity| { - entity - .metadata - .entity_type_ids - .iter() - .map(EntityTypeUuid::from_url) - }) - .collect::>() - .into_iter() - .collect::>(); - Some( - self.get_entity_type_resolve_definitions( - actor_id, - &entity_type_uuids, - request.include_entity_types - == Some(IncludeEntityTypeOption::ResolvedWithDataTypeChildren), - ) - .await?, - ) - } - None | Some(IncludeEntityTypeOption::Closed) => None, + FilterExpression::Parameter { + parameter: Parameter::Decimal(maximum_distance), + convert: None, }, - cursor, - entity_permissions: if request.include_permissions { - debug_assert!(permissions.is_none(), "Should not be populated yet"); + ), + Filter::Equal( + FilterExpression::Path { + path: EntityQueryPath::Archived, + }, + FilterExpression::Parameter { + parameter: Parameter::Boolean(false), + convert: None, + }, + ), + ]; - let entity_ids = subgraph - .vertices - .entities - .keys() - .map(|vertex_id| vertex_id.base_id) - .collect::>(); + if !entity_type_ids.is_empty() { + filters.push(Filter::Any( + entity_type_ids + .iter() + .map(Filter::for_entity_by_type_id) + .collect(), + )); + } + if !web_ids.is_empty() { + filters.push(Filter::In( + FilterExpression::Path { + path: EntityQueryPath::WebId, + }, + FilterExpressionList::ParameterList { + parameters: ParameterList::WebIds(&web_ids), + }, + )); + } - let update_permissions = self - .has_permission_for_entities( - actor.into(), - HasPermissionForEntitiesParams { - action: ActionName::UpdateEntity, - entity_ids: Cow::Borrowed(&entity_ids), - temporal_axes: request.temporal_axes, - include_drafts: request.include_drafts, - }, - ) - .await - .change_context(QueryError)?; + let response = self + .query_entities( + actor_id, + QueryEntitiesParams { + filter: Filter::All(filters), + temporal_axes: QueryTemporalAxesUnresolved::live_only(), + sorting: EntityQuerySorting { + paths: vec![], + cursor: None, + }, + conversions: Vec::new(), + limit, + include_drafts, + include_entity_types: include_entity_types + .then_some(IncludeEntityTypeOption::Closed), + include_permissions: false, + }, + ) + .await?; - let mut permissions: HashMap = - HashMap::with_capacity(update_permissions.len()); + Ok(SearchEntitiesResponse { + entities: response.entities, + closed_multi_entity_types: response.closed_multi_entity_types, + }) + } - for (entity_id, editions) in update_permissions { - permissions.entry(entity_id).or_default().update = editions; - } + #[tracing::instrument(level = "info", skip(self, params))] + async fn query_entity_subgraph( + &mut self, + actor_id: ActorEntityUuid, + params: QueryEntitySubgraphParams<'_>, + ) -> Result, Report> { + // A subgraph read consists of multiple statements: the roots query, one recursive CTE + // per traversal path, the permission filtering of the traversed edges, and the final + // vertex read. Under `READ COMMITTED` each statement uses its own MVCC snapshot, so a + // write committing mid-read can retroactively evict an edge endpoint's edition at the + // pinned timestamp, yielding a subgraph containing a link vertex without the edge to + // its endpoint. Running the whole read in a single `REPEATABLE READ, READ ONLY` + // transaction gives all statements one shared snapshot. + let transaction = self + .begin_read_only_transaction() + .await + .change_context(QueryError)?; - Some(permissions) - } else { - None - }, - subgraph, - }) - } - .instrument(tracing::trace_span!("construct_subgraph")) - .await + let response = transaction + .query_entity_subgraph_impl(actor_id, params) + .await?; + + transaction.commit().await.change_context(QueryError)?; + + Ok(response) } #[tracing::instrument(level = "info", skip_all)] @@ -1868,7 +2095,7 @@ where .decision_time .map_or_else(|| transaction_time.cast(), Timestamp::remove_nanosecond); - let transaction = self.transaction().await.change_context(UpdateError)?; + let transaction = self.begin_transaction().await.change_context(UpdateError)?; let locked_row = transaction .lock_entity_edition(params.entity_id, transaction_time, decision_time) @@ -2377,7 +2604,7 @@ where // TODO: Authorization — check delete permission via PolicyComponents let mut transaction = self - .transaction() + .begin_transaction() .await .change_context(DeletionError::Store)?; let summary = transaction @@ -2537,140 +2764,23 @@ where Ok(()) } - #[tracing::instrument(level = "info", skip(self))] async fn reindex_entity_cache(&mut self) -> Result<(), Report> { - tracing::info!("Reindexing entity cache"); - let transaction = self.transaction().await.change_context(UpdateError)?; - - // We remove the data from the reference tables first - transaction - .as_client() - .simple_query( - " - DELETE FROM entity_is_of_type WHERE inheritance_depth > 0; - - INSERT INTO entity_is_of_type ( - entity_edition_id, - entity_type_ontology_id, - inheritance_depth - ) - SELECT entity_edition_id, - target_entity_type_ontology_id AS entity_type_ontology_id, - MIN(entity_type_inherits_from.depth + 1) AS inheritance_depth - FROM entity_is_of_type - JOIN entity_type_inherits_from - ON entity_type_ontology_id = source_entity_type_ontology_id - GROUP BY entity_edition_id, target_entity_type_ontology_id; - - DELETE FROM entity_edition_cache; - ", - ) - .instrument(tracing::info_span!( - "INSERT", - otel.kind = "client", - db.system = "postgresql", - peer.service = "Postgres", - )) - .await - .change_context(UpdateError)?; - - transaction - .as_client() - .query(&insert_entity_edition_cache_statement(false), &[]) - .instrument(tracing::info_span!( - "INSERT", - otel.kind = "client", - db.system = "postgresql", - peer.service = "Postgres", - )) - .await - .change_context(UpdateError)?; - - transaction.commit().await.change_context(UpdateError)?; - - Ok(()) + // Delegates to the inherent method on `PostgresStore`, so the rebuild is also + // reachable where the `EntityStore` impl — bounded on `BeginReadOnlyTransaction` — is + // unavailable. + self.reindex_entity_cache_impl().await } - #[tracing::instrument(skip(self, params))] async fn has_permission_for_entities( &self, authenticated_actor: AuthenticatedActor, params: HasPermissionForEntitiesParams<'_>, ) -> Result>, Report> { - let temporal_axes = params.temporal_axes.resolve(); - let mut compiler = SelectCompiler::new(Some(&temporal_axes), params.include_drafts); - - let entity_uuids = params - .entity_ids - .iter() - .map(|id| id.entity_uuid) - .collect::>(); - - let entity_filter = Filter::In( - FilterExpression::Path { - path: EntityQueryPath::Uuid, - }, - FilterExpressionList::ParameterList { - parameters: ParameterList::EntityUuids(&entity_uuids), - }, - ); - compiler - .add_filter(&entity_filter) - .change_context(CheckPermissionError::CompileFilter)?; - - let policy_components = PolicyComponents::builder(self) - .with_actor(authenticated_actor) - .with_action(params.action, MergePolicies::Yes) - .await - .change_context(CheckPermissionError::BuildPolicyContext)?; - let policy_filter = Filter::::for_policies( - policy_components.extract_filter_policies(params.action), - policy_components.actor_id(), - policy_components.optimization_data(params.action), - ); - compiler - .add_filter(&policy_filter) - .change_context(CheckPermissionError::CompileFilter)?; - - let web_id_idx = compiler.add_selection_path(&EntityQueryPath::WebId); - let uuid_idx = compiler.add_selection_path(&EntityQueryPath::Uuid); - let draft_id_idx = compiler.add_selection_path(&EntityQueryPath::DraftId); - let edition_id_idx = compiler.add_distinct_selection_with_ordering( - &EntityQueryPath::EditionId, - Distinctness::Distinct, - None, - ); - - let mut permitted_ids = HashMap::>::new(); - - let (statement, parameters) = compiler.compile(); - let () = self - .as_client() - .query_raw(&statement, parameters.iter().copied()) - .instrument(tracing::info_span!( - "SELECT", - otel.kind = "client", - db.system = "postgresql", - peer.service = "Postgres", - db.query.text = statement, - )) + // Delegates to the inherent method on `PostgresStore`, so the permission check is + // also reachable where the `EntityStore` impl — bounded on `BeginReadOnlyTransaction` — + // is unavailable. + self.has_permission_for_entities_impl(authenticated_actor, params) .await - .change_context(CheckPermissionError::StoreError)? - .map_ok(|row| { - permitted_ids - .entry(EntityId { - web_id: row.get(web_id_idx), - entity_uuid: row.get(uuid_idx), - draft_id: row.get(draft_id_idx), - }) - .or_default() - .push(row.get(edition_id_idx)); - }) - .try_collect() - .await - .change_context(CheckPermissionError::StoreError)?; - - Ok(permitted_ids) } #[expect(clippy::too_many_lines)] @@ -2711,7 +2821,7 @@ where // Filter to entities the actor is allowed to view. let permitted = self - .has_permission_for_entities( + .has_permission_for_entities_impl( AuthenticatedActor::from(actor_id), HasPermissionForEntitiesParams { action: ActionName::ViewEntity, @@ -2889,7 +2999,7 @@ struct LockedEntityEdition { /// `entity_is_of_type` rows joined to the referenced types. /// /// The write paths pass `scoped` to restrict it to the just-written editions -/// (`$1: UUID[]`), `reindex_entity_cache` runs it unscoped over all editions. Must run +/// (`$1: UUID[]`), `reindex_entity_cache_impl` runs it unscoped over all editions. Must run /// after the editions' `entity_is_of_type` rows (including the inherited ones) have been /// written. fn insert_entity_edition_cache_statement(scoped: bool) -> String { @@ -2987,7 +3097,7 @@ fn insert_entity_edition_cache_statement(scoped: bool) -> String { ) } -impl PostgresStore> { +impl PostgresStore, InTransaction> { #[tracing::instrument(level = "info", skip_all)] async fn insert_entity_edition( &self, diff --git a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/read.rs b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/read.rs index fe1cecbd053..fcdfe7f36b6 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/read.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/knowledge/entity/read.rs @@ -33,7 +33,7 @@ use type_system::{ use crate::store::{ StoreProvider, - postgres::{AsClient, PostgresStore, query::SelectCompiler}, + postgres::{AsClient, PostgresStore, TransactionState, query::SelectCompiler}, }; #[derive(Debug)] @@ -113,9 +113,10 @@ pub struct EntityTraversalResult { pub edge_hops: Vec, } -impl PostgresStore +impl PostgresStore where C: AsClient, + S: TransactionState, { #[tracing::instrument(level = "info", skip(self))] pub(crate) async fn read_shared_edges<'t>( diff --git a/libs/@local/graph/postgres-store/src/store/postgres/migration.rs b/libs/@local/graph/postgres-store/src/store/postgres/migration.rs index c6b750bab21..1815e196a9f 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/migration.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/migration.rs @@ -2,7 +2,7 @@ use error_stack::{Report, ResultExt as _}; use hash_graph_store::migration::{Migration, MigrationError, MigrationState, StoreMigration}; use tokio_postgres::Client; -use super::{AsClient, PostgresStore}; +use super::{AsClient, PostgresStore, TransactionState}; mod embedded { use refinery::embed_migrations; @@ -25,9 +25,10 @@ fn create_postgres_migration(value: &refinery::Migration) -> Migration { Migration::new(name, state, value.checksum()) } -impl StoreMigration for PostgresStore +impl StoreMigration for PostgresStore where C: AsClient, + S: TransactionState, { async fn run_migrations(&mut self) -> Result, Report> { Ok(embedded::migrations::runner() diff --git a/libs/@local/graph/postgres-store/src/store/postgres/mod.rs b/libs/@local/graph/postgres-store/src/store/postgres/mod.rs index 48d57109be8..ddcfdddfba6 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/mod.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/mod.rs @@ -8,7 +8,7 @@ mod seed_policies; mod traversal_context; use alloc::{borrow::Cow, sync::Arc}; -use core::{borrow::Borrow, fmt::Debug, hash::Hash}; +use core::{borrow::Borrow, fmt::Debug, hash::Hash, marker::PhantomData}; use std::collections::{HashMap, HashSet}; use error_stack::{Report, ResultExt as _, TryReportStreamExt as _}; @@ -35,6 +35,7 @@ use hash_graph_authorization::policies::{ }, }, }; +pub use hash_graph_migrations::{Context, IsolationLevel, Transaction, TransactionBuilder}; use hash_graph_store::{ account::{ AccountGroupInsertionError, AccountInsertionError, AccountStore, CreateAiActorParams, @@ -76,7 +77,10 @@ use type_system::{ use uuid::Uuid; pub use self::{ - pool::{AsClient, PostgresStorePool}, + pool::{ + AsClient, InTransaction, NoTransaction, PostgresStorePool, TransactionOptions, + TransactionState, + }, traversal_context::TraversalContext, }; use crate::store::error::{ @@ -106,22 +110,27 @@ impl Default for PostgresStoreSettings { } /// A Postgres-backed store. -pub struct PostgresStore { +/// +/// The `S` parameter tracks at the type level whether the store is currently inside a database +/// transaction, see [`TransactionState`]. +pub struct PostgresStore { client: C, pub temporal_client: Option>, pub settings: Arc, + _transaction_state: PhantomData, } -impl AsRef for PostgresStore +impl AsRef for PostgresStore where C: AsClient, + S: TransactionState, { fn as_ref(&self) -> &Client { self.client.as_client() } } -impl PostgresStore> { +impl PostgresStore, InTransaction> { /// Inserts multiple policies into the database. /// /// # Errors @@ -581,9 +590,10 @@ impl PostgresStore> { } } -impl PostgresStore +impl PostgresStore where C: AsClient, + S: TransactionState, { async fn get_policy_from_database( &self, @@ -796,16 +806,17 @@ where } } -impl PrincipalStore for PostgresStore +impl PrincipalStore for PostgresStore where C: AsClient, + S: TransactionState, { async fn get_or_create_system_machine( &mut self, identifier: &str, ) -> Result> { let mut transaction = self - .transaction() + .begin_transaction() .await .change_context(GetSystemAccountError::StoreError)?; @@ -857,7 +868,7 @@ where } let mut transaction = self - .transaction() + .begin_transaction() .await .change_context(WebCreationError::StoreError)?; @@ -1049,7 +1060,7 @@ where } let mut transaction = self - .transaction() + .begin_transaction() .await .change_context(RoleAssignmentError::StoreError)?; @@ -1173,7 +1184,7 @@ where } let mut transaction = self - .transaction() + .begin_transaction() .await .change_context(RoleAssignmentError::StoreError)?; @@ -1424,9 +1435,10 @@ impl PolicyParts { } } -impl PolicyStore for PostgresStore +impl PolicyStore for PostgresStore where C: AsClient, + S: TransactionState, { async fn create_policy( &mut self, @@ -1434,7 +1446,7 @@ where policy: PolicyCreationParams, ) -> Result> { let transaction = self - .transaction() + .begin_transaction() .await .change_context(CreatePolicyError::StoreError)?; @@ -1745,7 +1757,7 @@ where operations: &[PolicyUpdateOperation], ) -> Result> { let transaction = self - .transaction() + .begin_transaction() .await .change_context(UpdatePolicyError::StoreError)?; @@ -1917,7 +1929,7 @@ where async fn seed_system_policies(&mut self) -> Result<(), Report> { let mut transaction = self - .transaction() + .begin_transaction() .await .change_context(EnsureSystemPoliciesError::StoreError)?; @@ -2460,9 +2472,10 @@ impl From> for HashMap { } } -impl PostgresStore +impl PostgresStore where C: AsClient, + S: TransactionState, { /// Creates a new `PostgresDatabase` object. #[must_use] @@ -2475,9 +2488,39 @@ where client, temporal_client, settings, + _transaction_state: PhantomData, } } + /// Begins an unconfigured database transaction. + /// + /// On a store which is not inside a transaction this issues a plain `BEGIN` using the + /// database's default transaction characteristics. On a store which is already inside a + /// transaction it creates a savepoint instead; a savepoint has no characteristics of its own + /// and runs within the enclosing transaction. + /// + /// Transaction characteristics such as the isolation level can only be configured when + /// beginning a top-level transaction via [`Context::transaction`], which is only available on + /// stores in the [`NoTransaction`] state. + /// + /// # Errors + /// + /// - if the underlying client cannot begin the transaction + pub async fn begin_transaction( + &mut self, + ) -> Result, InTransaction>, Report> + { + Ok(PostgresStore::new( + self.client + .as_mut_client() + .transaction() + .await + .change_context(StoreError)?, + self.temporal_client.clone(), + Arc::clone(&self.settings), + )) + } + async fn create_base_url( &self, base_url: &BaseUrl, @@ -3158,26 +3201,172 @@ where Ok(()) } +} + +/// A [`TransactionBuilder`] for a [`PostgresStore`]. +/// +/// Created by [`Context::transaction`], this builder begins the transaction when awaited. The +/// configured [`TransactionOptions`] are compiled into the single `BEGIN` statement issued to +/// the database. Configurable transactions are only available on stores in the +/// [`NoTransaction`] state, so the options always apply to a top-level transaction and are +/// never silently discarded. +pub struct PostgresStoreTransactionBuilder<'t, C> { + store: &'t mut PostgresStore, + options: TransactionOptions, +} + +impl<'t, C> IntoFuture for PostgresStoreTransactionBuilder<'t, C> +where + C: AsClient, +{ + type Output = + Result, InTransaction>, Report>; + + type IntoFuture = impl Future + Send; + + fn into_future(self) -> Self::IntoFuture { + async move { + let mut builder = self.store.client.as_mut_client().build_transaction(); + if let Some(isolation_level) = self.options.isolation_level { + builder = builder.isolation_level(isolation_level.into()); + } + if self.options.read_only { + builder = builder.read_only(true); + } + if self.options.deferrable { + builder = builder.deferrable(true); + } + Ok(PostgresStore::new( + builder.start().await.change_context(StoreError)?, + self.store.temporal_client.clone(), + Arc::clone(&self.store.settings), + )) + } + } +} + +impl<'t, C> TransactionBuilder for PostgresStoreTransactionBuilder<'t, C> +where + C: AsClient, +{ + type Error = StoreError; + type Transaction = PostgresStore, InTransaction>; + + fn isolation_level(mut self, isolation_level: IsolationLevel) -> Self { + self.options.isolation_level = Some(isolation_level); + self + } + + fn read_only(mut self) -> Self { + self.options.read_only = true; + self + } + + fn deferrable(mut self) -> Self { + self.options.deferrable = true; + self + } +} + +impl Context for PostgresStore +where + C: AsClient, +{ + type Error = StoreError; + type TransactionBuilder<'t> + = PostgresStoreTransactionBuilder<'t, C> + where + Self: 't; + /// Returns a [`PostgresStoreTransactionBuilder`] which begins the transaction when awaited. + /// + /// By default the transaction is begun with the database's default characteristics. Options + /// can be configured on the builder before awaiting it: + /// + /// ```ignore + /// let transaction = store + /// .transaction() + /// .isolation_level(IsolationLevel::RepeatableRead) + /// .read_only() + /// .await?; + /// ``` + fn transaction(&mut self) -> Self::TransactionBuilder<'_> { + PostgresStoreTransactionBuilder { + store: self, + options: TransactionOptions::default(), + } + } +} + +impl Transaction for PostgresStore, InTransaction> { + type Error = StoreError; + + async fn commit(self) -> Result<(), Report> { + self.client.commit().await.change_context(StoreError) + } + + async fn rollback(self) -> Result<(), Report> { + self.client.rollback().await.change_context(StoreError) + } +} + +/// Begins the transaction backing a snapshot-consistent, multi-statement read. +/// +/// The behavior is fixed per [`TransactionState`], so nothing is ever silently discarded: +/// +/// - In the [`NoTransaction`] state a `REPEATABLE READ, READ ONLY` transaction is begun, giving all +/// statements of the read one shared snapshot. +/// - In the [`InTransaction`] state — only available with the `test-utils` feature — a savepoint is +/// created instead: the read runs within the enclosing transaction and observes that +/// transaction's snapshot semantics. +// TODO(BE-688): The `InTransaction` impl exists only for the rollback-isolation test harness, +// which bypasses the `REPEATABLE READ` snapshot — remove it once the harness uses per-test +// databases. +pub trait BeginReadOnlyTransaction { + /// Begins the transaction serving a snapshot-consistent read. + /// /// # Errors /// - /// - if the underlying client cannot start a transaction - pub async fn transaction( + /// - if the underlying client cannot begin the transaction + fn begin_read_only_transaction( &mut self, - ) -> Result>, Report> { - Ok(PostgresStore::new( - self.client - .as_mut_client() - .transaction() - .await - .change_context(StoreError)?, - self.temporal_client.clone(), - Arc::clone(&self.settings), - )) + ) -> impl Future< + Output = Result< + PostgresStore, InTransaction>, + Report, + >, + > + Send; +} + +impl BeginReadOnlyTransaction for PostgresStore +where + C: AsClient, +{ + async fn begin_read_only_transaction( + &mut self, + ) -> Result, InTransaction>, Report> + { + self.transaction() + .isolation_level(IsolationLevel::RepeatableRead) + .read_only() + .await + } +} + +#[cfg(feature = "test-utils")] +impl BeginReadOnlyTransaction for PostgresStore +where + C: AsClient, +{ + async fn begin_read_only_transaction( + &mut self, + ) -> Result, InTransaction>, Report> + { + self.begin_transaction().await } } -impl PostgresStore> { +impl PostgresStore, InTransaction> { /// Inserts the specified ontology metadata. /// /// This first extracts the [`BaseUrl`] from the [`VersionedUrl`] and attempts to insert it into @@ -3339,30 +3528,16 @@ impl PostgresStore> { OntologyTemporalMetadata { transaction_time }, )) } - - /// # Errors - /// - /// - if the underlying client cannot commit the transaction - pub async fn commit(self) -> Result<(), Report> { - self.client.commit().await.change_context(StoreError) - } - - /// # Errors - /// - /// - if the underlying client cannot rollback the transaction - pub async fn rollback(self) -> Result<(), Report> { - self.client.rollback().await.change_context(StoreError) - } } -impl AccountStore for PostgresStore { +impl AccountStore for PostgresStore { async fn create_user_actor( &mut self, actor_id: ActorEntityUuid, params: CreateUserActorParams, ) -> Result> { let mut transaction = self - .transaction() + .begin_transaction() .await .change_context(AccountInsertionError)?; @@ -3829,7 +4004,10 @@ impl AccountStore for PostgresStore { actor_id: ActorEntityUuid, params: CreateOrgWebParams, ) -> Result> { - let mut transaction = self.transaction().await.change_context(WebInsertionError)?; + let mut transaction = self + .begin_transaction() + .await + .change_context(WebInsertionError)?; let actor_id = transaction .determine_actor(actor_id) @@ -3991,7 +4169,7 @@ impl AccountStore for PostgresStore { params: CreateTeamParams, ) -> Result> { let mut transaction = self - .transaction() + .begin_transaction() .await .change_context(AccountGroupInsertionError)?; @@ -4134,9 +4312,10 @@ impl AccountStore for PostgresStore { } } -impl PostgresStore +impl PostgresStore where C: AsClient, + S: TransactionState, { /// Deletes all principals (policies and actions) from the database. /// diff --git a/libs/@local/graph/postgres-store/src/store/postgres/ontology/data_type.rs b/libs/@local/graph/postgres-store/src/store/postgres/ontology/data_type.rs index ea76fe023f4..79bd5797e53 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/ontology/data_type.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/ontology/data_type.rs @@ -8,6 +8,7 @@ use hash_graph_authorization::policies::{ Authorized, MergePolicies, PolicyComponents, Request, RequestContext, ResourceId, action::ActionName, principal::actor::AuthenticatedActor, }; +use hash_graph_migrations::Transaction as _; use hash_graph_store::{ data_type::{ ArchiveDataTypeParams, CountDataTypesParams, CreateDataTypeParams, @@ -54,7 +55,7 @@ use type_system::{ use crate::store::{ error::DeletionError, postgres::{ - AsClient, PostgresStore, TraversalContext, + AsClient, PostgresStore, TransactionState, TraversalContext, crud::{QueryIndices, QueryRecordDecode, TypedRow}, ontology::{PostgresOntologyOwnership, read::OntologyTypeTraversalData}, query::{ @@ -65,9 +66,10 @@ use crate::store::{ validation::StoreProvider, }; -impl PostgresStore +impl PostgresStore where C: AsClient, + S: TransactionState, { #[tracing::instrument(level = "info", skip(data_types, provider))] pub(crate) async fn filter_data_types_by_permission( @@ -436,7 +438,10 @@ where /// if the transaction cannot be committed. #[tracing::instrument(level = "info", skip(self))] pub async fn delete_data_types(&mut self) -> Result<(), Report> { - let transaction = self.transaction().await.change_context(DeletionError)?; + let transaction = self + .begin_transaction() + .await + .change_context(DeletionError)?; transaction .as_client() @@ -485,9 +490,10 @@ where } } -impl DataTypeStore for PostgresStore +impl DataTypeStore for PostgresStore where C: AsClient, + S: TransactionState, { #[tracing::instrument(level = "info", skip(self, params))] #[expect(clippy::too_many_lines)] @@ -499,7 +505,10 @@ where where P: IntoIterator + Send, { - let transaction = self.transaction().await.change_context(InsertionError)?; + let transaction = self + .begin_transaction() + .await + .change_context(InsertionError)?; let mut inserted_data_type_metadata = Vec::new(); let mut inserted_data_types = Vec::new(); @@ -925,7 +934,7 @@ where where P: IntoIterator + Send, { - let transaction = self.transaction().await.change_context(UpdateError)?; + let transaction = self.begin_transaction().await.change_context(UpdateError)?; let mut updated_data_type_metadata = Vec::new(); let mut inserted_data_types = Vec::new(); @@ -1521,7 +1530,7 @@ where #[tracing::instrument(level = "info", skip(self))] async fn reindex_data_type_cache(&mut self) -> Result<(), Report> { tracing::info!("Reindexing data type cache"); - let transaction = self.transaction().await.change_context(UpdateError)?; + let transaction = self.begin_transaction().await.change_context(UpdateError)?; // We remove the data from the reference tables first transaction diff --git a/libs/@local/graph/postgres-store/src/store/postgres/ontology/entity_type.rs b/libs/@local/graph/postgres-store/src/store/postgres/ontology/entity_type.rs index 3bc27886be3..2d1d7c8a508 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/ontology/entity_type.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/ontology/entity_type.rs @@ -9,8 +9,9 @@ use hash_graph_authorization::policies::{ Authorized, MergePolicies, PolicyComponents, Request, RequestContext, ResourceId, action::ActionName, principal::actor::AuthenticatedActor, }; +use hash_graph_migrations::Transaction as _; use hash_graph_store::{ - entity::{ClosedMultiEntityTypeMap, EntityStore}, + entity::ClosedMultiEntityTypeMap, entity_type::{ ArchiveEntityTypeParams, ClosedDataTypeDefinition, CommonQueryEntityTypesParams, CountEntityTypesParams, CreateEntityTypeParams, EntityTypeQueryPath, @@ -69,7 +70,7 @@ use type_system::{ use crate::store::{ error::DeletionError, postgres::{ - AsClient, PostgresStore, ResponseCountMap, TraversalContext, + AsClient, PostgresStore, ResponseCountMap, TransactionState, TraversalContext, crud::{QueryIndices, QueryRecordDecode, TypedRow}, ontology::{PostgresOntologyOwnership, read::OntologyTypeTraversalData}, query::{ @@ -79,9 +80,10 @@ use crate::store::{ validation::StoreProvider, }; -impl PostgresStore +impl PostgresStore where C: AsClient, + S: TransactionState, { #[tracing::instrument(level = "info", skip(entity_types, provider))] pub(crate) async fn filter_entity_types_by_permission( @@ -823,7 +825,10 @@ where /// if the transaction cannot be committed. #[tracing::instrument(level = "info", skip(self))] pub async fn delete_entity_types(&mut self) -> Result<(), Report> { - let transaction = self.transaction().await.change_context(DeletionError)?; + let transaction = self + .begin_transaction() + .await + .change_context(DeletionError)?; transaction .as_client() @@ -874,9 +879,10 @@ where } } -impl EntityTypeStore for PostgresStore +impl EntityTypeStore for PostgresStore where C: AsClient, + S: TransactionState, { #[tracing::instrument(level = "info", skip(self, params))] #[expect(clippy::too_many_lines)] @@ -888,7 +894,10 @@ where where P: IntoIterator + Send, { - let transaction = self.transaction().await.change_context(InsertionError)?; + let transaction = self + .begin_transaction() + .await + .change_context(InsertionError)?; let mut inserted_entity_type_metadata = Vec::new(); let mut inserted_entity_types = Vec::new(); @@ -1530,7 +1539,7 @@ where where P: IntoIterator + Send, { - let transaction = self.transaction().await.change_context(UpdateError)?; + let transaction = self.begin_transaction().await.change_context(UpdateError)?; let mut updated_entity_type_metadata = Vec::new(); let mut inserted_entity_types = Vec::new(); @@ -1924,7 +1933,7 @@ where #[tracing::instrument(level = "info", skip(self))] async fn reindex_entity_type_cache(&mut self) -> Result<(), Report> { tracing::info!("Reindexing entity type cache"); - let mut transaction = self.transaction().await.change_context(UpdateError)?; + let mut transaction = self.begin_transaction().await.change_context(UpdateError)?; // We remove the data from the reference tables first transaction @@ -2004,7 +2013,7 @@ where // The entity edition cache derives type titles, labels (via `closed_schema`), and // the inherited type entries from the data rebuilt above, so it has to be rebuilt // as well — otherwise it silently keeps serving the pre-reindex schemas. - EntityStore::reindex_entity_cache(&mut transaction).await?; + transaction.reindex_entity_cache_impl().await?; transaction.commit().await.change_context(UpdateError)?; diff --git a/libs/@local/graph/postgres-store/src/store/postgres/ontology/mod.rs b/libs/@local/graph/postgres-store/src/store/postgres/ontology/mod.rs index 8e909ecc92b..7663bb1f02b 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/ontology/mod.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/ontology/mod.rs @@ -31,13 +31,13 @@ use type_system::{ use crate::store::{ error::DeletionError, postgres::{ - AsClient as _, PostgresStore, + AsClient as _, InTransaction, PostgresStore, crud::QueryRecordDecode, query::{Distinctness, PostgresSorting, SelectCompiler, SelectCompilerError}, }, }; -impl PostgresStore> { +impl PostgresStore, InTransaction> { /// Deletes ontology metadata for the specified ontology type UUIDs. /// /// This function removes ontology ownership metadata, temporal metadata, diff --git a/libs/@local/graph/postgres-store/src/store/postgres/ontology/property_type.rs b/libs/@local/graph/postgres-store/src/store/postgres/ontology/property_type.rs index 9962f0968b6..4aeeffd5573 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/ontology/property_type.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/ontology/property_type.rs @@ -7,6 +7,7 @@ use hash_graph_authorization::policies::{ Authorized, MergePolicies, PolicyComponents, Request, RequestContext, ResourceId, action::ActionName, principal::actor::AuthenticatedActor, }; +use hash_graph_migrations::Transaction as _; use hash_graph_store::{ error::{CheckPermissionError, InsertionError, QueryError, UpdateError}, filter::Filter, @@ -51,7 +52,7 @@ use type_system::{ use crate::store::{ error::DeletionError, postgres::{ - AsClient, PostgresStore, TraversalContext, + AsClient, PostgresStore, TransactionState, TraversalContext, crud::{QueryIndices, QueryRecordDecode, TypedRow}, ontology::{PostgresOntologyOwnership, read::OntologyTypeTraversalData}, query::{ @@ -61,9 +62,10 @@ use crate::store::{ validation::StoreProvider, }; -impl PostgresStore +impl PostgresStore where C: AsClient, + S: TransactionState, { #[tracing::instrument(level = "trace", skip(property_types, provider))] pub(crate) async fn filter_property_types_by_permission( @@ -434,7 +436,10 @@ where /// if the transaction cannot be committed. #[tracing::instrument(level = "info", skip(self))] pub async fn delete_property_types(&mut self) -> Result<(), Report> { - let transaction = self.transaction().await.change_context(DeletionError)?; + let transaction = self + .begin_transaction() + .await + .change_context(DeletionError)?; transaction .as_client() @@ -483,9 +488,10 @@ where } } -impl PropertyTypeStore for PostgresStore +impl PropertyTypeStore for PostgresStore where C: AsClient, + S: TransactionState, { #[tracing::instrument(level = "info", skip(self, params))] #[expect(clippy::too_many_lines)] @@ -497,7 +503,10 @@ where where P: IntoIterator + Send, { - let transaction = self.transaction().await.change_context(InsertionError)?; + let transaction = self + .begin_transaction() + .await + .change_context(InsertionError)?; let mut inserted_property_type_metadata = Vec::new(); let mut inserted_property_types = Vec::new(); @@ -835,7 +844,7 @@ where where P: IntoIterator + Send, { - let transaction = self.transaction().await.change_context(UpdateError)?; + let transaction = self.begin_transaction().await.change_context(UpdateError)?; let mut updated_property_type_metadata = Vec::new(); let mut inserted_property_types = Vec::new(); diff --git a/libs/@local/graph/postgres-store/src/store/postgres/ontology/read.rs b/libs/@local/graph/postgres-store/src/store/postgres/ontology/read.rs index 556f7b7ee3b..32be478ba5a 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/ontology/read.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/ontology/read.rs @@ -24,7 +24,7 @@ use type_system::ontology::{ use super::PostgresOntologyOwnership; use crate::store::postgres::{ - AsClient, PostgresStore, + AsClient, PostgresStore, TransactionState, query::{ Distinctness, ForeignKeyReference, ReferenceTable, SelectCompiler, Table, Transpile as _, table::DatabaseColumn as _, @@ -59,7 +59,7 @@ pub struct OntologyEdgeTraversal<'edges, L, R> { pub traversal_interval: RightBoundedTemporalInterval, } -impl PostgresStore { +impl PostgresStore { #[tracing::instrument(level = "info", skip(self, filter))] pub(crate) async fn read_closed_schemas<'f>( &self, diff --git a/libs/@local/graph/postgres-store/src/store/postgres/pool.rs b/libs/@local/graph/postgres-store/src/store/postgres/pool.rs index 34b81208677..152af1229bd 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/pool.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/pool.rs @@ -4,6 +4,7 @@ use deadpool_postgres::{ Hook, ManagerConfig, Object, Pool, PoolConfig, PoolError, RecyclingMethod, Timeouts, }; use error_stack::{Report, ResultExt as _}; +use hash_graph_migrations::IsolationLevel; use hash_graph_store::pool::StorePool; use hash_temporal_client::TemporalClient; use tokio_postgres::{ @@ -108,6 +109,50 @@ impl StorePool for PostgresStorePool { } } +/// Options used to begin a database transaction. +/// +/// The options are collected by a [`PostgresStoreTransactionBuilder`] and compiled into the +/// single `BEGIN` statement issued to the database when the transaction is begun, e.g. `START +/// TRANSACTION ISOLATION LEVEL REPEATABLE READ, READ ONLY`. +/// +/// [`PostgresStoreTransactionBuilder`]: crate::store::PostgresStoreTransactionBuilder +#[derive(Debug, Copy, Clone, Default, PartialEq, Eq)] +pub struct TransactionOptions { + pub isolation_level: Option, + pub read_only: bool, + pub deferrable: bool, +} + +mod sealed { + pub trait Sealed {} +} + +/// A type-level marker describing whether a [`PostgresStore`] is currently inside a database +/// transaction. +/// +/// The trait is sealed: the set of states is closed over [`NoTransaction`] and [`InTransaction`]. +/// The state determines which transaction APIs exist on the store: a *configurable* top-level +/// transaction ([`Context::transaction`]) can only be begun in the [`NoTransaction`] state, while +/// a store in the [`InTransaction`] state can only nest by creating savepoints, which have no +/// configurable characteristics of their own. +/// +/// [`Context::transaction`]: hash_graph_migrations::Context::transaction +pub trait TransactionState: sealed::Sealed + Send + Sync + 'static {} + +/// Marker for a [`PostgresStore`] which is not inside a database transaction. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub struct NoTransaction; + +impl sealed::Sealed for NoTransaction {} +impl TransactionState for NoTransaction {} + +/// Marker for a [`PostgresStore`] which is inside a database transaction. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub struct InTransaction; + +impl sealed::Sealed for InTransaction {} +impl TransactionState for InTransaction {} + pub trait AsClient: Send + Sync { type Client: GenericClient + Send + Sync; @@ -151,9 +196,10 @@ impl AsClient for Transaction<'_> { } } -impl AsClient for PostgresStore +impl AsClient for PostgresStore where C: AsClient, + S: TransactionState, { type Client = C::Client; diff --git a/libs/@local/graph/postgres-store/src/store/postgres/seed_policies.rs b/libs/@local/graph/postgres-store/src/store/postgres/seed_policies.rs index f69eb7d196e..97bfbb87cac 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/seed_policies.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/seed_policies.rs @@ -28,7 +28,7 @@ use type_system::{ }, }; -use super::PostgresStore; +use super::{InTransaction, PostgresStore}; struct EntityTypeConfig { base_url: BaseUrl, @@ -911,7 +911,7 @@ pub(crate) fn instance_admins_policies(role: &TeamRole) -> Vec> { +impl PostgresStore, InTransaction> { #[expect(clippy::too_many_lines)] pub(crate) async fn update_seeded_policies( &mut self, diff --git a/libs/@local/graph/postgres-store/src/store/postgres/traversal_context.rs b/libs/@local/graph/postgres-store/src/store/postgres/traversal_context.rs index 07d04eceb93..401c869223f 100644 --- a/libs/@local/graph/postgres-store/src/store/postgres/traversal_context.rs +++ b/libs/@local/graph/postgres-store/src/store/postgres/traversal_context.rs @@ -30,14 +30,15 @@ use type_system::{ }; use crate::store::postgres::{ - AsClient, PostgresStore, + AsClient, PostgresStore, TransactionState, crud::QueryRecordDecode as _, query::{PostgresRecord as _, SelectCompiler}, }; -impl PostgresStore +impl PostgresStore where C: AsClient, + S: TransactionState, { #[tracing::instrument(level = "info", skip(self, data_type_ids, subgraph))] async fn read_data_types_by_ids( @@ -281,9 +282,9 @@ impl<'edges> TraversalContext<'edges> { /// Returns an error if any of the database read operations fail or if there are issues /// inserting vertices into the subgraph. #[tracing::instrument(level = "info", skip(self, store, subgraph))] - pub async fn read_traversed_vertices( + pub async fn read_traversed_vertices( self, - store: &PostgresStore, + store: &PostgresStore, subgraph: &mut Subgraph, include_drafts: bool, policy_components: &PolicyComponents, diff --git a/libs/@local/graph/postgres-store/src/store/validation.rs b/libs/@local/graph/postgres-store/src/store/validation.rs index 32786bee6fb..ac889db9ee6 100644 --- a/libs/@local/graph/postgres-store/src/store/validation.rs +++ b/libs/@local/graph/postgres-store/src/store/validation.rs @@ -30,7 +30,7 @@ use type_system::{ }, }; -use crate::store::postgres::{AsClient, PostgresStore}; +use crate::store::postgres::{AsClient, PostgresStore, TransactionState}; #[derive(Debug, Clone)] enum Access { @@ -139,9 +139,10 @@ impl<'a, S> StoreProvider<'a, S> { } } -impl StoreProvider<'_, PostgresStore> +impl StoreProvider<'_, PostgresStore> where C: AsClient, + S: TransactionState, { async fn authorize_data_type(&self, type_id: DataTypeUuid) -> Result<(), Report> { if let Some(policy_components) = &self.policy_components { @@ -172,9 +173,10 @@ where } } -impl DataTypeLookup for StoreProvider<'_, PostgresStore> +impl DataTypeLookup for StoreProvider<'_, PostgresStore> where C: AsClient, + S: TransactionState, { type ClosedDataType = Arc; type DataTypeWithMetadata = Arc; @@ -365,9 +367,10 @@ where } } -impl StoreProvider<'_, PostgresStore> +impl StoreProvider<'_, PostgresStore> where C: AsClient, + S: TransactionState, { async fn fetch_property_type( &self, @@ -416,9 +419,10 @@ where } } -impl OntologyTypeProvider for StoreProvider<'_, PostgresStore> +impl OntologyTypeProvider for StoreProvider<'_, PostgresStore> where C: AsClient, + S: TransactionState, { type Value = Arc; @@ -454,9 +458,10 @@ where } } -impl StoreProvider<'_, PostgresStore> +impl StoreProvider<'_, PostgresStore> where C: AsClient, + S: TransactionState, { async fn fetch_entity_type( &self, @@ -503,9 +508,11 @@ where } } -impl OntologyTypeProvider for StoreProvider<'_, PostgresStore> +impl OntologyTypeProvider + for StoreProvider<'_, PostgresStore> where C: AsClient, + S: TransactionState, { type Value = Arc; @@ -548,9 +555,10 @@ where } } -impl OntologyTypeProvider for StoreProvider<'_, PostgresStore> +impl OntologyTypeProvider for StoreProvider<'_, PostgresStore> where C: AsClient, + S: TransactionState, { type Value = Arc; @@ -581,9 +589,10 @@ where } } -impl EntityProvider for StoreProvider<'_, PostgresStore> +impl EntityProvider for StoreProvider<'_, PostgresStore> where C: AsClient, + S: TransactionState, { #[expect(refining_impl_trait)] async fn provide_entity(&self, entity_id: EntityId) -> Result, Report> { diff --git a/libs/@local/graph/postgres-store/tests/deletion/main.rs b/libs/@local/graph/postgres-store/tests/deletion/main.rs index f08d528c58f..b5f146ad5c9 100644 --- a/libs/@local/graph/postgres-store/tests/deletion/main.rs +++ b/libs/@local/graph/postgres-store/tests/deletion/main.rs @@ -14,7 +14,7 @@ use std::collections::{HashMap, HashSet}; use error_stack::{Report, ResultExt as _}; use hash_graph_authorization::policies::store::{PolicyStore as _, PrincipalStore as _}; -use hash_graph_postgres_store::store::{AsClient as _, PostgresStore}; +use hash_graph_postgres_store::store::{AsClient as _, Context as _, InTransaction, PostgresStore}; use hash_graph_store::{ account::{AccountStore as _, CreateUserActorParams}, data_type::{CreateDataTypeParams, DataTypeStore as _}, @@ -59,7 +59,7 @@ use type_system::{ pub use crate::common::DatabaseTestWrapper; pub struct DatabaseApi<'pool> { - pub store: PostgresStore>, + pub store: PostgresStore, InTransaction>, pub account_id: ActorEntityUuid, } diff --git a/libs/@local/graph/postgres-store/tests/principals/actions.rs b/libs/@local/graph/postgres-store/tests/principals/actions.rs index 1a52b1156ba..62ec3b87317 100644 --- a/libs/@local/graph/postgres-store/tests/principals/actions.rs +++ b/libs/@local/graph/postgres-store/tests/principals/actions.rs @@ -1,7 +1,7 @@ use core::{assert_matches, error::Error}; use hash_graph_authorization::policies::action::ActionName; -use hash_graph_postgres_store::permissions::ActionError; +use hash_graph_postgres_store::{permissions::ActionError, store::Context as _}; use crate::DatabaseTestWrapper; diff --git a/libs/@local/graph/postgres-store/tests/principals/main.rs b/libs/@local/graph/postgres-store/tests/principals/main.rs index c60eb0e8e34..91f87082f92 100644 --- a/libs/@local/graph/postgres-store/tests/principals/main.rs +++ b/libs/@local/graph/postgres-store/tests/principals/main.rs @@ -21,7 +21,9 @@ use hash_graph_authorization::policies::{ principal::PrincipalConstraint, store::{PolicyCreationParams, PolicyStore as _, PrincipalStore as _}, }; -use hash_graph_postgres_store::store::{PostgresStore, error::StoreError}; +use hash_graph_postgres_store::store::{ + Context as _, InTransaction, PostgresStore, error::StoreError, +}; use tokio_postgres::Transaction; use type_system::principal::actor::ActorId; @@ -30,7 +32,7 @@ pub use crate::common::DatabaseTestWrapper; impl DatabaseTestWrapper { pub(crate) async fn seed( &mut self, - ) -> Result<(PostgresStore>, ActorId), Report> { + ) -> Result<(PostgresStore, InTransaction>, ActorId), Report> { let mut transaction = self.connection.transaction().await?; transaction diff --git a/libs/@local/graph/postgres-store/tests/principals/policies.rs b/libs/@local/graph/postgres-store/tests/principals/policies.rs index 49039a07004..5a54b5ac32b 100644 --- a/libs/@local/graph/postgres-store/tests/principals/policies.rs +++ b/libs/@local/graph/postgres-store/tests/principals/policies.rs @@ -12,7 +12,7 @@ use hash_graph_authorization::policies::{ ResolvePoliciesParams, }, }; -use hash_graph_postgres_store::store::{AsClient, PostgresStore}; +use hash_graph_postgres_store::store::{AsClient, PostgresStore, TransactionState}; use pretty_assertions::assert_eq; use type_system::principal::{ actor::{ActorId, ActorType, AiId, MachineId, UserId}, @@ -140,7 +140,7 @@ struct TestPolicyIds { /// ``` #[expect(clippy::too_many_lines)] async fn setup_policy_test_environment( - client: &mut PostgresStore, + client: &mut PostgresStore, actor_id: ActorId, ) -> Result> { // Create web teams (top level) diff --git a/libs/@local/graph/store/docs/dependency-diagram.mmd b/libs/@local/graph/store/docs/dependency-diagram.mmd index c4dd3693f57..0ff005c70d2 100644 --- a/libs/@local/graph/store/docs/dependency-diagram.mmd +++ b/libs/@local/graph/store/docs/dependency-diagram.mmd @@ -48,6 +48,7 @@ graph TD 5 --> 1 6 --> 11 7 --> 6 + 7 -.-> 7 7 --> 12 8 --> 5 8 --> 11 diff --git a/libs/@local/graph/store/src/entity/store.rs b/libs/@local/graph/store/src/entity/store.rs index ca2a0b1458a..64b297c68ec 100644 --- a/libs/@local/graph/store/src/entity/store.rs +++ b/libs/@local/graph/store/src/entity/store.rs @@ -809,33 +809,52 @@ pub trait EntityStore { /// Get a list of entities specified by the [`QueryEntitiesParams`]. /// + /// The whole read is expected to be evaluated against a single consistent snapshot of the + /// store, so a concurrent write cannot cause the returned entities, their resolved entity + /// types, and the permission information to reflect different states of the store. + /// Implementations backed by a database should run all statements of the read inside a + /// single read-only transaction with an isolation level of at least repeatable read. This + /// requires mutable access to the store to begin the transaction. + /// /// # Errors /// /// - if the requested [`Entities`][Entity] cannot be retrieved fn query_entities( - &self, + &mut self, actor_id: ActorEntityUuid, params: QueryEntitiesParams<'_>, ) -> impl Future, Report>> + Send; /// Searches for entities by embedding similarity, ordered by ascending cosine distance. /// + /// The underlying entity read runs against a single consistent snapshot of the store; see + /// [`query_entities`] for details. This requires mutable access to the store. + /// /// # Errors /// /// - if the requested [`Entities`][Entity] cannot be retrieved + /// + /// [`query_entities`]: Self::query_entities fn search_entities( - &self, + &mut self, actor_id: ActorEntityUuid, params: SearchEntitiesParams, ) -> impl Future>> + Send; /// Get the [`Subgraph`]s specified by the [`QueryEntitySubgraphParams`]. /// + /// The whole subgraph read is expected to be evaluated against a single consistent snapshot + /// of the store, so a concurrent write cannot cause the returned subgraph to contain a link + /// entity without the edge to its endpoint. Implementations backed by a database should run + /// all statements of the read inside a single read-only transaction with an isolation level + /// of at least repeatable read. This requires mutable access to the store to begin the + /// transaction. + /// /// # Errors /// /// - if the requested [`Entities`][Entity] cannot be retrieved fn query_entity_subgraph( - &self, + &mut self, actor_id: ActorEntityUuid, params: QueryEntitySubgraphParams<'_>, ) -> impl Future, Report>> + Send; diff --git a/libs/@local/graph/temporal-versioning/docs/dependency-diagram.mmd b/libs/@local/graph/temporal-versioning/docs/dependency-diagram.mmd index a6593b53518..6963312e885 100644 --- a/libs/@local/graph/temporal-versioning/docs/dependency-diagram.mmd +++ b/libs/@local/graph/temporal-versioning/docs/dependency-diagram.mmd @@ -48,6 +48,7 @@ graph TD 5 --> 1 6 --> 11 7 --> 6 + 7 -.-> 7 7 --> 12 8 --> 5 8 --> 11 diff --git a/libs/@local/graph/test-server/docs/dependency-diagram.mmd b/libs/@local/graph/test-server/docs/dependency-diagram.mmd index 713da84c791..6ff0a311595 100644 --- a/libs/@local/graph/test-server/docs/dependency-diagram.mmd +++ b/libs/@local/graph/test-server/docs/dependency-diagram.mmd @@ -64,7 +64,7 @@ graph TD 5 --> 1 6 --> 7 6 --> 34 - 8 -.-> 6 + 8 --> 6 8 --> 15 8 --> 33 9 --> 5 diff --git a/libs/@local/graph/type-fetcher/src/store.rs b/libs/@local/graph/type-fetcher/src/store.rs index 87a124b74b0..76d48312de5 100644 --- a/libs/@local/graph/type-fetcher/src/store.rs +++ b/libs/@local/graph/type-fetcher/src/store.rs @@ -1635,7 +1635,7 @@ where } async fn query_entities( - &self, + &mut self, actor_id: ActorEntityUuid, params: QueryEntitiesParams<'_>, ) -> Result, Report> { @@ -1643,7 +1643,7 @@ where } async fn search_entities( - &self, + &mut self, actor_id: ActorEntityUuid, params: SearchEntitiesParams, ) -> Result> { @@ -1651,7 +1651,7 @@ where } async fn query_entity_subgraph( - &self, + &mut self, actor_id: ActorEntityUuid, params: QueryEntitySubgraphParams<'_>, ) -> Result, Report> { diff --git a/libs/@local/graph/types/docs/dependency-diagram.mmd b/libs/@local/graph/types/docs/dependency-diagram.mmd index dfec2861ca5..b7440a68417 100644 --- a/libs/@local/graph/types/docs/dependency-diagram.mmd +++ b/libs/@local/graph/types/docs/dependency-diagram.mmd @@ -48,6 +48,7 @@ graph TD 5 --> 1 6 --> 11 7 --> 6 + 7 -.-> 7 7 --> 12 8 --> 5 8 --> 11 diff --git a/libs/@local/graph/validation/docs/dependency-diagram.mmd b/libs/@local/graph/validation/docs/dependency-diagram.mmd index a84b3c63ba9..709424e039e 100644 --- a/libs/@local/graph/validation/docs/dependency-diagram.mmd +++ b/libs/@local/graph/validation/docs/dependency-diagram.mmd @@ -41,6 +41,7 @@ graph TD 4 --> 15 4 --> 18 5 --> 1 + 6 -.-> 6 6 --> 10 7 --> 5 7 --> 9 diff --git a/libs/@local/harpc/types/docs/dependency-diagram.mmd b/libs/@local/harpc/types/docs/dependency-diagram.mmd index e19512e8d9f..771604da331 100644 --- a/libs/@local/harpc/types/docs/dependency-diagram.mmd +++ b/libs/@local/harpc/types/docs/dependency-diagram.mmd @@ -51,6 +51,7 @@ graph TD 4 --> 1 5 --> 10 6 --> 5 + 6 -.-> 6 6 --> 11 7 --> 4 7 --> 10 diff --git a/libs/@local/harpc/wire-protocol/docs/dependency-diagram.mmd b/libs/@local/harpc/wire-protocol/docs/dependency-diagram.mmd index 4b9d3502f7b..b8a6e9452d1 100644 --- a/libs/@local/harpc/wire-protocol/docs/dependency-diagram.mmd +++ b/libs/@local/harpc/wire-protocol/docs/dependency-diagram.mmd @@ -51,6 +51,7 @@ graph TD 4 --> 1 5 --> 10 6 --> 5 + 6 -.-> 6 6 --> 11 7 --> 4 7 --> 10 diff --git a/libs/@local/hashql/ast/docs/dependency-diagram.mmd b/libs/@local/hashql/ast/docs/dependency-diagram.mmd index 49b505b3f35..b0e9386631a 100644 --- a/libs/@local/hashql/ast/docs/dependency-diagram.mmd +++ b/libs/@local/hashql/ast/docs/dependency-diagram.mmd @@ -57,7 +57,8 @@ graph TD 7 --> 8 7 --> 26 9 --> 6 - 9 -.-> 7 + 9 --> 7 + 9 -.-> 9 9 --> 13 9 --> 25 10 --> 5 diff --git a/libs/@local/hashql/compiletest/docs/dependency-diagram.mmd b/libs/@local/hashql/compiletest/docs/dependency-diagram.mmd index 9a2ec12d921..1651fc83c73 100644 --- a/libs/@local/hashql/compiletest/docs/dependency-diagram.mmd +++ b/libs/@local/hashql/compiletest/docs/dependency-diagram.mmd @@ -57,7 +57,8 @@ graph TD 7 --> 8 7 --> 26 9 --> 6 - 9 -.-> 7 + 9 --> 7 + 9 -.-> 9 9 --> 13 9 --> 25 10 --> 5 diff --git a/libs/@local/hashql/eval/docs/dependency-diagram.mmd b/libs/@local/hashql/eval/docs/dependency-diagram.mmd index 0817d44f171..05c27cffadb 100644 --- a/libs/@local/hashql/eval/docs/dependency-diagram.mmd +++ b/libs/@local/hashql/eval/docs/dependency-diagram.mmd @@ -57,7 +57,8 @@ graph TD 7 --> 8 7 --> 26 9 --> 6 - 9 -.-> 7 + 9 --> 7 + 9 -.-> 9 9 --> 13 9 --> 25 10 --> 5 diff --git a/libs/@local/hashql/hir/docs/dependency-diagram.mmd b/libs/@local/hashql/hir/docs/dependency-diagram.mmd index d0f49158af5..f4e054a501d 100644 --- a/libs/@local/hashql/hir/docs/dependency-diagram.mmd +++ b/libs/@local/hashql/hir/docs/dependency-diagram.mmd @@ -57,7 +57,8 @@ graph TD 7 --> 8 7 --> 26 9 --> 6 - 9 -.-> 7 + 9 --> 7 + 9 -.-> 9 9 --> 13 9 --> 25 10 --> 5 diff --git a/libs/@local/hashql/mir/docs/dependency-diagram.mmd b/libs/@local/hashql/mir/docs/dependency-diagram.mmd index 08cc3a23141..710bdc515be 100644 --- a/libs/@local/hashql/mir/docs/dependency-diagram.mmd +++ b/libs/@local/hashql/mir/docs/dependency-diagram.mmd @@ -57,7 +57,8 @@ graph TD 7 --> 8 7 --> 26 9 --> 6 - 9 -.-> 7 + 9 --> 7 + 9 -.-> 9 9 --> 13 9 --> 25 10 --> 5 diff --git a/libs/@local/hashql/syntax-jexpr/docs/dependency-diagram.mmd b/libs/@local/hashql/syntax-jexpr/docs/dependency-diagram.mmd index 659be40ce02..b877d4884cc 100644 --- a/libs/@local/hashql/syntax-jexpr/docs/dependency-diagram.mmd +++ b/libs/@local/hashql/syntax-jexpr/docs/dependency-diagram.mmd @@ -57,7 +57,8 @@ graph TD 7 --> 8 7 --> 26 9 --> 6 - 9 -.-> 7 + 9 --> 7 + 9 -.-> 9 9 --> 13 9 --> 25 10 --> 5 diff --git a/libs/@local/status/rust/docs/dependency-diagram.mmd b/libs/@local/status/rust/docs/dependency-diagram.mmd index a9df8779a27..75e5f4c7662 100644 --- a/libs/@local/status/rust/docs/dependency-diagram.mmd +++ b/libs/@local/status/rust/docs/dependency-diagram.mmd @@ -26,6 +26,7 @@ graph TD 1 --> 3 1 --> 6 1 --> 9 + 2 -.-> 2 2 --> 10 3 --> 10 4 -.-> 5 diff --git a/libs/@local/telemetry/docs/dependency-diagram.mmd b/libs/@local/telemetry/docs/dependency-diagram.mmd index 42bf020869c..4c77fc340ad 100644 --- a/libs/@local/telemetry/docs/dependency-diagram.mmd +++ b/libs/@local/telemetry/docs/dependency-diagram.mmd @@ -28,7 +28,8 @@ graph TD 1 --> 6 1 --> 9 2 --> 11 - 3 -.-> 2 + 3 --> 2 + 3 -.-> 3 4 -.-> 5 5 --> 6 5 --> 9 diff --git a/libs/@local/temporal-client/docs/dependency-diagram.mmd b/libs/@local/temporal-client/docs/dependency-diagram.mmd index 3a4af027246..b5b858cf502 100644 --- a/libs/@local/temporal-client/docs/dependency-diagram.mmd +++ b/libs/@local/temporal-client/docs/dependency-diagram.mmd @@ -48,6 +48,7 @@ graph TD 5 --> 1 6 --> 11 7 --> 6 + 7 -.-> 7 7 --> 12 8 --> 5 8 --> 11 diff --git a/tests/graph/benches/Cargo.toml b/tests/graph/benches/Cargo.toml index a27c06a06de..7d7f6ad3840 100644 --- a/tests/graph/benches/Cargo.toml +++ b/tests/graph/benches/Cargo.toml @@ -14,10 +14,12 @@ autobenches = false [dev-dependencies] # Private workspace dependencies -error-stack = { workspace = true } -hash-graph-api = { workspace = true } -hash-graph-authorization = { workspace = true } -hash-graph-postgres-store = { workspace = true } +error-stack = { workspace = true } +hash-graph-api = { workspace = true } +hash-graph-authorization = { workspace = true } +# `test-utils` enables the `BeginReadOnlyTransaction` impl for `InTransaction` stores, which the +# transaction-wrapped seeding helpers rely on. See `TODO(BE-688)`. +hash-graph-postgres-store = { workspace = true, features = ["test-utils"] } hash-graph-store = { workspace = true } hash-graph-temporal-versioning = { workspace = true } hash-graph-test-data = { workspace = true } diff --git a/tests/graph/benches/graph/scenario/stages/entity_queries.rs b/tests/graph/benches/graph/scenario/stages/entity_queries.rs index ba9249ef0b8..8d55ef93163 100644 --- a/tests/graph/benches/graph/scenario/stages/entity_queries.rs +++ b/tests/graph/benches/graph/scenario/stages/entity_queries.rs @@ -102,7 +102,7 @@ impl QueryEntitiesByUserStage { .await .change_context(QueryEntitiesError::Acquire)?; - let store = pool + let mut store = pool .acquire(None) .await .change_context(QueryEntitiesError::Acquire)?; diff --git a/tests/graph/benches/manual_queries/entity_queries/mod.rs b/tests/graph/benches/manual_queries/entity_queries/mod.rs index 4153601d57b..1759f3e226c 100644 --- a/tests/graph/benches/manual_queries/entity_queries/mod.rs +++ b/tests/graph/benches/manual_queries/entity_queries/mod.rs @@ -1,4 +1,4 @@ -use core::{fmt, iter, mem}; +use core::{cell::RefCell, fmt, iter, mem}; use std::{collections::HashMap, env, ffi::OsStr, fs::File, io, path::Path}; use criterion::{BatchSize, BenchmarkId, Criterion}; @@ -303,7 +303,13 @@ fn read_groups(path: impl AsRef) -> Result, Repor .collect() } -async fn run_benchmark<'q, 's, 'p: 'q, S>(store: &S, request: GraphQuery<'q, 's, 'p>) +#[expect( + clippy::future_not_send, + clippy::await_holding_refcell_ref, + reason = "criterion drives one benchmark future at a time to completion on a single thread, \ + so the `RefCell` borrow is never contended and the future never crosses threads" +)] +async fn run_benchmark<'q, 's, 'p: 'q, S>(store: &RefCell, request: GraphQuery<'q, 's, 'p>) where S: EntityStore + Sync, { @@ -315,6 +321,7 @@ where match request { GraphQuery::QueryEntities(request) => { let _response = store + .borrow_mut() .query_entities( request.actor_id, request.request.into_params_unchecked(config, None), @@ -324,6 +331,7 @@ where } GraphQuery::QueryEntitySubgraph(request) => { let _response = store + .borrow_mut() .query_entity_subgraph( request.actor_id, request.request.into_traversal_params_unchecked(config), @@ -375,9 +383,13 @@ fn bench_json_queries(crit: &mut Criterion) { )) .expect("pool should be able to be created"); - let store = runtime - .block_on(pool.acquire(None)) - .expect("pool should be able to acquire store"); + // `query_entity_subgraph` takes `&mut self` to run the read in a single transaction; the + // `RefCell` provides the mutable borrow from within the benchmark closures. + let store = RefCell::new( + runtime + .block_on(pool.acquire(None)) + .expect("pool should be able to acquire store"), + ); for (query_type, requests) in groups { let group_id = query_type.to_string(); diff --git a/tests/graph/benches/policy/seed.rs b/tests/graph/benches/policy/seed.rs index a54a68da860..223b647bfc8 100644 --- a/tests/graph/benches/policy/seed.rs +++ b/tests/graph/benches/policy/seed.rs @@ -15,7 +15,9 @@ use hash_graph_authorization::policies::{ principal::PrincipalConstraint, store::{CreateWebParameter, PolicyCreationParams, PrincipalStore as _}, }; -use hash_graph_postgres_store::store::{AsClient, PostgresStore}; +use hash_graph_postgres_store::store::{ + AsClient, PostgresStore, Transaction as _, TransactionState, +}; use type_system::principal::{ actor::ActorId, actor_group::{ActorGroupId, TeamId, WebId}, @@ -110,7 +112,7 @@ pub async fn seed_benchmark_data( system_actor_id: ActorId, config: &SeedConfig, ) -> Result> { - let mut transaction = store.transaction().await?; + let mut transaction = store.begin_transaction().await?; let mut data = BenchmarkData::default(); @@ -341,7 +343,7 @@ pub async fn seed_benchmark_data( /// Creates nested team hierarchy that triggers recursive CTE performance issues. /// Returns (`all_teams`, `leaf_teams`) where `leaf_teams` are the deepest level teams. async fn create_team_hierarchy( - store: &mut PostgresStore, + store: &mut PostgresStore, web_id: WebId, root_teams: usize, max_depth: usize, diff --git a/tests/graph/benches/read_scaling/knowledge/complete/entity.rs b/tests/graph/benches/read_scaling/knowledge/complete/entity.rs index a3286da446b..699cf295191 100644 --- a/tests/graph/benches/read_scaling/knowledge/complete/entity.rs +++ b/tests/graph/benches/read_scaling/knowledge/complete/entity.rs @@ -1,4 +1,4 @@ -use core::{iter::repeat_n, str::FromStr as _}; +use core::{cell::RefCell, iter::repeat_n, str::FromStr as _}; use std::collections::HashSet; use criterion::{BatchSize::SmallInput, Bencher, BenchmarkId, Criterion}; @@ -6,6 +6,7 @@ use criterion_macro::criterion; use hash_graph_authorization::policies::store::{ CreateWebParameter, PolicyStore as _, PrincipalStore as _, }; +use hash_graph_postgres_store::store::{Context as _, Transaction as _}; use hash_graph_store::{ entity::{ CreateEntityParams, EntityQuerySorting, EntityStore as _, QueryEntitiesParams, @@ -217,10 +218,15 @@ async fn seed_db( } } +#[expect( + clippy::await_holding_refcell_ref, + reason = "criterion drives one benchmark future at a time to completion on a single thread, \ + so the `RefCell` borrow is never contended" +)] pub fn bench_get_entity_by_id( bencher: &mut Bencher, runtime: &Runtime, - store: &Store, + store: &RefCell<&mut Store>, actor_id: ActorEntityUuid, entity_metadata_list: &[Entity], traversal_params: &SubgraphTraversalParams, @@ -239,6 +245,7 @@ pub fn bench_get_entity_by_id( let traversal_params = traversal_params.clone(); async move { store + .borrow_mut() .query_entity_subgraph( actor_id, QueryEntitySubgraphParams::from_parts( @@ -288,7 +295,9 @@ fn bench_scaling_read_entity( entity_list: entity_metadata_list, .. } = runtime.block_on(seed_db(account_id, &mut store_wrapper, size)); - let store = &store_wrapper.store; + // `query_entity_subgraph` takes `&mut self` to run the read in a single transaction; + // the `RefCell` provides the mutable borrow from within the benchmark closures. + let store = RefCell::new(&mut *store_wrapper.store); let function_id = format!("entity_by_id;{name}"); let parameter = format!("{size} entities"); @@ -300,7 +309,7 @@ fn bench_scaling_read_entity( bench_get_entity_by_id( bencher, &runtime, - store, + &store, account_id, entity_list, traversal_params, diff --git a/tests/graph/benches/read_scaling/knowledge/linkless/entity.rs b/tests/graph/benches/read_scaling/knowledge/linkless/entity.rs index ed0d066362e..154831a9f6c 100644 --- a/tests/graph/benches/read_scaling/knowledge/linkless/entity.rs +++ b/tests/graph/benches/read_scaling/knowledge/linkless/entity.rs @@ -1,4 +1,4 @@ -use core::{iter::repeat_n, str::FromStr as _}; +use core::{cell::RefCell, iter::repeat_n, str::FromStr as _}; use std::collections::HashSet; use criterion::{BatchSize::SmallInput, Bencher, BenchmarkId, Criterion}; @@ -6,6 +6,7 @@ use criterion_macro::criterion; use hash_graph_authorization::policies::store::{ CreateWebParameter, PolicyStore as _, PrincipalStore as _, }; +use hash_graph_postgres_store::store::{Context as _, Transaction as _}; use hash_graph_store::{ entity::{CreateEntityParams, EntityQuerySorting, EntityStore as _, QueryEntitiesParams}, filter::Filter, @@ -155,10 +156,15 @@ async fn seed_db( entity_list } +#[expect( + clippy::await_holding_refcell_ref, + reason = "criterion drives one benchmark future at a time to completion on a single thread, \ + so the `RefCell` borrow is never contended" +)] pub fn bench_get_entity_by_id( bencher: &mut Bencher, runtime: &Runtime, - store: &Store, + store: &RefCell<&mut Store>, actor_id: ActorEntityUuid, entity_metadata_list: &[Entity], ) { @@ -174,6 +180,7 @@ pub fn bench_get_entity_by_id( }, |entity_record_id| async move { store + .borrow_mut() .query_entities( actor_id, QueryEntitiesParams { @@ -211,7 +218,9 @@ fn bench_scaling_read_entity(crit: &mut Criterion) { let (runtime, mut store_wrapper) = setup(DB_NAME, true, true, account_id); let entity_uuids = runtime.block_on(seed_db(account_id, &mut store_wrapper, size)); - let store = &store_wrapper.store; + // `query_entities` takes `&mut self` to run the read in a single transaction; the + // `RefCell` provides the mutable borrow from within the benchmark closures. + let store = RefCell::new(&mut *store_wrapper.store); let function_id = "entity_by_id"; let parameter = format!("{size} entities"); @@ -220,7 +229,7 @@ fn bench_scaling_read_entity(crit: &mut Criterion) { &(account_id, entity_uuids), |bencher, (_account_id, entity_list)| { let _guard = setup_subscriber(group_id, Some(function_id), Some(¶meter)); - bench_get_entity_by_id(bencher, &runtime, store, account_id, entity_list); + bench_get_entity_by_id(bencher, &runtime, &store, account_id, entity_list); }, ); } diff --git a/tests/graph/benches/representative_read/knowledge/entity.rs b/tests/graph/benches/representative_read/knowledge/entity.rs index f1fdaa7f644..719dee20dd9 100644 --- a/tests/graph/benches/representative_read/knowledge/entity.rs +++ b/tests/graph/benches/representative_read/knowledge/entity.rs @@ -1,4 +1,5 @@ use alloc::borrow::Cow; +use core::cell::RefCell; use criterion::{BatchSize::SmallInput, Bencher}; use hash_graph_store::{ @@ -18,10 +19,15 @@ use type_system::{knowledge::entity::id::EntityUuid, principal::actor::ActorEnti use crate::util::Store; +#[expect( + clippy::await_holding_refcell_ref, + reason = "criterion drives one benchmark future at a time to completion on a single thread, \ + so the `RefCell` borrow is never contended" +)] pub fn bench_get_entity_by_id( bencher: &mut Bencher, runtime: &Runtime, - store: &Store, + store: &RefCell<&mut Store>, actor_id: ActorEntityUuid, entity_uuids: &[EntityUuid], ) { @@ -35,6 +41,7 @@ pub fn bench_get_entity_by_id( }, |entity_uuid| async move { let response = store + .borrow_mut() .query_entities( actor_id, QueryEntitiesParams { @@ -67,10 +74,15 @@ pub fn bench_get_entity_by_id( ); } +#[expect( + clippy::await_holding_refcell_ref, + reason = "criterion drives one benchmark future at a time to completion on a single thread, \ + so the `RefCell` borrow is never contended" +)] pub fn bench_query_entities_by_property( bencher: &mut Bencher, runtime: &Runtime, - store: &Store, + store: &RefCell<&mut Store>, actor_id: ActorEntityUuid, traversal_params: &SubgraphTraversalParams, ) { @@ -91,6 +103,7 @@ pub fn bench_query_entities_by_property( }, ); let response = store + .borrow_mut() .query_entity_subgraph( actor_id, QueryEntitySubgraphParams::from_parts( @@ -117,10 +130,15 @@ pub fn bench_query_entities_by_property( }); } +#[expect( + clippy::await_holding_refcell_ref, + reason = "criterion drives one benchmark future at a time to completion on a single thread, \ + so the `RefCell` borrow is never contended" +)] pub fn bench_get_link_by_target_by_property( bencher: &mut Bencher, runtime: &Runtime, - store: &Store, + store: &RefCell<&mut Store>, actor_id: ActorEntityUuid, traversal_params: &SubgraphTraversalParams, ) { @@ -145,6 +163,7 @@ pub fn bench_get_link_by_target_by_property( }, ); let response = store + .borrow_mut() .query_entity_subgraph( actor_id, QueryEntitySubgraphParams::from_parts( diff --git a/tests/graph/benches/representative_read/lib.rs b/tests/graph/benches/representative_read/lib.rs index ecd72c84ee3..ec0ff582f63 100644 --- a/tests/graph/benches/representative_read/lib.rs +++ b/tests/graph/benches/representative_read/lib.rs @@ -43,7 +43,7 @@ mod ontology; mod seed; -use core::{iter, str::FromStr as _}; +use core::{cell::RefCell, iter, str::FromStr as _}; use criterion::{BenchmarkId, Criterion}; use criterion_macro::criterion; @@ -72,7 +72,9 @@ fn bench_representative_read_entity(crit: &mut Criterion) { let (runtime, mut store_wrapper) = setup(DB_NAME, false, false, account_id); let samples = runtime.block_on(setup_and_extract_samples(&mut store_wrapper, account_id)); - let store = &store_wrapper.store; + // `query_entities` takes `&mut self` to run the read in a single transaction; the `RefCell` + // provides the mutable borrow from within the benchmark closures. + let store = RefCell::new(&mut *store_wrapper.store); for (account_id, type_ids_and_entity_uuids) in samples.entities { for (entity_type_id, entity_uuids) in type_ids_and_entity_uuids { @@ -86,7 +88,7 @@ fn bench_representative_read_entity(crit: &mut Criterion) { knowledge::entity::bench_get_entity_by_id( bencher, &runtime, - store, + &store, account_id, entity_uuids, ); @@ -109,6 +111,10 @@ fn bench_representative_read_multiple_entities(crit: &mut Criterion) { let mut group = crit.benchmark_group(group_id); let (runtime, mut store_wrapper) = setup(DB_NAME, false, false, account_id); let _samples = runtime.block_on(setup_and_extract_samples(&mut store_wrapper, account_id)); + let account_id = store_wrapper.account_id; + // `query_entity_subgraph` takes `&mut self` to run the read in a single transaction; the + // `RefCell` provides the mutable borrow from within the benchmark closures. + let store = RefCell::new(&mut *store_wrapper.store); let traversal_params = [ SubgraphTraversalParams::Paths { @@ -248,8 +254,8 @@ fn bench_representative_read_multiple_entities(crit: &mut Criterion) { knowledge::entity::bench_query_entities_by_property( bencher, &runtime, - &store_wrapper.store, - store_wrapper.account_id, + &store, + account_id, traversal_params, ); }, @@ -267,8 +273,8 @@ fn bench_representative_read_multiple_entities(crit: &mut Criterion) { knowledge::entity::bench_get_link_by_target_by_property( bencher, &runtime, - &store_wrapper.store, - store_wrapper.account_id, + &store, + account_id, traversal_params, ); }, diff --git a/tests/graph/benches/representative_read/seed.rs b/tests/graph/benches/representative_read/seed.rs index a8f32b957b9..13f61d321b3 100644 --- a/tests/graph/benches/representative_read/seed.rs +++ b/tests/graph/benches/representative_read/seed.rs @@ -4,7 +4,7 @@ use std::collections::{HashMap, HashSet, hash_map::Entry}; use hash_graph_authorization::policies::store::{ CreateWebParameter, PolicyStore as _, PrincipalStore as _, }; -use hash_graph_postgres_store::store::AsClient as _; +use hash_graph_postgres_store::store::{AsClient as _, Context as _, Transaction as _}; use hash_graph_store::entity::{CreateEntityParams, EntityStore as _}; use hash_graph_test_data::{data_type, entity, entity_type, property_type}; use tracing::Instrument as _; diff --git a/tests/graph/benches/util.rs b/tests/graph/benches/util.rs index a2e84b66534..807db8f02ee 100644 --- a/tests/graph/benches/util.rs +++ b/tests/graph/benches/util.rs @@ -7,7 +7,7 @@ use hash_graph_postgres_store::{ Environment, load_env, store::{ AsClient, DatabaseConnectionInfo, DatabasePoolConfig, DatabaseType, PostgresStore, - PostgresStorePool, PostgresStoreSettings, + PostgresStorePool, PostgresStoreSettings, TransactionState, }, }; use hash_graph_store::{ @@ -286,8 +286,8 @@ impl Drop for StoreWrapper { } } -pub async fn seed( - store: &mut PostgresStore, +pub async fn seed( + store: &mut PostgresStore, account_id: ActorEntityUuid, data_types: D, property_types: P, @@ -297,6 +297,7 @@ pub async fn seed( P: IntoIterator + Send, E: IntoIterator + Send, C: AsClient, + S: TransactionState, { let domain_regex = Regex::new( &std::env::var("HASH_GRAPH_ALLOWED_URL_DOMAIN_PATTERN") diff --git a/tests/graph/integration/Cargo.toml b/tests/graph/integration/Cargo.toml index da1d6be507f..8943b95e193 100644 --- a/tests/graph/integration/Cargo.toml +++ b/tests/graph/integration/Cargo.toml @@ -8,10 +8,12 @@ authors.workspace = true [dev-dependencies] # Private workspace dependencies -error-stack = { workspace = true, features = ["spantrace"] } -hash-codec = { workspace = true, features = ["numeric"] } -hash-graph-authorization = { workspace = true } -hash-graph-postgres-store = { workspace = true } +error-stack = { workspace = true, features = ["spantrace"] } +hash-codec = { workspace = true, features = ["numeric"] } +hash-graph-authorization = { workspace = true } +# `test-utils` enables the `BeginReadOnlyTransaction` impl for `InTransaction` stores, which the +# rollback-isolation harness in `postgres/lib.rs` relies on. See `TODO(BE-688)`. +hash-graph-postgres-store = { workspace = true, features = ["test-utils"] } hash-graph-store = { workspace = true } hash-graph-temporal-versioning = { workspace = true } hash-graph-test-data = { workspace = true } diff --git a/tests/graph/integration/postgres/email_filter_protection.rs b/tests/graph/integration/postgres/email_filter_protection.rs index fec507509a1..8425a47bb56 100644 --- a/tests/graph/integration/postgres/email_filter_protection.rs +++ b/tests/graph/integration/postgres/email_filter_protection.rs @@ -374,11 +374,11 @@ impl DatabaseApi<'_> { } async fn query( - &self, + &mut self, filter: Filter<'_, Entity>, sorting: EntityQuerySorting<'static>, ) -> Vec { - self.query_entities( + Box::pin(self.query_entities( self.account_id, hash_graph_store::entity::QueryEntitiesParams { filter, @@ -390,7 +390,7 @@ impl DatabaseApi<'_> { include_drafts: false, include_permissions: false, }, - ) + )) .await .expect("query failed") .entities diff --git a/tests/graph/integration/postgres/lib.rs b/tests/graph/integration/postgres/lib.rs index 59053b59abb..c92eb3082a9 100644 --- a/tests/graph/integration/postgres/lib.rs +++ b/tests/graph/integration/postgres/lib.rs @@ -20,6 +20,7 @@ mod property_metadata; mod property_type; mod read_only; mod sorting; +mod transaction; use std::collections::{HashMap, HashSet}; @@ -31,8 +32,8 @@ use hash_graph_authorization::policies::{ use hash_graph_postgres_store::{ Environment, load_env, store::{ - DatabaseConnectionInfo, DatabasePoolConfig, DatabaseType, PostgresStore, PostgresStorePool, - PostgresStoreSettings, + Context as _, DatabaseConnectionInfo, DatabasePoolConfig, DatabaseType, InTransaction, + PostgresStore, PostgresStorePool, PostgresStoreSettings, }, }; use hash_graph_store::{ @@ -99,7 +100,7 @@ pub struct DatabaseTestWrapper { } pub struct DatabaseApi<'pool> { - store: PostgresStore>, + store: PostgresStore, InTransaction>, account_id: ActorEntityUuid, } @@ -820,7 +821,7 @@ impl EntityStore for DatabaseApi<'_> { } async fn query_entities( - &self, + &mut self, actor_id: ActorEntityUuid, params: QueryEntitiesParams<'_>, ) -> Result, Report> { @@ -828,7 +829,7 @@ impl EntityStore for DatabaseApi<'_> { } async fn search_entities( - &self, + &mut self, actor_id: ActorEntityUuid, params: SearchEntitiesParams, ) -> Result> { @@ -836,7 +837,7 @@ impl EntityStore for DatabaseApi<'_> { } async fn query_entity_subgraph( - &self, + &mut self, actor_id: ActorEntityUuid, params: QueryEntitySubgraphParams<'_>, ) -> Result, Report> { diff --git a/tests/graph/integration/postgres/sorting.rs b/tests/graph/integration/postgres/sorting.rs index a545e80bc5a..9ed8dd07728 100644 --- a/tests/graph/integration/postgres/sorting.rs +++ b/tests/graph/integration/postgres/sorting.rs @@ -30,17 +30,17 @@ use uuid::Uuid; use crate::{DatabaseApi, DatabaseTestWrapper}; async fn test_root_sorting_chunked( - api: &DatabaseApi<'_>, + api: &mut DatabaseApi<'_>, sort: [(EntityQueryPath<'static>, Ordering, NullOrdering); N], expected_order: [PropertyObject; M], ) { for chunk_size in 0..expected_order.len() { - test_root_sorting(api, chunk_size + 1, sort.clone(), &expected_order).await; + test_root_sorting(&mut *api, chunk_size + 1, sort.clone(), &expected_order).await; } } async fn test_root_sorting( - api: &DatabaseApi<'_>, + api: &mut DatabaseApi<'_>, chunk_size: usize, sort: impl IntoIterator, Ordering, NullOrdering)> + Send, expected_order: impl IntoIterator + Send, @@ -53,6 +53,7 @@ async fn test_root_sorting( nulls: Some(nulls), }) .collect::>(); + let actor_id = api.account_id; let mut cursor = None; let mut found_entities = HashSet::new(); @@ -66,7 +67,7 @@ async fn test_root_sorting( definitions: _, entity_permissions: _, } = Box::pin(api.query_entity_subgraph( - api.account_id, + actor_id, QueryEntitySubgraphParams::Paths { traversal_paths: Vec::new(), request: QueryEntitiesParams { @@ -246,10 +247,10 @@ fn page_v2() -> PropertyObject { #[tokio::test] async fn uuid_ascending() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [( EntityQueryPath::Uuid, Ordering::Ascending, @@ -263,10 +264,10 @@ async fn uuid_ascending() { #[tokio::test] async fn uuid_descending() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [( EntityQueryPath::Uuid, Ordering::Descending, @@ -280,10 +281,10 @@ async fn uuid_descending() { #[tokio::test] async fn age_ascending_last() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ (age_property_path(), Ordering::Ascending, NullOrdering::Last), ( @@ -300,10 +301,10 @@ async fn age_ascending_last() { #[tokio::test] async fn age_ascending_first() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ ( age_property_path(), @@ -324,9 +325,9 @@ async fn age_ascending_first() { #[tokio::test] async fn age_descending_last() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ ( age_property_path(), @@ -347,9 +348,9 @@ async fn age_descending_last() { #[tokio::test] async fn age_descending_first() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ ( age_property_path(), @@ -370,9 +371,9 @@ async fn age_descending_first() { #[tokio::test] async fn age_ascending_last_name_ascending_last() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ (age_property_path(), Ordering::Ascending, NullOrdering::Last), ( @@ -394,9 +395,9 @@ async fn age_ascending_last_name_ascending_last() { #[tokio::test] async fn age_ascending_last_name_ascending_first() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ (age_property_path(), Ordering::Ascending, NullOrdering::Last), ( @@ -418,9 +419,9 @@ async fn age_ascending_last_name_ascending_first() { #[tokio::test] async fn age_ascending_first_name_ascending_last() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ ( age_property_path(), @@ -446,9 +447,9 @@ async fn age_ascending_first_name_ascending_last() { #[tokio::test] async fn age_ascending_first_name_ascending_first() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ ( age_property_path(), @@ -474,9 +475,9 @@ async fn age_ascending_first_name_ascending_first() { #[tokio::test] async fn age_ascending_last_name_descending_last() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ (age_property_path(), Ordering::Ascending, NullOrdering::Last), ( @@ -498,9 +499,9 @@ async fn age_ascending_last_name_descending_last() { #[tokio::test] async fn age_ascending_first_name_descending_last() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ ( age_property_path(), @@ -526,9 +527,9 @@ async fn age_ascending_first_name_descending_last() { #[tokio::test] async fn age_ascending_last_name_descending_first() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ (age_property_path(), Ordering::Ascending, NullOrdering::Last), ( @@ -550,9 +551,9 @@ async fn age_ascending_last_name_descending_first() { #[tokio::test] async fn age_ascending_first_name_descending_first() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ ( age_property_path(), @@ -578,9 +579,9 @@ async fn age_ascending_first_name_descending_first() { #[tokio::test] async fn age_descending_last_name_ascending_last() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ ( age_property_path(), @@ -606,9 +607,9 @@ async fn age_descending_last_name_ascending_last() { #[tokio::test] async fn age_descending_first_name_ascending_last() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ ( age_property_path(), @@ -634,9 +635,9 @@ async fn age_descending_first_name_ascending_last() { #[tokio::test] async fn age_descending_last_name_ascending_first() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ ( age_property_path(), @@ -662,9 +663,9 @@ async fn age_descending_last_name_ascending_first() { #[tokio::test] async fn age_descending_first_name_ascending_first() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ ( age_property_path(), @@ -690,9 +691,9 @@ async fn age_descending_first_name_ascending_first() { #[tokio::test] async fn age_descending_last_name_descending_last() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ ( age_property_path(), @@ -718,9 +719,9 @@ async fn age_descending_last_name_descending_last() { #[tokio::test] async fn age_descending_first_name_descending_last() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ ( age_property_path(), @@ -746,9 +747,9 @@ async fn age_descending_first_name_descending_last() { #[tokio::test] async fn age_descending_last_name_descending_first() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ ( age_property_path(), @@ -774,9 +775,9 @@ async fn age_descending_last_name_descending_first() { #[tokio::test] async fn age_descending_first_name_descending_first() { let mut database = DatabaseTestWrapper::new().await; - let api = insert(&mut database).await; + let mut api = insert(&mut database).await; test_root_sorting_chunked( - &api, + &mut api, [ ( age_property_path(), diff --git a/tests/graph/integration/postgres/transaction.rs b/tests/graph/integration/postgres/transaction.rs new file mode 100644 index 00000000000..085e472ee8b --- /dev/null +++ b/tests/graph/integration/postgres/transaction.rs @@ -0,0 +1,107 @@ +use hash_graph_postgres_store::store::{ + AsClient as _, Context as _, IsolationLevel, Transaction as _, TransactionBuilder as _, +}; + +use crate::DatabaseTestWrapper; + +/// The transaction builder composes all options into the single `BEGIN` statement, so the +/// transaction's characteristics are in effect from the very first statement. +#[tokio::test] +async fn transaction_builder_composes_options_into_begin() { + let mut database = DatabaseTestWrapper::new().await; + + let transaction = database + .connection + .transaction() + .isolation_level(IsolationLevel::RepeatableRead) + .read_only() + .await + .expect("should be able to begin a transaction"); + + let row = transaction + .as_client() + .query_one( + "SELECT current_setting('transaction_isolation'), \ + current_setting('transaction_read_only');", + &[], + ) + .await + .expect("should be able to read the transaction characteristics"); + + assert_eq!(row.get::<_, String>(0), "repeatable read"); + assert_eq!(row.get::<_, String>(1), "on"); + + transaction + .rollback() + .await + .expect("should be able to roll back the transaction"); +} + +/// Without options the builder behaves like the previous plain `transaction()` method. +#[tokio::test] +async fn transaction_builder_defaults_to_session_characteristics() { + let mut database = DatabaseTestWrapper::new().await; + + let transaction = database + .connection + .transaction() + .await + .expect("should be able to begin a transaction"); + + let row = transaction + .as_client() + .query_one("SELECT current_setting('transaction_read_only');", &[]) + .await + .expect("should be able to read the transaction characteristics"); + + assert_eq!(row.get::<_, String>(0), "off"); + + transaction + .rollback() + .await + .expect("should be able to roll back the transaction"); +} + +/// Beginning a transaction on a store which is already inside a transaction creates a +/// savepoint. A savepoint has no configurable characteristics of its own: it runs within the +/// enclosing transaction and inherits its characteristics. Requesting an isolation level or +/// read-only access on a nested transaction is a compile error, as those options only exist on +/// stores in the `NoTransaction` state. +#[tokio::test] +async fn nested_transactions_are_savepoints() { + let mut database = DatabaseTestWrapper::new().await; + + let mut outer = database + .connection + .transaction() + .await + .expect("should be able to begin a transaction"); + + let inner = outer + .begin_transaction() + .await + .expect("should be able to begin a nested transaction"); + + let row = inner + .as_client() + .query_one( + "SELECT current_setting('transaction_isolation'), \ + current_setting('transaction_read_only');", + &[], + ) + .await + .expect("should be able to read the transaction characteristics"); + + // The savepoint runs within the enclosing transaction and inherits its characteristics. + assert_eq!(row.get::<_, String>(0), "read committed"); + assert_eq!(row.get::<_, String>(1), "off"); + + inner + .rollback() + .await + .expect("should be able to roll back the nested transaction"); + outer + .rollback() + .await + .expect("should be able to roll back the transaction"); +} diff --git a/tests/graph/test-data/rust/docs/dependency-diagram.mmd b/tests/graph/test-data/rust/docs/dependency-diagram.mmd index 0390665dcd2..4ec8f634ee5 100644 --- a/tests/graph/test-data/rust/docs/dependency-diagram.mmd +++ b/tests/graph/test-data/rust/docs/dependency-diagram.mmd @@ -48,6 +48,7 @@ graph TD 5 --> 1 6 --> 11 7 --> 6 + 7 -.-> 7 7 --> 12 8 --> 5 8 --> 11