From 40fcb6d622de0d5dac6ddc8c0e1735d9fc608424 Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Mon, 7 Sep 2026 10:21:37 +0200 Subject: [PATCH 01/22] Test relation column paths --- crates/dbkit/tests/derive_ui.rs | 7 + .../dbkit/tests/integration_relation_paths.rs | 468 ++++++++++++++++++ crates/dbkit/tests/relation_paths_sql.rs | 405 +++++++++++++++ .../tests/support/relation_path_graphs.rs | 26 + crates/dbkit/tests/support/relation_paths.rs | 36 ++ .../ui/fail_relation_paths_assignment.rs | 11 + .../ui/fail_relation_paths_nullability.rs | 12 + .../ui/fail_relation_paths_unknown_field.rs | 10 + .../ui/fail_relation_paths_value_types.rs | 11 + crates/dbkit/tests/ui/pass_relation_paths.rs | 65 +++ .../ui/pass_relation_paths_field_names.rs | 39 ++ .../tests/ui/pass_relation_paths_graphs.rs | 42 ++ 12 files changed, 1132 insertions(+) create mode 100644 crates/dbkit/tests/integration_relation_paths.rs create mode 100644 crates/dbkit/tests/relation_paths_sql.rs create mode 100644 crates/dbkit/tests/support/relation_path_graphs.rs create mode 100644 crates/dbkit/tests/support/relation_paths.rs create mode 100644 crates/dbkit/tests/ui/fail_relation_paths_assignment.rs create mode 100644 crates/dbkit/tests/ui/fail_relation_paths_nullability.rs create mode 100644 crates/dbkit/tests/ui/fail_relation_paths_unknown_field.rs create mode 100644 crates/dbkit/tests/ui/fail_relation_paths_value_types.rs create mode 100644 crates/dbkit/tests/ui/pass_relation_paths.rs create mode 100644 crates/dbkit/tests/ui/pass_relation_paths_field_names.rs create mode 100644 crates/dbkit/tests/ui/pass_relation_paths_graphs.rs diff --git a/crates/dbkit/tests/derive_ui.rs b/crates/dbkit/tests/derive_ui.rs index a547799..956e1a5 100644 --- a/crates/dbkit/tests/derive_ui.rs +++ b/crates/dbkit/tests/derive_ui.rs @@ -69,6 +69,13 @@ fn main() -> ui_test::color_eyre::Result<()> { "fail_column_nullability_comparison_types.rs".into(), "pass_relation_state_into_generic_names.rs".into(), "pass_relation_state_into_shadowed_traits.rs".into(), + "pass_relation_paths.rs".into(), + "pass_relation_paths_graphs.rs".into(), + "pass_relation_paths_field_names.rs".into(), + "fail_relation_paths_value_types.rs".into(), + "fail_relation_paths_nullability.rs".into(), + "fail_relation_paths_assignment.rs".into(), + "fail_relation_paths_unknown_field.rs".into(), "fail_appended_model_type.rs".into(), "pass_model_suffix_preserved_api.rs".into(), "fail_model_suffix_double_model_type.rs".into(), diff --git a/crates/dbkit/tests/integration_relation_paths.rs b/crates/dbkit/tests/integration_relation_paths.rs new file mode 100644 index 0000000..adc1340 --- /dev/null +++ b/crates/dbkit/tests/integration_relation_paths.rs @@ -0,0 +1,468 @@ +#[path = "support/relation_path_graphs.rs"] +mod relation_path_graphs; +#[path = "support/relation_paths.rs"] +mod relation_paths; + +use dbkit::sqlx::postgres::PgArguments; +use dbkit::{func, Database, Error, Executor, NotLoaded, Order, SelectExt}; +use relation_path_graphs::{Assignment, Node}; +use relation_paths::{Member, Organization, Record}; + +fn db_url() -> String { + let _ = dotenvy::dotenv(); + std::env::var("DB_URL") + .or_else(|_| std::env::var("DATABASE_URL")) + .expect("DB_URL or DATABASE_URL must be set for integration tests") +} + +async fn setup(ex: &(impl Executor + Send + Sync)) -> Result<(), Error> { + // Temporary tables and a rolled-back transaction isolate every test. Missing + // targets are deliberate: model annotations alone do not enforce foreign keys. + for sql in [ + "CREATE TEMP TABLE path_organizations (id BIGINT PRIMARY KEY, label TEXT NOT NULL) ON COMMIT DROP", + "CREATE TEMP TABLE path_members ( + id BIGINT PRIMARY KEY, organization_id BIGINT NOT NULL, label TEXT NOT NULL, + enabled BOOLEAN NOT NULL, score INTEGER NOT NULL, note TEXT, + external_ref TEXT NOT NULL UNIQUE) ON COMMIT DROP", + "CREATE TEMP TABLE path_records ( + id BIGINT PRIMARY KEY, owner_id BIGINT, label TEXT NOT NULL, enabled BOOLEAN NOT NULL) ON COMMIT DROP", + "CREATE TEMP TABLE path_assignments ( + id BIGINT PRIMARY KEY, first_id BIGINT, second_code TEXT) ON COMMIT DROP", + "CREATE TEMP TABLE path_nodes ( + id BIGINT PRIMARY KEY, parent_id BIGINT, label TEXT NOT NULL) ON COMMIT DROP", + "INSERT INTO path_organizations VALUES (1, 'north'), (2, 'south')", + "INSERT INTO path_members VALUES + (1, 1, 'Atlas', true, 30, NULL, 'a'), + (2, 2, 'Birch', false, 10, 'memo', 'b'), + (3, 2, 'Cedar', true, 20, '', 'c'), + (4, 99, 'Delta', false, 5, NULL, 'd')", + "INSERT INTO path_records VALUES + (1, 1, 'first', false), (2, 2, 'second', true), (3, 1, 'third', false), + (4, 3, 'fourth', false), (5, NULL, 'fifth', true), (6, 99, 'sixth', false), + (7, 4, 'seventh', false)", + "INSERT INTO path_assignments VALUES + (1, 1, 'b'), (2, 2, 'a'), (3, 1, 'a'), (4, NULL, 'a'), + (5, 1, NULL), (6, 99, 'missing'), (7, 3, 'b'), (8, 2, 'b')", + "INSERT INTO path_nodes VALUES + (1, NULL, 'root'), (2, 1, 'branch'), (3, 2, 'leaf'), (4, 99, 'orphan'), (5, 5, 'cycle')", + ] { + ex.execute(sql, PgArguments::default()).await?; + } + Ok(()) +} + +#[tokio::test] +async fn filtering_is_independent_of_loading_strategy() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let query = Record::query() + .filter(Record::owner.enabled.eq(true)) + .order_by(Order::asc(Record::id)); + let bare: Vec = query.clone().all(&tx).await?; + assert_eq!(bare.iter().map(|row| row.id).collect::>(), [1, 3, 4]); + let _: &NotLoaded = &bare[0].owner; + + let joined: Vec>> = query.clone().with(Record::owner.joined()).all(&tx).await?; + let selectin: Vec>> = query.with(Record::owner.selectin()).all(&tx).await?; + for rows in [joined, selectin] { + assert_eq!(rows.iter().map(|row| row.id).collect::>(), [1, 3, 4]); + assert_eq!(rows.iter().map(|row| row.owner.as_ref().unwrap().id).collect::>(), [1, 1, 3]); + assert!(rows.iter().all(|row| row.owner.as_ref().unwrap().enabled)); + } + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn outer_joins_preserve_missing_rows_in_or_null_and_projection_expressions() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let rows: Vec>> = Record::query() + .filter(Record::enabled.eq(true).or(Record::owner.enabled.eq(true))) + .with(Record::owner.joined()) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + assert_eq!(rows.iter().map(|row| row.id).collect::>(), [1, 2, 3, 4, 5]); + assert!(rows.last().unwrap().owner.is_none()); + + let missing: Vec>> = Record::query() + .filter(Record::owner.id.is_null()) + .with(Record::owner.joined()) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + assert_eq!(missing.iter().map(|row| row.id).collect::>(), [5, 6]); + assert!(missing.iter().all(|row| row.owner.is_none())); + + let disabled: Vec = Record::query() + .filter(Record::owner.enabled.eq(true).not()) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + assert_eq!(disabled.iter().map(|row| row.id).collect::>(), [2, 7]); + + let rows: Vec<(i64, Option, Option)> = Record::query() + .select_only() + .column(Record::id) + .column(Record::owner.enabled.eq(true)) + .column(Record::owner.note) + .order_by(Order::asc(Record::id)) + .into_model() + .all(&tx) + .await?; + assert_eq!( + rows, + vec![ + (1, Some(true), None), + (2, Some(false), Some("memo".into())), + (3, Some(true), None), + (4, Some(true), Some("".into())), + (5, None, None), + (6, None, None), + (7, Some(false), None), + ] + ); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn expressions_renamed_columns_and_null_checks_use_the_related_row() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let rows: Vec = Record::query() + .filter(func::lower(Record::owner.label).eq("atlas")) + .filter((Record::owner.score + 5_i32).ge(35_i32)) + .filter(Record::owner.code.in_(["a", "b"])) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + assert_eq!(rows.iter().map(|row| row.id).collect::>(), [1, 3]); + + let null_notes: Vec = Record::query() + .filter(Record::owner.note.is_null()) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + assert_eq!(null_notes.iter().map(|row| row.id).collect::>(), [1, 3, 5, 6, 7]); + + let none: Vec = Record::query().filter(Record::owner.code.eq("a' OR true --")).all(&tx).await?; + assert!(none.is_empty()); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn related_ordering_runs_before_limit_and_keeps_missing_rows() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let query = Record::query() + .order_by(Order::asc(Record::owner.label)) + .order_by(Order::asc(Record::id)); + let all: Vec = query.clone().all(&tx).await?; + assert_eq!(all.iter().map(|row| row.id).collect::>(), [1, 3, 2, 4, 7, 5, 6]); + let page: Vec>> = query.with(Record::owner.joined()).limit(2).offset(1).all(&tx).await?; + assert_eq!(page.iter().map(|row| row.id).collect::>(), [3, 2]); + assert_eq!( + page.iter() + .map(|row| row.owner.as_ref().unwrap().label.as_str()) + .collect::>(), + ["Atlas", "Birch"] + ); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn count_exists_one_and_paginate_use_the_same_relation_filter() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let query = Record::query() + .filter(Record::owner.enabled.eq(true)) + .with(Record::owner.joined()) + .order_by(Order::asc(Record::id)); + assert_eq!(query.clone().limit(1).offset(50).count(&tx).await?, 3); + assert!(query.clone().limit(1).offset(50).exists(&tx).await?); + let first = query.clone().one(&tx).await?.unwrap(); + assert_eq!((first.id, first.owner.unwrap().id), (1, 1)); + let page = query.paginate(2, 2, &tx).await?; + assert_eq!(page.total, 3); + assert_eq!(page.total_pages(), 2); + assert_eq!(page.items.iter().map(|row| row.id).collect::>(), [4]); + assert_eq!(page.items[0].owner.as_ref().unwrap().id, 3); + + let empty = Record::query().filter(Record::owner.score.gt(100_i32)).with(Record::owner.joined()); + assert_eq!(empty.count(&tx).await?, 0); + assert!(!empty.exists(&tx).await?); + assert!(empty.clone().one(&tx).await?.is_none()); + assert!(empty.all(&tx).await?.is_empty()); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn nested_filters_and_both_nested_loading_strategies_agree() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let query = Record::query() + .filter(Record::owner.organization.label.eq("north")) + .order_by(Order::asc(Record::id)); + let joined: Vec>>>> = query + .clone() + .with(Record::owner.joined().with(Member::organization.joined())) + .all(&tx) + .await?; + let selectin: Vec>>>> = query + .with(Record::owner.selectin().with(Member::organization.selectin())) + .all(&tx) + .await?; + for rows in [joined, selectin] { + assert_eq!(rows.iter().map(|row| row.id).collect::>(), [1, 3]); + assert!(rows + .iter() + .all(|row| row.owner.as_ref().unwrap().organization.as_ref().unwrap().label == "north")); + } + + let missing: Vec>>>> = Record::query() + .filter(Record::owner.organization.id.is_null()) + .with(Record::owner.joined().with(Member::organization.joined())) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + assert_eq!(missing.iter().map(|row| row.id).collect::>(), [5, 6, 7]); + assert!(missing[0].owner.is_none() && missing[1].owner.is_none()); + assert_eq!(missing[2].owner.as_ref().unwrap().id, 4); + assert!(missing[2].owner.as_ref().unwrap().organization.is_none()); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn sibling_relations_filter_and_decode_independently_in_both_loaders() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let query = Assignment::query() + .filter(Assignment::first.enabled.eq(true)) + .filter(Assignment::second.enabled.eq(false)) + .filter(Assignment::first.score.gt(Assignment::second.score)) + .order_by(Order::asc(Assignment::id)); + let joined: Vec, Option>> = query + .clone() + .with(Assignment::first.joined()) + .with(Assignment::second.joined()) + .all(&tx) + .await?; + let selectin: Vec, Option>> = query + .with(Assignment::second.selectin()) + .with(Assignment::first.selectin()) + .all(&tx) + .await?; + for rows in [joined, selectin] { + assert_eq!(rows.iter().map(|row| row.id).collect::>(), [1, 7]); + assert_eq!(rows.iter().map(|row| row.first.as_ref().unwrap().id).collect::>(), [1, 3]); + assert!(rows.iter().all(|row| row.second.as_ref().unwrap().id == 2)); + } + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn missing_or_equal_sibling_targets_do_not_overwrite_each_other() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let query = Assignment::query().order_by(Order::asc(Assignment::id)); + let joined: Vec, Option>> = query + .clone() + .with(Assignment::second.joined()) + .with(Assignment::first.joined()) + .all(&tx) + .await?; + let selectin: Vec, Option>> = query + .with(Assignment::first.selectin()) + .with(Assignment::second.selectin()) + .all(&tx) + .await?; + for rows in [joined, selectin] { + let actual: Vec<_> = rows + .iter() + .map(|row| (row.id, row.first.as_ref().map(|m| m.id), row.second.as_ref().map(|m| m.id))) + .collect(); + assert_eq!( + actual, + [ + (1, Some(1), Some(2)), + (2, Some(2), Some(1)), + (3, Some(1), Some(1)), + (4, None, Some(1)), + (5, Some(1), None), + (6, None, None), + (7, Some(3), Some(2)), + (8, Some(2), Some(2)), + ] + ); + } + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn nested_siblings_keep_their_own_organizations() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let rows: Vec>>, Option>>>> = Assignment::query() + .filter(Assignment::first.organization.label.eq("north")) + .filter(Assignment::second.organization.label.eq("south")) + .with(Assignment::first.joined().with(Member::organization.joined())) + .with(Assignment::second.joined().with(Member::organization.joined())) + .all(&tx) + .await?; + assert_eq!(rows.len(), 1); + assert_eq!(rows[0].id, 1); + assert_eq!(rows[0].first.as_ref().unwrap().organization.as_ref().unwrap().id, 1); + assert_eq!(rows[0].second.as_ref().unwrap().organization.as_ref().unwrap().id, 2); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn self_relations_support_finite_depth_missing_parents_and_cycles() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let leaf: Node>>> = Node::query() + .filter(Node::parent.parent.label.eq("root")) + .with(Node::parent.joined().with(Node::parent.joined())) + .one(&tx) + .await? + .unwrap(); + assert_eq!(leaf.id, 3); + let parent = leaf.parent.unwrap(); + assert_eq!(parent.id, 2); + assert_eq!(parent.parent.unwrap().id, 1); + + let query = Node::query().order_by(Order::asc(Node::id)); + let joined: Vec>>>> = query + .clone() + .with(Node::parent.joined().with(Node::parent.joined())) + .all(&tx) + .await?; + let selectin: Vec>>>> = + query.with(Node::parent.selectin().with(Node::parent.selectin())).all(&tx).await?; + for rows in [joined, selectin] { + assert_eq!(rows.len(), 5); + assert!(rows[0].parent.is_none()); + assert!(rows[3].parent.is_none()); + let cycle = rows[4].parent.as_ref().unwrap(); + assert_eq!(cycle.id, 5); + assert_eq!(cycle.parent.as_ref().unwrap().id, 5); + } + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn explicit_inner_joins_and_existing_table_column_filters_still_work() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let inner: Vec>> = Record::query() + .join(Record::owner) + .with(Record::owner.joined()) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + assert_eq!(inner.iter().map(|row| row.id).collect::>(), [1, 2, 3, 4, 7]); + + let legacy: Vec>> = Record::query() + .with(Record::owner.joined()) + .filter(Member::enabled.eq(true)) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + let mixed: Vec>> = Record::query() + .join(Record::owner) + .filter(Member::enabled.eq(true)) + .filter(Record::owner.code.eq("a")) + .with(Record::owner.joined()) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + assert_eq!(legacy.iter().map(|row| row.id).collect::>(), [1, 3, 4]); + assert_eq!(mixed.iter().map(|row| row.id).collect::>(), [1, 3]); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn grouped_projections_and_correlated_exists_resolve_paths_in_their_query() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let counts: Vec<(Option, i64)> = Record::query() + .select_only() + .column(Record::owner.enabled) + .column(func::count(Record::id)) + .group_by(Record::owner.enabled) + .having(func::count(Record::id).gt(1_i64)) + .order_by(Order::asc(Record::owner.enabled)) + .into_model() + .all(&tx) + .await?; + assert_eq!(counts, [(Some(false), 2), (Some(true), 3), (None, 2)]); + + let organizations: Vec = Organization::query() + .where_exists( + Record::query() + .select_only() + .column(Record::id) + .filter(Record::owner.organization_id.eq(Organization::id)) + .filter(Record::owner.score.gt(25_i32)), + ) + .all(&tx) + .await?; + assert_eq!(organizations.iter().map(|row| row.id).collect::>(), [1]); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn explicit_load_and_partial_unload_preserve_sibling_relation_identity() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let bare: Assignment = Assignment::query().filter(Assignment::id.eq(1_i64)).one(&tx).await?.unwrap(); + let second: Assignment> = bare.load(Assignment::second, &tx).await?; + assert_eq!(second.second.as_ref().unwrap().id, 2); + let both: Assignment, Option> = second.load(Assignment::first, &tx).await?; + assert_eq!(both.first.as_ref().unwrap().id, 1); + assert_eq!(both.second.as_ref().unwrap().id, 2); + let first_only: Assignment> = both.clone().into(); + let second_only: Assignment> = both.into(); + assert_eq!(first_only.first.unwrap().id, 1); + assert_eq!(second_only.second.unwrap().id, 2); + tx.rollback().await?; + Ok(()) +} diff --git a/crates/dbkit/tests/relation_paths_sql.rs b/crates/dbkit/tests/relation_paths_sql.rs new file mode 100644 index 0000000..91ba787 --- /dev/null +++ b/crates/dbkit/tests/relation_paths_sql.rs @@ -0,0 +1,405 @@ +//! Proposed contract for BelongsTo column paths, intentionally red until implemented. +//! +//! `Record::owner.enabled` is a typed, read-only SQL column path. Using a path +//! adds LEFT JOINs without loading model fields. `.with(...)` still controls loading. +//! Repeated paths share joins; different paths to the same table do not. +//! Explicit `.join(path)` retains INNER JOIN semantics and shares its join with loading. +//! Paths compose through BelongsTo, including self-relations. Collection predicates +//! and mutation syntax are outside this specification. +//! Related expressions are nullable, even for required target columns, because the +//! related row can be absent. Existing Option columns do not become nested Options. +//! Alias spelling is internal: tests inspect its use, not a particular naming scheme. + +#[path = "support/relation_path_graphs.rs"] +mod relation_path_graphs; +#[path = "support/relation_paths.rs"] +mod relation_paths; + +use dbkit::executor::BoxFuture; +use dbkit::sqlx::postgres::PgArguments; +use dbkit::{func, Error, Executor, Order, SelectExt, Value}; +use relation_path_graphs::{Assignment, Node}; +use relation_paths::{Member, Organization, Record}; +use std::sync::Mutex; + +// Only inspect dbkit's generated JOIN clauses; execution semantics are tested against PostgreSQL. +fn aliases<'a>(sql: &'a str, table: &str) -> Vec<&'a str> { + sql.split(&format!(" JOIN {table} ")) + .skip(1) + .map(|rest| { + let alias = rest.split_once(" ON ").expect("JOIN needs ON").0; + let alias = alias.strip_prefix("AS ").unwrap_or(alias); + assert!(!alias.is_empty() && !alias.contains(' '), "expected a table alias: {sql}"); + alias + }) + .collect() +} + +fn only_alias<'a>(sql: &'a str, table: &str) -> &'a str { + let found = aliases(sql, table); + assert_eq!(found.len(), 1, "expected one join of {table}: {sql}"); + found[0] +} + +#[test] +fn filter_adds_a_left_join_without_selecting_or_loading_the_relation() { + let query: dbkit::Select = Record::query().filter(Record::owner.enabled.eq(true)); + let compiled = query.compile(); + let owner = only_alias(&compiled.sql, "path_members"); + + assert!(compiled + .sql + .starts_with("SELECT path_records.* FROM path_records LEFT JOIN path_members ")); + assert!(compiled.sql.contains(&format!("({owner}.id = path_records.owner_id)"))); + assert!(compiled.sql.ends_with(&format!("WHERE ({owner}.enabled = $1)"))); + assert_eq!(compiled.binds, vec![Value::Bool(true)]); +} + +#[test] +fn repeated_columns_and_computed_expressions_reuse_the_path() { + let compiled = Record::query() + .filter(Record::owner.enabled.eq(true)) + .filter(func::lower(Record::owner.label).eq("atlas")) + .filter((Record::owner.score + 1_i32).gt(10_i32)) + .order_by(Order::desc(Record::owner.score)) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + + assert!(compiled.sql.contains(&format!("LOWER({owner}.label)"))); + assert!(compiled.sql.contains(&format!("({owner}.score + $3)"))); + assert!(compiled.sql.ends_with(&format!("ORDER BY {owner}.score DESC"))); + assert_eq!( + compiled.binds, + vec![Value::Bool(true), Value::String("atlas".into()), Value::I32(1), Value::I32(10)] + ); +} + +#[test] +fn or_and_null_checks_keep_the_automatic_join_outer() { + let compiled = Record::query() + .filter(Record::enabled.eq(true).or(Record::owner.id.is_null())) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + + assert_eq!(compiled.sql.matches(" LEFT JOIN ").count(), 1); + assert!(compiled + .sql + .contains(&format!("((path_records.enabled = $1) OR ({owner}.id IS NULL))"))); +} + +#[test] +fn renamed_columns_use_database_names_and_values_stay_bound() { + let value = "x' OR true --"; + let compiled = Record::query().filter(Record::owner.code.eq(value)).compile(); + let owner = only_alias(&compiled.sql, "path_members"); + + assert!(compiled.sql.ends_with(&format!("WHERE ({owner}.external_ref = $1)"))); + assert!(!compiled.sql.contains(value)); + assert_eq!(compiled.binds, vec![Value::String(value.into())]); +} + +#[test] +fn order_only_paths_add_joins_before_pagination() { + let compiled = Record::query() + .order_by(Order::asc(Record::owner.label)) + .order_by(Order::asc(Record::id)) + .limit(2) + .offset(1) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + + assert!(compiled + .sql + .ends_with(&format!("ORDER BY {owner}.label ASC, path_records.id ASC LIMIT 2 OFFSET 1"))); + assert!(compiled.binds.is_empty()); +} + +#[test] +fn projection_only_paths_add_joins_and_keep_output_aliases() { + let compiled = Record::query() + .select_only() + .column(Record::id) + .column_as(Record::owner.label, "owner_label") + .column_as(func::coalesce(Record::owner.note, "missing"), "note") + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + + assert!(compiled.sql.starts_with(&format!( + "SELECT path_records.id, {owner}.label AS owner_label, COALESCE({owner}.note, $1) AS note FROM " + ))); + assert_eq!(compiled.binds, vec![Value::String("missing".into())]); +} + +#[test] +fn group_by_and_having_also_discover_relation_paths() { + let grouped = Record::query() + .select_only() + .column(func::count(Record::id)) + .group_by(Record::owner.enabled) + .compile(); + let owner = only_alias(&grouped.sql, "path_members"); + assert!(grouped.sql.ends_with(&format!("GROUP BY {owner}.enabled"))); + + let having = Record::query() + .select_only() + .column(func::count(Record::id)) + .having(func::count(Record::owner.id).gt(0_i64)) + .compile(); + let owner = only_alias(&having.sql, "path_members"); + assert!(having.sql.contains(&format!("HAVING (COUNT({owner}.id) > $1)"))); + assert_eq!(having.binds, vec![Value::I64(0)]); +} + +#[test] +fn explicit_join_kind_is_preserved_without_an_extra_implicit_join() { + let inner = Record::query().join(Record::owner).filter(Record::owner.enabled.eq(true)).compile(); + only_alias(&inner.sql, "path_members"); + assert!(!inner.sql.contains("LEFT JOIN"), "explicit INNER JOIN changed: {}", inner.sql); + + let outer = Record::query() + .filter(Record::owner.enabled.eq(true)) + .left_join(Record::owner) + .compile(); + only_alias(&outer.sql, "path_members"); + assert_eq!(outer.sql.matches("LEFT JOIN").count(), 1); +} + +#[test] +fn nested_paths_share_their_prefix_and_join_in_dependency_order() { + let compiled = Record::query() + .filter(Record::owner.organization.label.eq("north")) + .filter(Record::owner.enabled.eq(true)) + .order_by(Order::asc(Record::owner.organization.id)) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + let organization = only_alias(&compiled.sql, "path_organizations"); + + assert_ne!(owner, organization); + assert!(compiled.sql.find("JOIN path_members").unwrap() < compiled.sql.find("JOIN path_organizations").unwrap()); + assert!(compiled.sql.contains(&format!("({organization}.id = {owner}.organization_id)"))); + assert!(compiled.sql.contains(&format!("({organization}.label = $1)"))); + assert_eq!(compiled.binds, vec![Value::String("north".into()), Value::Bool(true)]); +} + +#[test] +fn sibling_paths_keep_distinct_aliases_and_their_own_reference_keys() { + let compiled = Assignment::query() + .filter(Assignment::first.enabled.eq(true)) + .filter(Assignment::second.enabled.eq(false)) + .filter(Assignment::first.score.gt(Assignment::second.score)) + .compile(); + let members = aliases(&compiled.sql, "path_members"); + assert_eq!(members.len(), 2, "{}", compiled.sql); + assert_ne!(members[0], members[1]); + let first = members + .iter() + .find(|alias| compiled.sql.contains(&format!("({alias}.id = path_assignments.first_id)"))) + .unwrap(); + let second = members + .iter() + .find(|alias| { + compiled + .sql + .contains(&format!("({alias}.external_ref = path_assignments.second_code)")) + }) + .unwrap(); + assert_ne!(first, second); + assert!(compiled.sql.contains(&format!("({first}.enabled = $1)"))); + assert!(compiled.sql.contains(&format!("({second}.enabled = $2)"))); + assert!(compiled.sql.contains(&format!("({first}.score > {second}.score)"))); + assert_eq!(compiled.binds, vec![Value::Bool(true), Value::Bool(false)]); +} + +#[test] +fn nested_sibling_paths_do_not_merge_at_the_same_target_table() { + let compiled = Assignment::query() + .filter(Assignment::first.organization.label.eq("north")) + .filter(Assignment::second.organization.label.eq("south")) + .compile(); + let members = aliases(&compiled.sql, "path_members"); + let organizations = aliases(&compiled.sql, "path_organizations"); + assert_eq!(members.len(), 2); + assert_eq!(organizations.len(), 2); + assert_ne!(organizations[0], organizations[1]); + for member in members { + assert_eq!( + organizations + .iter() + .filter(|organization| { compiled.sql.contains(&format!("({organization}.id = {member}.organization_id)")) }) + .count(), + 1, + "{}", + compiled.sql + ); + } +} + +#[test] +fn self_paths_keep_the_base_parent_and_grandparent_distinct() { + let compiled = Node::query() + .filter(Node::label.eq("leaf")) + .filter(Node::parent.label.eq("branch")) + .filter(Node::parent.parent.label.eq("root")) + .compile(); + let nodes = aliases(&compiled.sql, "path_nodes"); + assert_eq!(nodes.len(), 2); + assert_ne!(nodes[0], nodes[1]); + assert!(nodes.iter().all(|alias| *alias != "path_nodes")); + let parent = nodes + .iter() + .find(|alias| compiled.sql.contains(&format!("({alias}.id = path_nodes.parent_id)"))) + .unwrap(); + let grandparent = nodes + .iter() + .find(|alias| compiled.sql.contains(&format!("({alias}.id = {parent}.parent_id)"))) + .unwrap(); + assert!(compiled.sql.contains(&format!("({parent}.label = $2)"))); + assert!(compiled.sql.contains(&format!("({grandparent}.label = $3)"))); + assert_eq!( + compiled.binds, + vec![ + Value::String("leaf".into()), + Value::String("branch".into()), + Value::String("root".into()) + ] + ); +} + +#[test] +fn subquery_paths_stay_in_the_subquery_and_preserve_correlated_base_columns() { + let compiled = Organization::query() + .filter(Organization::label.eq("north")) + .where_exists( + Record::query() + .select_only() + .column(Record::id) + .filter(Record::owner.organization_id.eq(Organization::id)) + .filter(Record::owner.enabled.eq(true)), + ) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + let (outer, inner) = compiled.sql.split_once("EXISTS (").expect("EXISTS subquery"); + assert!(!outer.contains("JOIN"), "subquery join leaked into outer query: {}", compiled.sql); + assert!(inner.contains(&format!("({owner}.organization_id = path_organizations.id)"))); + assert!(inner.contains(&format!("({owner}.enabled = $2)"))); + assert_eq!(compiled.binds, vec![Value::String("north".into()), Value::Bool(true)]); +} + +#[test] +fn compiling_and_cloning_do_not_accumulate_joins_or_change_aliases() { + let query = Record::query().filter(Record::owner.enabled.eq(true)); + let first = query.compile(); + assert_eq!(query.compile(), first); + assert_eq!(query.clone().compile(), first); + let extended = query.clone().filter(Record::owner.organization.label.eq("north")).compile(); + only_alias(&extended.sql, "path_members"); + only_alias(&extended.sql, "path_organizations"); + assert_eq!(query.compile(), first); +} + +#[derive(Default)] +struct CaptureExecutor(Mutex>); + +impl Executor for CaptureExecutor { + fn fetch_all<'e, T>(&'e self, sql: &'e str, _: PgArguments) -> BoxFuture<'e, Result, Error>> + where + T: for<'r> dbkit::sqlx::FromRow<'r, dbkit::sqlx::postgres::PgRow> + Send + Unpin + 'e, + { + self.0.lock().unwrap().push(sql.to_owned()); + Box::pin(async { Ok(Vec::new()) }) + } + + fn fetch_optional<'e, T>(&'e self, sql: &'e str, _: PgArguments) -> BoxFuture<'e, Result, Error>> + where + T: for<'r> dbkit::sqlx::FromRow<'r, dbkit::sqlx::postgres::PgRow> + Send + Unpin + 'e, + { + self.0.lock().unwrap().push(sql.to_owned()); + Box::pin(async { Ok(None) }) + } + + fn fetch_rows<'e>(&'e self, sql: &'e str, _: PgArguments) -> BoxFuture<'e, Result, Error>> { + self.0.lock().unwrap().push(sql.to_owned()); + Box::pin(async { Ok(Vec::new()) }) + } + + fn execute<'e>(&'e self, _: &'e str, _: PgArguments) -> BoxFuture<'e, Result> { + panic!("read-only query must not call execute") + } +} + +#[tokio::test] +async fn filtering_and_joined_loading_share_one_join_regardless_of_builder_order() -> Result<(), Error> { + let ex = CaptureExecutor::default(); + let _: Vec>> = Record::query() + .filter(Record::owner.enabled.eq(true)) + .with(Record::owner.joined()) + .all(&ex) + .await?; + let _: Vec>> = Record::query() + .with(Record::owner.joined()) + .filter(Record::owner.enabled.eq(true)) + .all(&ex) + .await?; + + let sqls = ex.0.lock().unwrap(); + assert_eq!(sqls.len(), 2); + assert_eq!(sqls[0], sqls[1]); + let owner = only_alias(&sqls[0], "path_members"); + assert!(sqls[0].contains(&format!("{owner}.label AS "))); + assert!(sqls[0].contains(&format!("({owner}.enabled = $1)"))); + Ok(()) +} + +#[tokio::test] +async fn explicit_join_and_joined_loading_share_the_relation_join() -> Result<(), Error> { + let ex = CaptureExecutor::default(); + let _: Vec>> = Record::query() + .join(Record::owner) + .filter(Record::owner.enabled.eq(true)) + .with(Record::owner.joined()) + .all(&ex) + .await?; + let sqls = ex.0.lock().unwrap(); + assert_eq!(sqls.len(), 1); + only_alias(&sqls[0], "path_members"); + assert!(!sqls[0].contains("LEFT JOIN")); + Ok(()) +} + +#[tokio::test] +async fn nested_filter_and_nested_loader_reuse_the_entire_path() -> Result<(), Error> { + let ex = CaptureExecutor::default(); + let _: Vec>>>> = Record::query() + .filter(Record::owner.organization.label.eq("north")) + .with(Record::owner.joined().with(Member::organization.joined())) + .all(&ex) + .await?; + let sqls = ex.0.lock().unwrap(); + assert_eq!(sqls.len(), 1); + let owner = only_alias(&sqls[0], "path_members"); + let organization = only_alias(&sqls[0], "path_organizations"); + assert!(sqls[0].contains(&format!("{owner}.label AS "))); + assert!(sqls[0].contains(&format!("{organization}.label AS "))); + Ok(()) +} + +#[tokio::test] +async fn count_and_exists_include_filter_joins_without_loading_columns() -> Result<(), Error> { + let ex = CaptureExecutor::default(); + let query = Record::query() + .filter(Record::owner.enabled.eq(true)) + .with(Record::owner.joined()) + .limit(1) + .offset(2); + query.count(&ex).await?; + query.exists(&ex).await?; + let sqls = ex.0.lock().unwrap(); + assert_eq!(sqls.len(), 2); + for sql in sqls.iter() { + let owner = only_alias(sql, "path_members"); + assert!(sql.contains(&format!("({owner}.enabled = $1)"))); + assert!(!sql.contains("LIMIT") && !sql.contains("OFFSET")); + assert!(!sql.contains(&format!("{owner}.label AS "))); + } + Ok(()) +} diff --git a/crates/dbkit/tests/support/relation_path_graphs.rs b/crates/dbkit/tests/support/relation_path_graphs.rs new file mode 100644 index 0000000..01d4f85 --- /dev/null +++ b/crates/dbkit/tests/support/relation_path_graphs.rs @@ -0,0 +1,26 @@ +#![allow(dead_code, non_upper_case_globals)] + +use super::relation_paths::Member; +use dbkit::model; + +#[model(table = "path_assignments")] +pub struct Assignment { + #[key] + pub id: i64, + pub first_id: Option, + pub second_code: Option, + #[belongs_to(key = first_id, references = id)] + pub first: dbkit::BelongsTo, + #[belongs_to(key = second_code, references = code)] + pub second: dbkit::BelongsTo, +} + +#[model(table = "path_nodes")] +pub struct Node { + #[key] + pub id: i64, + pub parent_id: Option, + pub label: String, + #[belongs_to(key = parent_id, references = id)] + pub parent: dbkit::BelongsTo, +} diff --git a/crates/dbkit/tests/support/relation_paths.rs b/crates/dbkit/tests/support/relation_paths.rs new file mode 100644 index 0000000..0238e14 --- /dev/null +++ b/crates/dbkit/tests/support/relation_paths.rs @@ -0,0 +1,36 @@ +#![allow(dead_code, non_upper_case_globals)] + +use dbkit::model; + +#[model(table = "path_organizations")] +pub struct Organization { + #[key] + pub id: i64, + pub label: String, +} + +#[model(table = "path_members")] +pub struct Member { + #[key] + pub id: i64, + pub organization_id: i64, + pub label: String, + pub enabled: bool, + pub score: i32, + pub note: Option, + #[dbkit(column = "external_ref")] + pub code: String, + #[belongs_to(key = organization_id, references = id)] + pub organization: dbkit::BelongsTo, +} + +#[model(table = "path_records")] +pub struct Record { + #[key] + pub id: i64, + pub owner_id: Option, + pub label: String, + pub enabled: bool, + #[belongs_to(key = owner_id, references = id)] + pub owner: dbkit::BelongsTo, +} diff --git a/crates/dbkit/tests/ui/fail_relation_paths_assignment.rs b/crates/dbkit/tests/ui/fail_relation_paths_assignment.rs new file mode 100644 index 0000000..0718997 --- /dev/null +++ b/crates/dbkit/tests/ui/fail_relation_paths_assignment.rs @@ -0,0 +1,11 @@ +#[path = "../support/relation_paths.rs"] +mod relation_paths; + +use relation_paths::Record; + +fn main() { + // Filtering through a relation automatically adds its join if not already declared. + let _ = Record::query().filter(Record::owner.enabled.eq(true)); + // A relation path must not silently become an UPDATE of the base row's column. + let _ = Record::update().set(Record::owner.enabled, true); //~ E0308 +} diff --git a/crates/dbkit/tests/ui/fail_relation_paths_nullability.rs b/crates/dbkit/tests/ui/fail_relation_paths_nullability.rs new file mode 100644 index 0000000..1be509b --- /dev/null +++ b/crates/dbkit/tests/ui/fail_relation_paths_nullability.rs @@ -0,0 +1,12 @@ +#[path = "../support/relation_paths.rs"] +mod relation_paths; + +use dbkit::{Expr, IntoExpr}; +use relation_paths::Record; + +fn main() { + let _: Expr> = Record::owner.enabled.eq(true); + let _: Expr = Record::owner.enabled.eq(true); //~ E0308 + let _: Expr = Record::owner.label.into_expr(); //~ E0308 + let _: Expr>> = Record::owner.note.into_expr(); //~ E0308 +} diff --git a/crates/dbkit/tests/ui/fail_relation_paths_unknown_field.rs b/crates/dbkit/tests/ui/fail_relation_paths_unknown_field.rs new file mode 100644 index 0000000..0c56857 --- /dev/null +++ b/crates/dbkit/tests/ui/fail_relation_paths_unknown_field.rs @@ -0,0 +1,10 @@ +#[path = "../support/relation_paths.rs"] +mod relation_paths; + +use relation_paths::Record; + +fn main() { + let _ = Record::owner.enabled.eq(true); + let _ = Record::owner.unknown.eq(true); //~ E0609 + let _ = Record::owner.organization.unknown.eq(true); //~ E0609 +} diff --git a/crates/dbkit/tests/ui/fail_relation_paths_value_types.rs b/crates/dbkit/tests/ui/fail_relation_paths_value_types.rs new file mode 100644 index 0000000..6be1290 --- /dev/null +++ b/crates/dbkit/tests/ui/fail_relation_paths_value_types.rs @@ -0,0 +1,11 @@ +#[path = "../support/relation_paths.rs"] +mod relation_paths; + +use relation_paths::Record; + +fn main() { + let _ = Record::owner.enabled.eq(true); + let _ = Record::owner.enabled.eq("enabled"); //~ E0277 + let _ = Record::owner.score.gt("high"); //~ E0277 + let _ = dbkit::func::lower(Record::owner.score); //~ E0277 +} diff --git a/crates/dbkit/tests/ui/pass_relation_paths.rs b/crates/dbkit/tests/ui/pass_relation_paths.rs new file mode 100644 index 0000000..ea9b52f --- /dev/null +++ b/crates/dbkit/tests/ui/pass_relation_paths.rs @@ -0,0 +1,65 @@ +//@check-pass +#[path = "../support/relation_paths.rs"] +mod relation_paths; + +use dbkit::{func, Expr, IntoExpr, NotLoaded, Order, SelectExt}; +use relation_paths::{Member, Organization, Record}; + +fn nullable(_: Expr>) {} +fn required(_: Expr) {} + +async fn loading(ex: &(impl dbkit::Executor + Send + Sync)) -> Result<(), dbkit::Error> { + let bare: Vec = Record::query().filter(Record::owner.enabled.eq(true)).all(ex).await?; + let _: &NotLoaded = &bare[0].owner; + let _: Vec>> = Record::query() + .with(Record::owner.joined()) + .filter(Record::owner.enabled.eq(true)) + .all(ex) + .await?; + let _: Vec>> = Record::query() + .filter(Record::owner.enabled.eq(true)) + .with(Record::owner.selectin()) + .all(ex) + .await?; + let _: Vec>>>> = Record::query() + .filter(Record::owner.organization.label.eq("north")) + .with(Record::owner.joined().with(Member::organization.joined())) + .all(ex) + .await?; + Ok(()) +} + +fn main() { + // Relation columns include outer-join nullability without changing base columns. + required::(Member::enabled.into_expr()); + nullable::(Record::owner.enabled.into_expr()); + nullable::(Record::owner.id.into_expr()); + nullable::(Record::owner.label.into_expr()); + nullable::(Record::owner.note.into_expr()); + nullable::(Record::owner.organization.label.into_expr()); + nullable::(Record::owner.enabled.eq(true)); + nullable::(Record::owner.score.gt(10_i32)); + nullable::(Record::owner.score.gt(Member::score)); + nullable::(Record::owner.note.eq(None)); + nullable::(Record::owner.label.in_(["atlas", "birch"])); + nullable::(Record::owner.label.ilike("a%")); + nullable::(Record::owner.enabled.eq(true).and(Record::enabled.eq(true))); + nullable::(Record::owner.enabled.eq(true).not()); + required::(Record::owner.id.is_null()); + required::(Record::owner.note.is_not_null()); + nullable::(Record::owner.score + 1_i32); + nullable::(func::lower(Record::owner.label)); + required::(func::coalesce(Record::owner.note, "missing")); + + let path = Record::owner; + let column = path.code; + let predicate = column.eq("a"); + let _: dbkit::Select = Record::query().filter(predicate.clone()); + let _reused = Record::query().filter(predicate).order_by(Order::asc(path.label)); + let _projection = Record::query() + .select_only() + .column_as(path.label, "owner_label") + .column_as(path.organization.label, "organization_label"); + let _explicit_inner = Record::query().join(path).filter(column.eq("a")); + let _explicit_outer = Record::query().left_join(path).filter(path.id.is_null()); +} diff --git a/crates/dbkit/tests/ui/pass_relation_paths_field_names.rs b/crates/dbkit/tests/ui/pass_relation_paths_field_names.rs new file mode 100644 index 0000000..ef069ad --- /dev/null +++ b/crates/dbkit/tests/ui/pass_relation_paths_field_names.rs @@ -0,0 +1,39 @@ +//@check-pass +use dbkit::{model, Expr}; + +mod targets { + use dbkit::model; + + #[model(table = "named_targets")] + pub struct TargetModel { + #[key] + pub id: i64, + pub joined: bool, + pub selectin: bool, + pub r#type: String, + #[dbkit(column = "external_ref")] + pub code: String, + } +} + +#[model(table = "named_sources")] +pub struct SourceModel { + #[key] + pub id: i64, + pub target_id: i64, + #[belongs_to(key = target_id, references = id)] + pub target: dbkit::BelongsTo, +} + +fn main() { + let _: Expr> = SourceModel::target.joined.eq(true); + let _: Expr> = SourceModel::target.selectin.eq(false); + let _: Expr> = SourceModel::target.r#type.eq("primary"); + let _ = SourceModel::query() + .with(SourceModel::target.joined()) + .filter(SourceModel::target.joined.eq(true)); + let _ = SourceModel::query() + .with(SourceModel::target.selectin()) + .filter(SourceModel::target.selectin.eq(true)); + let _ = SourceModel::query().filter(SourceModel::target.code.eq("a")); +} diff --git a/crates/dbkit/tests/ui/pass_relation_paths_graphs.rs b/crates/dbkit/tests/ui/pass_relation_paths_graphs.rs new file mode 100644 index 0000000..c5574e3 --- /dev/null +++ b/crates/dbkit/tests/ui/pass_relation_paths_graphs.rs @@ -0,0 +1,42 @@ +//@check-pass +#[path = "../support/relation_path_graphs.rs"] +mod relation_path_graphs; +#[path = "../support/relation_paths.rs"] +mod relation_paths; + +use dbkit::{NotLoaded, SelectExt}; +use relation_path_graphs::{Assignment, Node}; +use relation_paths::{Member, Organization}; + +async fn loading(ex: &(impl dbkit::Executor + Send + Sync)) -> Result<(), dbkit::Error> { + let _: Vec>> = Assignment::query().with(Assignment::first.joined()).all(ex).await?; + let _: Vec>> = Assignment::query().with(Assignment::second.joined()).all(ex).await?; + let _: Vec, Option>> = Assignment::query() + .with(Assignment::second.joined()) + .with(Assignment::first.joined()) + .all(ex) + .await?; + let _: Vec>>, Option>> = Assignment::query() + .with(Assignment::first.joined().with(Member::organization.joined())) + .with(Assignment::second.selectin()) + .all(ex) + .await?; + + let _: Vec>>>> = Node::query() + .filter(Node::parent.parent.label.eq("root")) + .with(Node::parent.joined().with(Node::parent.joined())) + .all(ex) + .await?; + Ok(()) +} + +fn main() { + let _ = Assignment::query() + .filter(Assignment::first.enabled.eq(true)) + .filter(Assignment::second.enabled.eq(false)) + .filter(Assignment::first.score.gt(Assignment::second.score)); + let _ = Assignment::query() + .filter(Assignment::first.organization.label.eq("north")) + .filter(Assignment::second.organization.label.eq("south")); + let _ = Node::query().filter(Node::parent.parent.parent.id.eq(1_i64)); +} From 6d84cc8456c41844f640b8f2fda960a9b0f7b7ed Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Mon, 7 Sep 2026 10:59:31 +0200 Subject: [PATCH 02/22] Implement relation column paths --- crates/dbkit-core/src/compile.rs | 48 +- crates/dbkit-core/src/expr.rs | 4 + crates/dbkit-core/src/lib.rs | 2 + crates/dbkit-core/src/path.rs | 430 ++++++++++++++++++ crates/dbkit-core/src/query.rs | 62 +-- crates/dbkit-core/src/rel.rs | 76 ++-- crates/dbkit-core/src/schema.rs | 26 +- crates/dbkit-derive/src/lib.rs | 71 ++- crates/dbkit/src/joined.rs | 93 ++-- .../dbkit/tests/integration_relation_paths.rs | 8 + crates/dbkit/tests/relation_paths_sql.rs | 19 + docs/relations.md | 43 ++ 12 files changed, 784 insertions(+), 98 deletions(-) create mode 100644 crates/dbkit-core/src/path.rs diff --git a/crates/dbkit-core/src/compile.rs b/crates/dbkit-core/src/compile.rs index e4fd641..80cbff0 100644 --- a/crates/dbkit-core/src/compile.rs +++ b/crates/dbkit-core/src/compile.rs @@ -11,6 +11,9 @@ pub struct CompiledSql { pub struct SqlBuilder { sql: String, binds: Vec, + base_table: Option, + relation_aliases: Vec<(Vec, String)>, + declared_tables: Vec, } impl SqlBuilder { @@ -55,8 +58,50 @@ impl SqlBuilder { } } + pub(crate) fn for_query( + base: crate::Table, + relation_aliases: Vec<(Vec, String)>, + declared_tables: Vec, + ) -> Self { + Self { + base_table: Some(base), + relation_aliases, + declared_tables, + ..Self::default() + } + } + pub fn push_column(&mut self, col: ColumnRef) { - self.sql.push_str(&col.qualified_name()); + if let Some(path) = col.path { + self.push_related_column(col, &path.steps()); + return; + } + // Preserve table-qualified queries when exactly one relation supplies that + // table. The base table always keeps its own identity, including self joins. + let mut matches = self.relation_aliases.iter().filter(|(path, _)| { + Some(col.table) != self.base_table + && !self.declared_tables.contains(&col.table) + && path.last().is_some_and(|rel| rel.join_table() == col.table) + }); + let alias = matches.next().filter(|_| matches.next().is_none()); + if let Some((_, alias)) = alias { + self.sql.push_str(alias); + self.sql.push('.'); + self.sql.push_str(col.name); + } else { + self.sql.push_str(&col.qualified_name()); + } + } + + pub fn push_related_column(&mut self, col: ColumnRef, path: &[crate::Relation]) { + let (_, alias) = self + .relation_aliases + .iter() + .find(|(existing, _)| existing == path) + .expect("relation columns require a SELECT query containing their path"); + self.sql.push_str(alias); + self.sql.push('.'); + self.sql.push_str(col.name); } pub fn push_compiled_sql(&mut self, compiled: &CompiledSql) { @@ -130,6 +175,7 @@ impl ToSql for ExprNode { fn to_sql(&self, builder: &mut SqlBuilder) { match self { ExprNode::Column(col) => builder.push_column(*col), + ExprNode::RelatedColumn { column, path } => builder.push_related_column(*column, path), ExprNode::Value(value) => builder.push_value(value.clone()), ExprNode::Row { values } => { builder.push_sql("("); diff --git a/crates/dbkit-core/src/expr.rs b/crates/dbkit-core/src/expr.rs index e5599af..0dfd146 100644 --- a/crates/dbkit-core/src/expr.rs +++ b/crates/dbkit-core/src/expr.rs @@ -284,6 +284,10 @@ pub enum TrimDirection { #[derive(Debug, Clone)] pub enum ExprNode { Column(ColumnRef), + RelatedColumn { + column: ColumnRef, + path: Vec, + }, Value(Value), Row { values: Vec, diff --git a/crates/dbkit-core/src/lib.rs b/crates/dbkit-core/src/lib.rs index 161cc3c..997bced 100644 --- a/crates/dbkit-core/src/lib.rs +++ b/crates/dbkit-core/src/lib.rs @@ -4,6 +4,8 @@ pub mod func; pub mod interval; pub mod load; pub mod mutation; +#[doc(hidden)] +pub mod path; pub mod query; pub mod rel; pub mod schema; diff --git a/crates/dbkit-core/src/path.rs b/crates/dbkit-core/src/path.rs new file mode 100644 index 0000000..b4fac45 --- /dev/null +++ b/crates/dbkit-core/src/path.rs @@ -0,0 +1,430 @@ +//! Typed relation paths and their query-local join planning. + +use std::marker::PhantomData; +use std::ops::{Add, BitAnd, BitOr, BitXor, Deref, Div, Mul, Not, Shl, Shr, Sub}; + +use crate::expr::{ + BinaryOp, BitwiseOperand, ComparisonValue, ExprComparisonMarker, ExprOperand, NullableExprComparisonMarker, ValueComparisonMarker, + ValueComparisonOutput, +}; +use crate::query::{IntoOrderExpr, OrderExpr}; +use crate::rel::Relation; +use crate::{Column, Expr, ExprNode, IntoExpr, Table, Value}; + +#[derive(Debug, PartialEq, Eq)] +pub struct RelationPath { + pub previous: Option<&'static RelationPath>, + pub relation: Relation, +} + +impl RelationPath { + pub fn steps(&self) -> Vec { + let mut steps = self.previous.map_or_else(Vec::new, Self::steps); + steps.push(self.relation); + steps + } +} + +pub trait PathKey: Send + Sync + 'static { + const PATH: Option<&'static RelationPath>; +} + +impl PathKey for () { + const PATH: Option<&'static RelationPath> = None; +} + +pub struct Next(PhantomData<(P, K)>); +impl PathKey for Next { + const PATH: Option<&'static RelationPath> = Some(&RelationPath { + previous: P::PATH, + relation: match K::PATH { + Some(path) => path.relation, + None => panic!("missing relation path"), + }, + }); +} + +pub trait RelationFields { + type Fields: 'static; + const FIELDS: &'static Self::Fields; +} + +pub struct Related(PhantomData<(P, M)>); +impl Related { + pub const NEW: Self = Self(PhantomData); +} +impl Copy for Related {} +impl Clone for Related { + fn clone(&self) -> Self { + *self + } +} +impl> Deref for Related { + type Target = M::Fields; + fn deref(&self) -> &Self::Target { + M::FIELDS + } +} + +// Keep mutation APIs accepting ordinary Column values. Deref reuses the column +// methods, while trait implementations preserve paths in functions and operators. +pub struct RelatedColumn(Column); +impl Copy for RelatedColumn {} +impl Clone for RelatedColumn { + fn clone(&self) -> Self { + *self + } +} +impl RelatedColumn { + pub const fn new(table: Table, name: &'static str) -> Self { + Self(Column::new(table, name).with_path(P::PATH)) + } +} +impl Deref for RelatedColumn { + type Target = Column; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl IntoExpr for RelatedColumn { + fn into_expr(self) -> Expr { + self.0.into_expr() + } +} +impl ExprOperand for RelatedColumn { + type Value = T; + fn into_operand_expr(self) -> Expr { + self.into_expr() + } +} +impl BitwiseOperand for RelatedColumn { + type Value = T; + fn into_bitwise_expr(self) -> Expr { + self.into_expr() + } +} +impl IntoOrderExpr for RelatedColumn { + fn into_order_expr(self) -> OrderExpr { + self.0.into_order_expr() + } +} +impl ComparisonValue for RelatedColumn { + type Output = T::Output; + fn into_comparison_expr(self) -> Expr { + self.into_expr() + } +} +impl ComparisonValue, ExprComparisonMarker> for RelatedColumn { + type Output = Option; + fn into_comparison_expr(self) -> Expr> { + Expr::new(self.into_expr().node) + } +} +impl ComparisonValue for RelatedColumn> { + type Output = Option; + fn into_comparison_expr(self) -> Expr { + Expr::new(self.into_expr().node) + } +} +/// Comparison operands for relation columns retain their Rust value types. +/// The general ordered-comparison API also accepts untyped SQL literals; paths +/// deliberately require ColumnValue for literals, just like Column::eq. +pub trait PathComparisonValue { + type Output; + fn into_path_expr(self) -> Expr; +} +impl> PathComparisonValue for V { + type Output = T::Output; + fn into_path_expr(self) -> Expr { + Expr::new(ExprNode::Value(self.into_value().unwrap_or(Value::Null))) + } +} +impl> PathComparisonValue for V { + type Output = V::Output; + fn into_path_expr(self) -> Expr { + self.into_comparison_expr() + } +} +impl> PathComparisonValue for V { + type Output = V::Output; + fn into_path_expr(self) -> Expr { + self.into_comparison_expr() + } +} +impl RelatedColumn { + pub fn eq(self, value: V) -> Expr + where + V: PathComparisonValue, + { + self.compare(value, BinaryOp::Eq) + } + pub fn ne(self, value: V) -> Expr + where + V: PathComparisonValue, + { + self.compare(value, BinaryOp::Ne) + } + pub fn lt(self, value: V) -> Expr + where + V: PathComparisonValue, + { + self.compare(value, BinaryOp::Lt) + } + pub fn le(self, value: V) -> Expr + where + V: PathComparisonValue, + { + self.compare(value, BinaryOp::Le) + } + pub fn gt(self, value: V) -> Expr + where + V: PathComparisonValue, + { + self.compare(value, BinaryOp::Gt) + } + pub fn ge(self, value: V) -> Expr + where + V: PathComparisonValue, + { + self.compare(value, BinaryOp::Ge) + } + fn compare(self, value: V, op: BinaryOp) -> Expr + where + V: PathComparisonValue, + { + let right = value.into_path_expr().node; + let left = Box::new(self.into_expr().node); + if matches!(right, ExprNode::Value(Value::Null)) && matches!(op, BinaryOp::Eq | BinaryOp::Ne) { + Expr::new(ExprNode::IsNull { + expr: left, + negated: matches!(op, BinaryOp::Ne), + }) + } else { + Expr::new(ExprNode::Binary { + left, + op, + right: Box::new(right), + }) + } + } +} + +macro_rules! forward_operator { + ($($trait:ident::$method:ident),* $(,)?) => {$( + impl $trait for RelatedColumn + where Column: $trait { + type Output = as $trait>::Output; + fn $method(self, rhs: R) -> Self::Output { self.0.$method(rhs) } + } + )*}; +} +forward_operator!( + Add::add, + Sub::sub, + Mul::mul, + Div::div, + BitAnd::bitand, + BitOr::bitor, + BitXor::bitxor, + Shl::shl, + Shr::shr +); +impl Not for RelatedColumn +where + Column: Not, +{ + type Output = as Not>::Output; + fn not(self) -> Self::Output { + self.0.not() + } +} + +pub fn column(column: crate::ColumnRef, path: &[Relation]) -> ExprNode { + if path.is_empty() { + ExprNode::Column(column) + } else { + ExprNode::RelatedColumn { + column, + path: path.to_vec(), + } + } +} + +pub fn join_on(path: &[Relation]) -> Expr { + let (relation, previous) = path.split_last().expect("nonempty relation path"); + let (target, source) = match relation.kind { + crate::RelationKind::BelongsTo => (relation.parent_key, relation.child_key), + crate::RelationKind::HasMany => (relation.child_key, relation.parent_key), + crate::RelationKind::ManyToMany => unreachable!("bridge joins are expanded before planning"), + }; + Expr::new(ExprNode::Binary { + left: Box::new(column(target, path)), + op: BinaryOp::Eq, + right: Box::new(column(source, previous)), + }) +} + +impl ExprNode { + pub(crate) fn visit_paths(&self, visit: &mut impl FnMut(&[Relation])) { + match self { + Self::Column(col) => { + if let Some(path) = col.path { + visit(&path.steps()); + } + } + Self::RelatedColumn { path, .. } => visit(path), + Self::Row { values } | Self::Func { args: values, .. } => { + for value in values { + value.visit_paths(visit); + } + } + Self::Trim { expr, characters, .. } => { + expr.visit_paths(visit); + if let Some(chars) = characters { + chars.visit_paths(visit); + } + } + Self::AggregateFilter { + aggregate: left, + predicate: right, + } + | Self::VectorBinary { left, right, .. } + | Self::Binary { left, right, .. } + | Self::Bool { left, right, .. } => { + left.visit_paths(visit); + right.visit_paths(visit); + } + Self::Normalize { expr, .. } + | Self::MakeInterval { value: expr, .. } + | Self::Cast { expr, .. } + | Self::Unary { expr, .. } + | Self::In { expr, .. } + | Self::RowIn { expr, .. } + | Self::IsNull { expr, .. } + | Self::Like { expr, .. } => expr.visit_paths(visit), + // Subqueries have already compiled their own paths in their own scope. + Self::Value(_) | Self::Exists { .. } => {} + } + } +} + +enum PlannedJoin { + Declared(crate::Join), + Related { + path: Vec, + alias: String, + kind: crate::JoinKind, + }, +} + +pub(crate) struct JoinPlan { + joins: Vec, + reserved: Vec, +} + +impl JoinPlan { + pub(crate) fn new(base: Table, declared: &[crate::Join], extra: &[crate::Join]) -> Self { + let mut plan = Self { + joins: Vec::new(), + reserved: std::iter::once(base.qualifier().to_owned()) + .chain(declared.iter().chain(extra).map(|join| join.table.qualifier().to_owned())) + .collect(), + }; + for (joins, explicit) in [(declared, true), (extra, false)] { + for join in joins { + if let ExprNode::Binary { + left, op: BinaryOp::Eq, .. + } = &join.on.node + { + if let ExprNode::RelatedColumn { path, column } = &**left { + if column.table == join.table { + plan.require(path, explicit.then_some(join.kind)); + continue; + } + } + } + join.on.node.visit_paths(&mut |path| plan.require(path, None)); + plan.joins.push(PlannedJoin::Declared(join.clone())); + } + } + plan + } + + pub(crate) fn discover(&mut self, expr: &ExprNode) { + expr.visit_paths(&mut |path| self.require(path, None)); + } + + fn require(&mut self, path: &[Relation], explicit: Option) { + if path.is_empty() { + return; + } + if let Some(PlannedJoin::Related { kind, .. }) = self + .joins + .iter_mut() + .find(|join| matches!(join, PlannedJoin::Related { path: existing, .. } if existing == path)) + { + if let Some(declared) = explicit { + *kind = declared; + } + return; + } + self.require(&path[..path.len() - 1], None); + let mut index = self.joins.len(); + let alias = loop { + let alias = format!("__dbkit_r{index}"); + if !self.reserved.contains(&alias) { + break alias; + } + index += 1; + }; + self.reserved.push(alias.clone()); + self.joins.push(PlannedJoin::Related { + path: path.to_vec(), + alias, + kind: explicit.unwrap_or(crate::JoinKind::Left), + }); + } + + pub(crate) fn aliases(&self) -> Vec<(Vec, String)> { + self.joins + .iter() + .filter_map(|join| match join { + PlannedJoin::Related { path, alias, .. } => Some((path.clone(), alias.clone())), + PlannedJoin::Declared(_) => None, + }) + .collect() + } + + pub(crate) fn declared_tables(&self) -> Vec { + self.joins + .iter() + .filter_map(|join| match join { + PlannedJoin::Declared(join) => Some(join.table), + PlannedJoin::Related { .. } => None, + }) + .collect() + } + + pub(crate) fn write(&self, builder: &mut crate::compile::SqlBuilder) { + use crate::compile::ToSql; + for join in &self.joins { + let (table, alias, kind, on) = match join { + PlannedJoin::Declared(join) => (join.table, join.table.alias, join.kind, join.on.clone()), + PlannedJoin::Related { path, alias, kind } => { + (path.last().unwrap().join_table(), Some(alias.as_str()), *kind, join_on(path)) + } + }; + builder.push_sql(match kind { + crate::JoinKind::Inner => " JOIN ", + crate::JoinKind::Left => " LEFT JOIN ", + }); + builder.push_sql(&table.qualified_name()); + if let Some(alias) = alias { + builder.push_sql(" "); + builder.push_sql(alias); + } + builder.push_sql(" ON "); + on.node.to_sql(builder); + } + } +} diff --git a/crates/dbkit-core/src/query.rs b/crates/dbkit-core/src/query.rs index f4f88f8..0037885 100644 --- a/crates/dbkit-core/src/query.rs +++ b/crates/dbkit-core/src/query.rs @@ -202,6 +202,14 @@ impl Select, { + if let Some(path) = rel.path() { + self.joins.push(Join { + table: path.relation.parent, + on: crate::path::join_on(&path.steps()), + kind: JoinKind::Inner, + }); + return self; + } let relation = rel.relation(); for (table, on) in relation.join_steps() { self.joins.push(Join { @@ -217,6 +225,14 @@ impl Select, { + if let Some(path) = rel.path() { + self.joins.push(Join { + table: path.relation.parent, + on: crate::path::join_on(&path.steps()), + kind: JoinKind::Left, + }); + return self; + } let relation = rel.relation(); for (table, on) in relation.join_steps() { self.joins.push(Join { @@ -352,7 +368,24 @@ impl Select CompiledSql { - let mut builder = SqlBuilder::new(); + let mut plan = crate::path::JoinPlan::new(self.table, &self.joins, extra_joins); + for item in self.columns.iter().flatten().chain(extra_columns) { + plan.discover(&item.expr); + } + for expr in self.filters.iter().chain(&self.having) { + plan.discover(&expr.node); + } + for expr in &self.group_by { + plan.discover(expr); + } + if include_order { + for order in &self.order_by { + if let OrderExpr::Expr(expr) = &order.expr { + plan.discover(expr); + } + } + } + let mut builder = SqlBuilder::for_query(self.table, plan.aliases(), plan.declared_tables()); builder.push_sql("SELECT "); if self.distinct { builder.push_sql("DISTINCT "); @@ -401,32 +434,7 @@ impl Select " JOIN ", - JoinKind::Left => " LEFT JOIN ", - }); - builder.push_sql(&join.table.qualified_name()); - if let Some(alias) = join.table.alias { - builder.push_sql(" "); - builder.push_sql(alias); - } - builder.push_sql(" ON "); - join.on.node.to_sql(&mut builder); - } - for join in extra_joins { - builder.push_sql(match join.kind { - JoinKind::Inner => " JOIN ", - JoinKind::Left => " LEFT JOIN ", - }); - builder.push_sql(&join.table.qualified_name()); - if let Some(alias) = join.table.alias { - builder.push_sql(" "); - builder.push_sql(alias); - } - builder.push_sql(" ON "); - join.on.node.to_sql(&mut builder); - } + plan.write(&mut builder); if !self.filters.is_empty() { builder.push_sql(" WHERE "); for (idx, expr) in self.filters.iter().enumerate() { diff --git a/crates/dbkit-core/src/rel.rs b/crates/dbkit-core/src/rel.rs index 7fd97f3..52cd618 100644 --- a/crates/dbkit-core/src/rel.rs +++ b/crates/dbkit-core/src/rel.rs @@ -10,7 +10,7 @@ pub enum RelationKind { ManyToMany, } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct Relation { pub kind: RelationKind, pub parent: Table, @@ -69,6 +69,9 @@ impl Relation { pub trait RelationInfo { type Parent; fn relation(&self) -> Relation; + fn path(&self) -> Option<&'static crate::path::RelationPath> { + None + } } pub trait RelationTarget { @@ -132,55 +135,64 @@ impl RelationTarget for HasMany { type Target = Child; } -#[derive(Debug, Clone, Copy)] -pub struct BelongsTo { - child: Table, - parent: Table, - child_key: ColumnRef, - parent_key: ColumnRef, - _marker: PhantomData<(Child, Parent)>, +#[derive(Debug)] +pub struct BelongsTo(Relation, PhantomData<(Child, Parent, Key)>); + +impl Copy for BelongsTo {} +impl Clone for BelongsTo { + fn clone(&self) -> Self { + *self + } } -impl BelongsTo { +impl BelongsTo { pub const fn new(child: Table, parent: Table, child_key: ColumnRef, parent_key: ColumnRef) -> Self { - Self { - child, - parent, - child_key, - parent_key, - _marker: PhantomData, - } + Self( + Relation { + kind: RelationKind::BelongsTo, + parent, + child, + parent_key, + child_key, + join_table: None, + join_parent_key: None, + join_child_key: None, + }, + PhantomData, + ) + } + pub const fn descriptor(&self) -> Relation { + self.0 } - pub fn selectin(self) -> crate::load::SelectIn { crate::load::SelectIn::new(self) } - pub fn joined(self) -> crate::load::Joined { crate::load::Joined::new(self) } } -impl RelationInfo for BelongsTo { +impl RelationInfo for BelongsTo { type Parent = Child; - fn relation(&self) -> Relation { - Relation { - kind: RelationKind::BelongsTo, - parent: self.parent, - child: self.child, - parent_key: self.parent_key, - child_key: self.child_key, - join_table: None, - join_parent_key: None, - join_child_key: None, - } + self.0 + } + fn path(&self) -> Option<&'static crate::path::RelationPath> { + Key::PATH } } - -impl RelationTarget for BelongsTo { +impl RelationTarget for BelongsTo { type Target = Parent; } +impl std::ops::Deref for BelongsTo +where + Parent: crate::path::RelationFields, +{ + type Target = Parent::Fields; + fn deref(&self) -> &Self::Target { + Parent::FIELDS + } +} pub trait ManyToManyThrough { type Through; diff --git a/crates/dbkit-core/src/schema.rs b/crates/dbkit-core/src/schema.rs index 8c1db8e..20c3c10 100644 --- a/crates/dbkit-core/src/schema.rs +++ b/crates/dbkit-core/src/schema.rs @@ -46,11 +46,12 @@ impl Table { pub struct ColumnRef { pub table: Table, pub name: &'static str, + pub path: Option<&'static crate::path::RelationPath>, } impl ColumnRef { pub const fn new(table: Table, name: &'static str) -> Self { - Self { table, name } + Self { table, name, path: None } } pub fn qualified_name(&self) -> String { @@ -58,10 +59,11 @@ impl ColumnRef { } } -#[derive(Debug, Clone, Copy)] +#[derive(Debug)] pub struct Column { pub table: Table, pub name: &'static str, + path: Option<&'static crate::path::RelationPath>, _marker: PhantomData<(M, T)>, } @@ -70,11 +72,29 @@ impl Column { Self { table, name, + path: None, _marker: PhantomData, } } pub const fn as_ref(&self) -> ColumnRef { - ColumnRef::new(self.table, self.name) + ColumnRef { + table: self.table, + name: self.name, + path: self.path, + } + } +} + +impl Copy for Column {} +impl Clone for Column { + fn clone(&self) -> Self { + *self + } +} +impl Column { + pub(crate) const fn with_path(mut self, path: Option<&'static crate::path::RelationPath>) -> Self { + self.path = path; + self } } diff --git a/crates/dbkit-derive/src/lib.rs b/crates/dbkit-derive/src/lib.rs index 6e664e1..f6133ff 100644 --- a/crates/dbkit-derive/src/lib.rs +++ b/crates/dbkit-derive/src/lib.rs @@ -594,6 +594,8 @@ fn expand_model(args: ModelArgs, input: ItemStruct) -> syn::Result }; quote!( pub mod #state_mod { + #[derive(Debug, Clone, Copy)] + pub struct Key; mod sealed { pub trait Sealed {} impl Sealed for ::dbkit::NotLoaded {} @@ -856,6 +858,7 @@ fn expand_model(args: ModelArgs, input: ItemStruct) -> syn::Result let set_relation_impls = relation_fields.iter().map(|rel| { let field_ident = rel.field.ident.as_ref().expect("field ident"); let child_type = &rel.child_type; + let relation_key = &rel.state_mod_ident; let item_ident = format_ident!("{}Item", to_camel_case(&field_ident.to_string())); let (value_ty, rel_ty) = match rel.kind { RelationKind::HasMany => ( @@ -871,7 +874,7 @@ fn expand_model(args: ModelArgs, input: ItemStruct) -> syn::Result } RelationKind::BelongsTo => ( quote!(Option<#item_ident>), - quote!(::dbkit::rel::BelongsTo<#default_model_ty, #child_type>), + quote!(::dbkit::rel::BelongsTo<#default_model_ty, #child_type, #relation_key::Key>), ), }; @@ -916,6 +919,7 @@ fn expand_model(args: ModelArgs, input: ItemStruct) -> syn::Result let get_relation_impls = relation_fields.iter().map(|rel| { let field_ident = rel.field.ident.as_ref().expect("field ident"); let child_type = &rel.child_type; + let relation_key = &rel.state_mod_ident; let item_ident = format_ident!("{}Item", to_camel_case(&field_ident.to_string())); let (value_ty, rel_ty) = match rel.kind { RelationKind::HasMany => ( @@ -931,7 +935,7 @@ fn expand_model(args: ModelArgs, input: ItemStruct) -> syn::Result } RelationKind::BelongsTo => ( quote!(Option<#item_ident>), - quote!(::dbkit::rel::BelongsTo<#default_model_ty, #child_type>), + quote!(::dbkit::rel::BelongsTo<#default_model_ty, #child_type, #relation_key::Key>), ), }; @@ -992,9 +996,10 @@ fn expand_model(args: ModelArgs, input: ItemStruct) -> syn::Result let load_relation_impls = relation_fields.iter().map(|rel| { let field_ident = rel.field.ident.as_ref().expect("field ident"); let child_type = &rel.child_type; + let relation_key = &rel.state_mod_ident; let rel_type = match rel.kind { RelationKind::HasMany => quote!(::dbkit::rel::HasMany<#default_model_ty, #child_type>), - RelationKind::BelongsTo => quote!(::dbkit::rel::BelongsTo<#default_model_ty, #child_type>), + RelationKind::BelongsTo => quote!(::dbkit::rel::BelongsTo<#default_model_ty, #child_type, #relation_key::Key>), RelationKind::ManyToMany => { let through = rel.many_to_many_through.as_ref().expect("many-to-many through"); quote!(::dbkit::rel::ManyToMany<#default_model_ty, #child_type, #through>) @@ -1094,6 +1099,7 @@ fn expand_model(args: ModelArgs, input: ItemStruct) -> syn::Result let relation_consts = relation_fields.iter().filter_map(|rel| { let field_ident = rel.field.ident.as_ref().expect("field ident"); let child_type = &rel.child_type; + let relation_key = &rel.state_mod_ident; match rel.kind { RelationKind::HasMany => Some(quote!( pub const #field_ident: ::dbkit::rel::HasMany<#default_model_ty, #child_type> = @@ -1108,7 +1114,7 @@ fn expand_model(args: ModelArgs, input: ItemStruct) -> syn::Result let key = rel.belongs_to_key.as_ref().expect("belongs_to key"); let references = rel.belongs_to_ref.as_ref().expect("belongs_to references"); Some(quote!( - pub const #field_ident: ::dbkit::rel::BelongsTo<#default_model_ty, #child_type> = + pub const #field_ident: ::dbkit::rel::BelongsTo<#default_model_ty, #child_type, #relation_key::Key> = ::dbkit::rel::BelongsTo::new( Self::TABLE, #child_type::TABLE, @@ -1143,6 +1149,15 @@ fn expand_model(args: ModelArgs, input: ItemStruct) -> syn::Result return None; } let parent_type = &rel.child_type; + // An inverse has-many can infer its foreign key only for an unambiguous target. + if relation_fields + .iter() + .filter(|other| other.kind == RelationKind::BelongsTo && other.child_type == *parent_type) + .count() + != 1 + { + return None; + } let key = rel.belongs_to_key.as_ref().expect("belongs_to key"); let references = rel.belongs_to_ref.as_ref().expect("belongs_to references"); Some(quote!( @@ -1157,9 +1172,10 @@ fn expand_model(args: ModelArgs, input: ItemStruct) -> syn::Result let apply_load_impls = relation_fields.iter().flat_map(|rel| { let child_type = &rel.child_type; + let relation_key = &rel.state_mod_ident; let rel_type = match rel.kind { RelationKind::HasMany => quote!(::dbkit::rel::HasMany<#default_model_ty, #child_type>), - RelationKind::BelongsTo => quote!(::dbkit::rel::BelongsTo<#default_model_ty, #child_type>), + RelationKind::BelongsTo => quote!(::dbkit::rel::BelongsTo<#default_model_ty, #child_type, #relation_key::Key>), RelationKind::ManyToMany => { let through = rel.many_to_many_through.as_ref().expect("many-to-many through"); quote!(::dbkit::rel::ManyToMany<#default_model_ty, #child_type, #through>) @@ -1223,10 +1239,11 @@ fn expand_model(args: ModelArgs, input: ItemStruct) -> syn::Result let run_load_impls = relation_fields.iter().flat_map(|rel| { let child_type = &rel.child_type; + let relation_key = &rel.state_mod_ident; let through = rel.many_to_many_through.as_ref(); let rel_type = match rel.kind { RelationKind::HasMany => quote!(::dbkit::rel::HasMany<#default_model_ty, #child_type>), - RelationKind::BelongsTo => quote!(::dbkit::rel::BelongsTo<#default_model_ty, #child_type>), + RelationKind::BelongsTo => quote!(::dbkit::rel::BelongsTo<#default_model_ty, #child_type, #relation_key::Key>), RelationKind::ManyToMany => { let through = through.expect("many-to-many through"); quote!(::dbkit::rel::ManyToMany<#default_model_ty, #child_type, #through>) @@ -1337,6 +1354,35 @@ fn expand_model(args: ModelArgs, input: ItemStruct) -> syn::Result items.into_iter() }); + let path_fields_ident = format_ident!("{}RelationFields", model_ident); + let mut path_fields = Vec::new(); + let mut path_values = Vec::new(); + for field in output_fields.iter().filter(|field| !is_relation_field(field, &relation_fields)) { + let ident = field.ident.as_ref().expect("field ident"); + let name = scalar_column_name(&scalar_fields, ident).expect("column name"); + let inner = option_inner_type(&field.ty).unwrap_or_else(|| field.ty.clone()); + path_fields.push(quote!(pub #ident: ::dbkit::path::RelatedColumn<__DbkitPath, ::core::option::Option<#inner>>)); + path_values.push(quote!(#ident: ::dbkit::path::RelatedColumn::new(#default_model_path::TABLE, #name))); + } + let mut path_keys = Vec::new(); + for rel in &relation_fields { + if rel.kind != RelationKind::BelongsTo { + continue; + } + let ident = rel.field.ident.as_ref().expect("field ident"); + let key = &rel.state_mod_ident; + let target = &rel.child_type; + path_fields.push(quote!(pub #ident: ::dbkit::path::Related<::dbkit::path::Next<__DbkitPath, #key::Key>, #target>)); + path_values.push(quote!(#ident: ::dbkit::path::Related::NEW)); + path_keys.push(quote!( + impl ::dbkit::path::PathKey for #key::Key { + const PATH: ::core::option::Option<&'static ::dbkit::path::RelationPath> = ::core::option::Option::Some( + &::dbkit::path::RelationPath { previous: ::core::option::Option::None, relation: #default_model_path::#ident.descriptor() } + ); + } + )); + } + let output = quote! { #(#struct_attrs)* #[derive(Debug, Clone)] @@ -1345,6 +1391,19 @@ fn expand_model(args: ModelArgs, input: ItemStruct) -> syn::Result } #(#relation_state_modules)* + #(#path_keys)* + #[doc(hidden)] + #vis struct #path_fields_ident<__DbkitPath: ::dbkit::path::PathKey> { + #(#path_fields,)* + __dbkit_marker: ::core::marker::PhantomData<__DbkitPath>, + } + impl<__DbkitPath: ::dbkit::path::PathKey> ::dbkit::path::RelationFields<__DbkitPath> for #default_model_ty { + type Fields = #path_fields_ident<__DbkitPath>; + const FIELDS: &'static Self::Fields = &#path_fields_ident { + #(#path_values,)* + __dbkit_marker: ::core::marker::PhantomData, + }; + } #vis trait #any_state_ident {} impl #impl_generics #any_state_ident for #model_ident #struct_type_args {} diff --git a/crates/dbkit/src/joined.rs b/crates/dbkit/src/joined.rs index 4f0a796..54935d5 100644 --- a/crates/dbkit/src/joined.rs +++ b/crates/dbkit/src/joined.rs @@ -1,5 +1,4 @@ use crate::executor::{build_arguments, BoxFuture}; -use crate::expr::ExprNode; use crate::load::{ApplyLoad, Joined, LoadChain, NoLoad, SelectIn}; use crate::query::{Join, JoinKind, SelectItem}; use crate::rel::RelationInfo; @@ -199,6 +198,7 @@ pub(crate) struct JoinContext { pub extra_joins: Vec, joined_tables: Vec
, next_alias: usize, + current_path: Vec, } impl JoinContext { @@ -209,6 +209,7 @@ impl JoinContext { extra_joins: Vec::new(), joined_tables, next_alias: 0, + current_path: Vec::new(), } } @@ -226,11 +227,59 @@ impl JoinContext { self.extra_joins.push(join); } + fn enter_relation(&mut self, relation: crate::Relation, scoped: bool) { + if !scoped && self.current_path.is_empty() { + for (table, on) in relation.join_steps() { + self.ensure_join(Join { + table, + on, + kind: JoinKind::Left, + }); + } + return; + } + let steps = if relation.kind == crate::RelationKind::ManyToMany { + let bridge = relation.join_table.expect("join table"); + vec![ + crate::Relation { + kind: crate::RelationKind::HasMany, + parent: relation.parent, + child: bridge, + parent_key: relation.parent_key, + child_key: relation.join_parent_key.unwrap(), + join_table: None, + join_parent_key: None, + join_child_key: None, + }, + crate::Relation { + kind: crate::RelationKind::BelongsTo, + parent: relation.child, + child: bridge, + parent_key: relation.child_key, + child_key: relation.join_child_key.unwrap(), + join_table: None, + join_parent_key: None, + join_child_key: None, + }, + ] + } else { + vec![relation] + }; + for step in steps { + self.current_path.push(step); + self.extra_joins.push(Join { + table: step.join_table(), + on: crate::path::join_on(&self.current_path), + kind: JoinKind::Left, + }); + } + } + fn add_columns(&mut self, prefix: &str) { for column in T::joined_columns() { let alias = format!("{}{}", prefix, column.name); self.extra_columns.push(SelectItem { - expr: ExprNode::Column(*column), + expr: crate::path::column(*column, &self.current_path), alias: Some(alias), }); } @@ -397,20 +446,15 @@ where Child: Clone + Send + 'static, { fn build_collectors(&self, ctx: &mut JoinContext) -> Vec>> { - let relation = self.rel.relation(); - for (table, on) in relation.join_steps() { - ctx.ensure_join(Join { - table, - on, - kind: JoinKind::Left, - }); - } + let previous_path = ctx.current_path.clone(); + ctx.enter_relation(self.rel.relation(), self.rel.path().is_some()); let prefix = ctx.next_prefix(); ctx.add_columns::<>::Out2>(&prefix); let nested = >::Out2>>::build_collectors(&self.nested, ctx); + ctx.current_path = previous_path; let collector = HasManyCollector { rel: self.rel.clone(), prefix, @@ -420,31 +464,27 @@ where } } -impl BuildJoinPlan for Joined, Nested> +impl BuildJoinPlan for Joined, Nested> where Nested: ApplyLoad + BuildJoinPlan<>::Out2>, >::Out2: JoinedModel + 'static, - ChildOut: GetRelation, Option<>::Out2>> + 'static, + ChildOut: GetRelation, Option<>::Out2>> + 'static, Child: Clone + Send + 'static, Parent: Clone + Send + 'static, + Key: crate::path::PathKey, { fn build_collectors(&self, ctx: &mut JoinContext) -> Vec>> { - let relation = self.rel.relation(); - for (table, on) in relation.join_steps() { - ctx.ensure_join(Join { - table, - on, - kind: JoinKind::Left, - }); - } + let previous_path = ctx.current_path.clone(); + ctx.enter_relation(self.rel.relation(), self.rel.path().is_some()); let prefix = ctx.next_prefix(); ctx.add_columns::<>::Out2>(&prefix); let nested = >::Out2>>::build_collectors(&self.nested, ctx); + ctx.current_path = previous_path; let collector = BelongsToCollector { - rel: self.rel.clone(), + rel: self.rel, prefix, nested, }; @@ -462,20 +502,15 @@ where Through: Clone + Send + 'static, { fn build_collectors(&self, ctx: &mut JoinContext) -> Vec>> { - let relation = self.rel.relation(); - for (table, on) in relation.join_steps() { - ctx.ensure_join(Join { - table, - on, - kind: JoinKind::Left, - }); - } + let previous_path = ctx.current_path.clone(); + ctx.enter_relation(self.rel.relation(), self.rel.path().is_some()); let prefix = ctx.next_prefix(); ctx.add_columns::<>::Out2>(&prefix); let nested = >::Out2>>::build_collectors(&self.nested, ctx); + ctx.current_path = previous_path; let collector = ManyToManyCollector { rel: self.rel.clone(), prefix, diff --git a/crates/dbkit/tests/integration_relation_paths.rs b/crates/dbkit/tests/integration_relation_paths.rs index adc1340..da44ef7 100644 --- a/crates/dbkit/tests/integration_relation_paths.rs +++ b/crates/dbkit/tests/integration_relation_paths.rs @@ -410,6 +410,14 @@ async fn explicit_inner_joins_and_existing_table_column_filters_still_work() -> .await?; assert_eq!(legacy.iter().map(|row| row.id).collect::>(), [1, 3, 4]); assert_eq!(mixed.iter().map(|row| row.id).collect::>(), [1, 3]); + + let custom: Vec = Record::query() + .left_join_on(Member::TABLE, Member::id.eq_col(Record::owner_id).and(Member::score.gt(25_i32))) + .filter(Member::note.is_null()) + .filter(Record::owner.code.eq("c")) + .all(&tx) + .await?; + assert_eq!(custom.iter().map(|row| row.id).collect::>(), [4]); tx.rollback().await?; Ok(()) } diff --git a/crates/dbkit/tests/relation_paths_sql.rs b/crates/dbkit/tests/relation_paths_sql.rs index 91ba787..97be332 100644 --- a/crates/dbkit/tests/relation_paths_sql.rs +++ b/crates/dbkit/tests/relation_paths_sql.rs @@ -164,6 +164,25 @@ fn explicit_join_kind_is_preserved_without_an_extra_implicit_join() { assert_eq!(outer.sql.matches("LEFT JOIN").count(), 1); } +#[test] +fn custom_table_join_keeps_its_columns_when_a_relation_path_uses_the_same_table() { + // A custom ON condition is not interchangeable with the declared relation. + let compiled = Record::query() + .left_join_on(Member::TABLE, Member::id.eq_col(Record::owner_id).and(Member::score.gt(25_i32))) + .filter(Member::note.is_null()) + .filter(Record::owner.code.eq("c")) + .compile(); + assert!( + compiled + .sql + .contains("LEFT JOIN path_members ON ((path_members.id = path_records.owner_id)"), + "{}", + compiled.sql + ); + assert!(compiled.sql.contains("WHERE (path_members.note IS NULL)"), "{}", compiled.sql); + assert_eq!(compiled.sql.matches("JOIN path_members ").count(), 2); +} + #[test] fn nested_paths_share_their_prefix_and_join_in_dependency_order() { let compiled = Record::query() diff --git a/docs/relations.md b/docs/relations.md index 0a29ae6..a77dafd 100644 --- a/docs/relations.md +++ b/docs/relations.md @@ -58,6 +58,49 @@ let filtered = User::query() .await?; ``` +## Relation Column Paths + +Access a `BelongsTo` target's columns through the relation itself: + +```rust,ignore +let todos = Todo::query() + .filter(Todo::user.name.eq("Sam")) + .with(Todo::user.joined()) + .all(&db) + .await?; +``` + +Using a relation column adds a `LEFT JOIN` automatically. `.with(...)` separately +controls loading: without it, `todo.user` remains `NotLoaded`. You can also use +`.selectin()` to load the filtered results' users in a separate batched query. + +Relation paths work in filters, expressions, projections, ordering, grouping, and +aggregate predicates. They can traverse multiple `BelongsTo` relations, including +self-relations: + +```rust,ignore +let records = Record::query() + .filter(Record::owner.organization.name.eq("Acme")) + .order_by(dbkit::Order::asc(Record::owner.name)) + .all(&db) + .await?; +``` + +Repeated uses of a path share its join, including joins needed for eager loading. +An explicit `.join(relation)` or `.left_join(relation)` is reused with its declared +join kind. Different foreign keys to the same model get separate aliases and +loaded fields. Custom `join_on(...)` conditions keep their own joins and columns. +The referenced key must be unique so each `BelongsTo` matches at most one row. + +Related columns are nullable expressions because an outer join can find no target, +even if the target column is required. `Todo::user.id.is_null()` finds missing users; +an already nullable column stays `Option`, not `Option>`. + +Paths are supported in SELECT queries and start at the current query's model. +They are read-only and cannot be passed directly to `.set(...)`. Collection filters still use explicit joins or +`.where_exists(...)`. When multiple `BelongsTo` fields reference the same model, +an inverse `#[has_many]` cannot infer which foreign key to use. + ## Nested Eager Loading Parent -> children -> grandchildren loading is reflected in the result type: From 6a39445abb8b4f50e0fe9674f142fbd787671179 Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Mon, 7 Sep 2026 11:49:04 +0200 Subject: [PATCH 03/22] Test correlated relation column paths --- .../dbkit/tests/integration_relation_paths.rs | 214 ++++++++++++++++++ crates/dbkit/tests/relation_paths_sql.rs | 201 ++++++++++++++++ 2 files changed, 415 insertions(+) diff --git a/crates/dbkit/tests/integration_relation_paths.rs b/crates/dbkit/tests/integration_relation_paths.rs index da44ef7..66f8d9c 100644 --- a/crates/dbkit/tests/integration_relation_paths.rs +++ b/crates/dbkit/tests/integration_relation_paths.rs @@ -51,6 +51,220 @@ async fn setup(ex: &(impl Executor + Send + Sync)) -> Result<(), Error> { Ok(()) } +async fn setup_correlated(ex: &(impl Executor + Send + Sync)) -> Result<(), Error> { + setup(ex).await?; + // The original fixture gives every member a record. Add unmatched outer rows + // so accidentally uncorrelated EXISTS queries cannot pass by returning everyone. + ex.execute( + "INSERT INTO path_members VALUES (5, 1, 'atlas', false, 0, NULL, 'e')", + PgArguments::default(), + ) + .await?; + ex.execute("INSERT INTO path_organizations VALUES (3, 'unused')", PgArguments::default()) + .await?; + Ok(()) +} + +#[tokio::test] +async fn correlated_exists_returns_only_members_with_matching_records() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup_correlated(&tx).await?; + + for inner in [ + Record::query(), + Record::query().join(Record::owner), + Record::query().left_join(Record::owner), + ] { + let members: Vec = Member::query() + .where_exists(inner.filter(Record::owner.id.eq(Member::id))) + .order_by(Order::asc(Member::id)) + .all(&tx) + .await?; + assert_eq!(members.iter().map(|row| row.id).collect::>(), [1, 2, 3, 4]); + } + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn correlated_not_exists_returns_unmatched_members_despite_missing_record_owners() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup_correlated(&tx).await?; + + let members: Vec = Member::query() + .where_not_exists(Record::query().filter(Record::owner.id.eq(Member::id))) + .order_by(Order::asc(Member::id)) + .all(&tx) + .await?; + // NULL and dangling owner IDs in records 5 and 6 must not match any member. + assert_eq!(members.iter().map(|row| row.id).collect::>(), [5]); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn correlated_exists_projection_is_evaluated_for_each_outer_row() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup_correlated(&tx).await?; + + let rows: Vec<(i64, bool)> = Member::query() + .select_only() + .column(Member::id) + .column_as(func::exists(Record::query().filter(Record::owner.id.eq(Member::id))), "has_records") + .order_by(Order::asc(Member::id)) + .into_model() + .all(&tx) + .await?; + assert_eq!(rows, [(1, true), (2, true), (3, true), (4, true), (5, false)]); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn correlated_renamed_column_and_local_predicate_select_the_matching_member() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup_correlated(&tx).await?; + + let members: Vec = Member::query() + .where_exists( + Record::query() + .filter(Record::owner.code.eq(Member::code)) + .filter(Record::owner.score.gt(25_i32)), + ) + .order_by(Order::asc(Member::id)) + .all(&tx) + .await?; + assert_eq!(members.iter().map(|row| row.id).collect::>(), [1]); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn correlated_computed_comparison_uses_the_outer_text_value() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup_correlated(&tx).await?; + + let members: Vec = Member::query() + .where_exists(Record::query().filter(func::lower(Record::owner.label).eq_col(Member::label))) + .order_by(Order::asc(Member::id)) + .all(&tx) + .await?; + // Member 5's label is the lowercase version of member 1's label. Comparing + // the owner's normalized label to its own original label would find nobody. + assert_eq!(members.iter().map(|row| row.id).collect::>(), [5]); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn correlated_nullable_comparison_does_not_compare_the_inner_column_to_itself() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup_correlated(&tx).await?; + + let members: Vec = Member::query() + .where_exists( + Record::query() + .filter(Record::id.eq(1_i64)) + .filter(Record::owner.note.is_distinct_from_col(Member::note)), + ) + .order_by(Order::asc(Member::id)) + .all(&tx) + .await?; + // Record 1's owner has a NULL note; only members 2 and 3 have non-NULL notes. + assert_eq!(members.iter().map(|row| row.id).collect::>(), [2, 3]); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn correlated_nested_relation_excludes_an_organization_without_records() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup_correlated(&tx).await?; + + let organizations: Vec = Organization::query() + .where_exists(Record::query().filter(Record::owner.organization.id.eq(Organization::id))) + .order_by(Order::asc(Organization::id)) + .all(&tx) + .await?; + assert_eq!(organizations.iter().map(|row| row.id).collect::>(), [1, 2]); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn correlated_nested_exists_resolves_both_enclosing_models() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup_correlated(&tx).await?; + + let members: Vec = Member::query() + .where_exists( + Organization::query().where_exists( + Record::query() + .filter(Record::owner.id.eq(Member::id)) + .filter(Record::owner.organization_id.eq(Organization::id)), + ), + ) + .order_by(Order::asc(Member::id)) + .all(&tx) + .await?; + // Member 4 has a record but no organization; member 5 has no record. + assert_eq!(members.iter().map(|row| row.id).collect::>(), [1, 2, 3]); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn correlated_explicit_outer_alias_is_not_shadowed_by_an_automatic_alias() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup_correlated(&tx).await?; + + for outer_alias in ["outer_member", "__dbkit_r0"] { + let table = Member::TABLE.with_alias(outer_alias); + let outer_id = dbkit::Column::::new(table, "id"); + let members: Vec = dbkit::Select::new(table) + .where_exists(Record::query().filter(Record::owner.id.eq(outer_id))) + .order_by(Order::asc(outer_id)) + .all(&tx) + .await?; + assert_eq!( + members.iter().map(|row| row.id).collect::>(), + [1, 2, 3, 4], + "outer alias: {outer_alias}" + ); + } + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn correlated_sibling_paths_keep_their_local_comparison_and_outer_identity() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup_correlated(&tx).await?; + + let members: Vec = Member::query() + .where_exists( + Assignment::query() + .filter(Assignment::first.id.eq(Member::id)) + .filter(Assignment::first.score.gt(Assignment::second.score)), + ) + .order_by(Order::asc(Member::id)) + .all(&tx) + .await?; + assert_eq!(members.iter().map(|row| row.id).collect::>(), [1, 3]); + tx.rollback().await?; + Ok(()) +} + #[tokio::test] async fn filtering_is_independent_of_loading_strategy() -> Result<(), Error> { let db = Database::connect(&db_url()).await?; diff --git a/crates/dbkit/tests/relation_paths_sql.rs b/crates/dbkit/tests/relation_paths_sql.rs index 97be332..03020af 100644 --- a/crates/dbkit/tests/relation_paths_sql.rs +++ b/crates/dbkit/tests/relation_paths_sql.rs @@ -304,6 +304,207 @@ fn subquery_paths_stay_in_the_subquery_and_preserve_correlated_base_columns() { assert_eq!(compiled.binds, vec![Value::String("north".into()), Value::Bool(true)]); } +#[test] +fn correlated_exists_preserves_outer_columns_with_automatic_and_declared_relation_joins() { + for inner in [ + Record::query(), + Record::query().join(Record::owner), + Record::query().left_join(Record::owner), + ] { + let compiled = Member::query() + .where_exists(inner.filter(Record::owner.id.eq(Member::id))) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + assert!( + compiled.sql.contains(&format!("({owner}.id = path_members.id)")), + "{}", + compiled.sql + ); + assert!(!compiled.sql.contains(&format!("({owner}.id = {owner}.id)")), "{}", compiled.sql); + assert!(compiled.binds.is_empty()); + } +} + +#[test] +fn correlated_not_exists_keeps_outer_filters_and_bind_order() { + let compiled = Member::query() + .filter(Member::label.eq("Atlas")) + .where_not_exists( + Record::query() + .filter(Record::owner.id.eq(Member::id)) + .filter(Record::owner.score.gt(25_i32)), + ) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + assert!(compiled.sql.contains("NOT (EXISTS (")); + assert!( + compiled.sql.contains(&format!("({owner}.id = path_members.id)")), + "{}", + compiled.sql + ); + assert!(compiled.sql.contains(&format!("({owner}.score > $2)")), "{}", compiled.sql); + assert_eq!(compiled.binds, vec![Value::String("Atlas".into()), Value::I32(25)]); +} + +#[test] +fn correlated_renamed_computed_and_nullable_columns_keep_the_outer_qualifier() { + let compiled = Member::query() + .where_exists( + Record::query() + .filter(Record::owner.code.eq(Member::code)) + .filter(func::lower(Record::owner.label).eq_col(Member::label)) + .filter(Member::score.gt(Record::owner.score)) + .filter(Record::owner.note.is_distinct_from_col(Member::note)), + ) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + for expected in [ + format!("({owner}.external_ref = path_members.external_ref)"), + format!("(LOWER({owner}.label) = path_members.label)"), + format!("(path_members.score > {owner}.score)"), + format!("({owner}.note IS DISTINCT FROM path_members.note)"), + ] { + assert!(compiled.sql.contains(&expected), "missing {expected}: {}", compiled.sql); + } +} + +#[test] +fn correlated_nested_relation_keeps_the_outer_target_table() { + let compiled = Organization::query() + .where_exists(Record::query().filter(Record::owner.organization.id.eq(Organization::id))) + .compile(); + only_alias(&compiled.sql, "path_members"); + let organization = only_alias(&compiled.sql, "path_organizations"); + assert!( + compiled.sql.contains(&format!("({organization}.id = path_organizations.id)")), + "{}", + compiled.sql + ); +} + +#[test] +fn correlated_columns_can_reference_both_enclosing_query_levels() { + let compiled = Member::query() + .where_exists( + Organization::query().where_exists( + Record::query() + .filter(Record::owner.id.eq(Member::id)) + .filter(Record::owner.organization_id.eq(Organization::id)), + ), + ) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + assert_eq!(compiled.sql.matches("EXISTS (").count(), 2); + assert!( + compiled.sql.contains(&format!("({owner}.id = path_members.id)")), + "{}", + compiled.sql + ); + assert!( + compiled.sql.contains(&format!("({owner}.organization_id = path_organizations.id)")), + "{}", + compiled.sql + ); +} + +#[test] +fn correlated_exists_projection_preserves_outer_column_identity() { + let compiled = Member::query() + .select_only() + .column(Member::id) + .column_as(func::exists(Record::query().filter(Record::owner.id.eq(Member::id))), "has_records") + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + assert!(compiled.sql.starts_with("SELECT path_members.id, EXISTS (")); + assert!(compiled.sql.contains(" AS has_records FROM path_members")); + assert!( + compiled.sql.contains(&format!("({owner}.id = path_members.id)")), + "{}", + compiled.sql + ); +} + +#[test] +fn correlated_outer_alias_cannot_be_shadowed_by_an_automatic_join_alias() { + // Include a name currently used by the automatic allocator. Its spelling is + // not a contract; respecting an explicitly named enclosing table is. + for outer_alias in ["outer_member", "__dbkit_r0"] { + let table = Member::TABLE.with_alias(outer_alias); + let outer_id = dbkit::Column::::new(table, "id"); + let compiled = dbkit::Select::::new(table) + .where_exists(Record::query().filter(Record::owner.id.eq(outer_id))) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + assert_ne!(owner, outer_alias, "inner alias shadows outer table: {}", compiled.sql); + assert!( + compiled.sql.contains(&format!("({owner}.id = {outer_alias}.id)")), + "{}", + compiled.sql + ); + } +} + +#[test] +fn correlated_reused_exists_expression_is_resolved_in_each_enclosing_scope() { + let inner = Record::query().filter(Record::owner.id.eq(Member::id)); + let standalone = inner.compile(); + let predicate = func::exists(inner.clone()); + let correlated = Member::query().filter(predicate.clone()).compile(); + let local = Organization::query().filter(predicate).compile(); + + let owner = only_alias(&correlated.sql, "path_members"); + assert!( + correlated.sql.contains(&format!("({owner}.id = path_members.id)")), + "{}", + correlated.sql + ); + // No enclosing Member binding exists here, so the legacy local table-column + // spelling still refers to this query's joined owner. Compilation must not mutate the reusable expression. + let local_owner = only_alias(&local.sql, "path_members"); + assert!( + local.sql.contains(&format!("({local_owner}.id = {local_owner}.id)")), + "{}", + local.sql + ); + assert_eq!(inner.compile(), standalone); +} + +#[test] +fn correlated_sibling_paths_keep_local_comparisons_distinct_from_outer_columns() { + let compiled = Member::query() + .where_exists( + Assignment::query() + .filter(Assignment::first.id.eq(Member::id)) + .filter(Assignment::first.score.gt(Assignment::second.score)), + ) + .compile(); + let members = aliases(&compiled.sql, "path_members"); + assert_eq!(members.len(), 2); + let first = members + .iter() + .find(|alias| compiled.sql.contains(&format!("({alias}.id = path_assignments.first_id)"))) + .unwrap(); + let second = members + .iter() + .find(|alias| { + compiled + .sql + .contains(&format!("({alias}.external_ref = path_assignments.second_code)")) + }) + .unwrap(); + assert_ne!(first, second); + assert!( + compiled.sql.contains(&format!("({first}.id = path_members.id)")), + "{}", + compiled.sql + ); + assert!( + compiled.sql.contains(&format!("({first}.score > {second}.score)")), + "{}", + compiled.sql + ); +} + #[test] fn compiling_and_cloning_do_not_accumulate_joins_or_change_aliases() { let query = Record::query().filter(Record::owner.enabled.eq(true)); From fee754eb6224817d55042ed18d2e98c2ec60f852 Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Mon, 7 Sep 2026 11:58:53 +0200 Subject: [PATCH 04/22] Fix correlated relation column scopes --- crates/dbkit-core/src/compile.rs | 36 +++++++++++++++++-- crates/dbkit-core/src/expr.rs | 3 +- crates/dbkit-core/src/func.rs | 10 +++--- crates/dbkit-core/src/mutation.rs | 4 +-- crates/dbkit-core/src/path.rs | 5 +-- crates/dbkit-core/src/query.rs | 46 +++++++++++++++++++----- crates/dbkit/tests/relation_paths_sql.rs | 16 +++++++++ 7 files changed, 96 insertions(+), 24 deletions(-) diff --git a/crates/dbkit-core/src/compile.rs b/crates/dbkit-core/src/compile.rs index 80cbff0..e59b2b0 100644 --- a/crates/dbkit-core/src/compile.rs +++ b/crates/dbkit-core/src/compile.rs @@ -7,6 +7,13 @@ pub struct CompiledSql { pub binds: Vec, } +/// Enclosing table bindings and SQL qualifiers visible to a subquery. +#[derive(Debug, Default, Clone)] +pub(crate) struct QueryScope { + tables: Vec, + pub(crate) qualifiers: Vec, +} + #[derive(Debug, Default)] pub struct SqlBuilder { sql: String, @@ -14,6 +21,7 @@ pub struct SqlBuilder { base_table: Option, relation_aliases: Vec<(Vec, String)>, declared_tables: Vec, + outer_scope: QueryScope, } impl SqlBuilder { @@ -21,6 +29,13 @@ impl SqlBuilder { Self::default() } + pub(crate) fn for_table(table: crate::Table) -> Self { + Self { + base_table: Some(table), + ..Self::default() + } + } + pub fn push_sql(&mut self, fragment: &str) { self.sql.push_str(fragment); } @@ -62,11 +77,13 @@ impl SqlBuilder { base: crate::Table, relation_aliases: Vec<(Vec, String)>, declared_tables: Vec, + outer_scope: QueryScope, ) -> Self { Self { base_table: Some(base), relation_aliases, declared_tables, + outer_scope, ..Self::default() } } @@ -76,11 +93,12 @@ impl SqlBuilder { self.push_related_column(col, &path.steps()); return; } - // Preserve table-qualified queries when exactly one relation supplies that - // table. The base table always keeps its own identity, including self joins. + // An enclosing table binding takes precedence over the legacy shorthand + // for a local relation target. Explicit paths always identify local joins. let mut matches = self.relation_aliases.iter().filter(|(path, _)| { Some(col.table) != self.base_table && !self.declared_tables.contains(&col.table) + && !self.outer_scope.tables.contains(&col.table) && path.last().is_some_and(|rel| rel.join_table() == col.table) }); let alias = matches.next().filter(|_| matches.next().is_none()); @@ -104,6 +122,18 @@ impl SqlBuilder { self.sql.push_str(col.name); } + fn push_subquery(&mut self, subquery: &crate::query::Select<()>) { + let mut scope = self.outer_scope.clone(); + for table in self.base_table.iter().chain(&self.declared_tables) { + scope.tables.push(*table); + scope.qualifiers.push(table.qualifier().to_owned()); + } + scope + .qualifiers + .extend(self.relation_aliases.iter().map(|(_, alias)| alias.clone())); + self.push_compiled_sql(&subquery.compile_for_exists(scope)); + } + pub fn push_compiled_sql(&mut self, compiled: &CompiledSql) { let bytes = compiled.sql.as_bytes(); let mut idx = 0; @@ -393,7 +423,7 @@ impl ToSql for ExprNode { } ExprNode::Exists { subquery } => { builder.push_sql("EXISTS ("); - builder.push_compiled_sql(subquery); + builder.push_subquery(subquery); builder.push_sql(")"); } } diff --git a/crates/dbkit-core/src/expr.rs b/crates/dbkit-core/src/expr.rs index 0dfd146..a47c8dc 100644 --- a/crates/dbkit-core/src/expr.rs +++ b/crates/dbkit-core/src/expr.rs @@ -1,7 +1,6 @@ use std::marker::PhantomData; use std::ops::{Add, BitAnd, BitOr, BitXor, Div, Mul, Not, Shl, Shr, Sub}; -use crate::compile::CompiledSql; use crate::func::{StringBinaryExpr, StringUnaryExpr}; use crate::schema::{Column, ColumnRef}; use crate::types::{PgInterval, PgVector}; @@ -354,7 +353,7 @@ pub enum ExprNode { case_insensitive: bool, }, Exists { - subquery: CompiledSql, + subquery: Box>, }, } diff --git a/crates/dbkit-core/src/func.rs b/crates/dbkit-core/src/func.rs index ba3ff38..f3a1048 100644 --- a/crates/dbkit-core/src/func.rs +++ b/crates/dbkit-core/src/func.rs @@ -1,6 +1,5 @@ use bitflags::bitflags; -use crate::compile::CompiledSql; use crate::expr::{AggregateExpr, Expr, ExprNode, ExprOperand, IntoExpr, NumericExprType, TrimDirection, Value, VectorBinaryOp}; use crate::query::Select; use crate::PgVector; @@ -987,12 +986,11 @@ pub fn date_trunc(part: impl IntoExpr, value: impl IntoExpr) -> Ex }) } -fn exists_expr(subquery: CompiledSql) -> Expr { - Expr::new(ExprNode::Exists { subquery }) -} - pub fn exists(subquery: Select) -> Expr { - exists_expr(subquery.compile_for_exists()) + // Keep the query tree until compilation can see its enclosing SQL scopes. + Expr::new(ExprNode::Exists { + subquery: Box::new(subquery.into_subquery()), + }) } /// Marker trait for values that can participate in vector distance/similarity expressions. diff --git a/crates/dbkit-core/src/mutation.rs b/crates/dbkit-core/src/mutation.rs index 08921dc..6d19826 100644 --- a/crates/dbkit-core/src/mutation.rs +++ b/crates/dbkit-core/src/mutation.rs @@ -394,7 +394,7 @@ impl Update { } pub fn compile(&self) -> CompiledSql { - let mut builder = SqlBuilder::new(); + let mut builder = SqlBuilder::for_table(self.table); builder.push_sql("UPDATE "); builder.push_sql(&self.table.qualified_name()); builder.push_sql(" SET "); @@ -485,7 +485,7 @@ impl Delete { } pub fn compile(&self) -> CompiledSql { - let mut builder = SqlBuilder::new(); + let mut builder = SqlBuilder::for_table(self.table); builder.push_sql("DELETE FROM "); builder.push_sql(&self.table.qualified_name()); if !self.filters.is_empty() { diff --git a/crates/dbkit-core/src/path.rs b/crates/dbkit-core/src/path.rs index b4fac45..0c5134f 100644 --- a/crates/dbkit-core/src/path.rs +++ b/crates/dbkit-core/src/path.rs @@ -302,7 +302,7 @@ impl ExprNode { | Self::RowIn { expr, .. } | Self::IsNull { expr, .. } | Self::Like { expr, .. } => expr.visit_paths(visit), - // Subqueries have already compiled their own paths in their own scope. + // Subqueries discover their own paths when compiled in their enclosing scope. Self::Value(_) | Self::Exists { .. } => {} } } @@ -323,11 +323,12 @@ pub(crate) struct JoinPlan { } impl JoinPlan { - pub(crate) fn new(base: Table, declared: &[crate::Join], extra: &[crate::Join]) -> Self { + pub(crate) fn new(base: Table, declared: &[crate::Join], extra: &[crate::Join], outer_qualifiers: &[String]) -> Self { let mut plan = Self { joins: Vec::new(), reserved: std::iter::once(base.qualifier().to_owned()) .chain(declared.iter().chain(extra).map(|join| join.table.qualifier().to_owned())) + .chain(outer_qualifiers.iter().cloned()) .collect(), }; for (joins, explicit) in [(declared, true), (extra, false)] { diff --git a/crates/dbkit-core/src/query.rs b/crates/dbkit-core/src/query.rs index 0037885..73790f9 100644 --- a/crates/dbkit-core/src/query.rs +++ b/crates/dbkit-core/src/query.rs @@ -1,6 +1,6 @@ use std::marker::PhantomData; -use crate::compile::{CompiledSql, SqlBuilder, ToSql}; +use crate::compile::{CompiledSql, QueryScope, SqlBuilder, ToSql}; use crate::expr::{into_predicate, BooleanExprType, Expr, ExprNode, IntoExpr}; use crate::func; use crate::load::{ApplyLoad, LoadChain, NoLoad}; @@ -348,27 +348,55 @@ impl Select CompiledSql { - self.compile_inner(true, true, true) + pub(crate) fn into_subquery(self) -> Select<()> { + Select { + table: self.table, + columns: self.columns, + joins: self.joins, + filters: self.filters, + group_by: self.group_by, + having: self.having, + order_by: self.order_by, + limit: self.limit, + offset: self.offset, + distinct: self.distinct, + row_lock_wait: self.row_lock_wait, + loads: NoLoad, + _marker: PhantomData, + _lock_marker: PhantomData, + _distinct_marker: PhantomData, + _group_marker: PhantomData, + } + } + + pub(crate) fn compile_for_exists(&self, scope: QueryScope) -> CompiledSql { + self.compile_inner_with((&[], &[]), true, true, true, scope) } pub fn compile_with_extra(&self, extra_columns: &[SelectItem], extra_joins: &[Join]) -> CompiledSql { - self.compile_inner_with(extra_columns, extra_joins, true, true, true) + self.compile_inner_with((extra_columns, extra_joins), true, true, true, QueryScope::default()) } fn compile_inner(&self, include_order: bool, include_pagination: bool, include_locking: bool) -> CompiledSql { - self.compile_inner_with(&[], &[], include_order, include_pagination, include_locking) + self.compile_inner_with( + (&[], &[]), + include_order, + include_pagination, + include_locking, + QueryScope::default(), + ) } fn compile_inner_with( &self, - extra_columns: &[SelectItem], - extra_joins: &[Join], + extra: (&[SelectItem], &[Join]), include_order: bool, include_pagination: bool, include_locking: bool, + scope: QueryScope, ) -> CompiledSql { - let mut plan = crate::path::JoinPlan::new(self.table, &self.joins, extra_joins); + let (extra_columns, extra_joins) = extra; + let mut plan = crate::path::JoinPlan::new(self.table, &self.joins, extra_joins, &scope.qualifiers); for item in self.columns.iter().flatten().chain(extra_columns) { plan.discover(&item.expr); } @@ -385,7 +413,7 @@ impl Select Date: Mon, 7 Sep 2026 12:08:00 +0200 Subject: [PATCH 05/22] Test automatic join lock scoping --- .../dbkit/tests/integration_relation_paths.rs | 19 +++++++++++++++++++ crates/dbkit/tests/relation_paths_sql.rs | 14 ++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/crates/dbkit/tests/integration_relation_paths.rs b/crates/dbkit/tests/integration_relation_paths.rs index 66f8d9c..f1856f8 100644 --- a/crates/dbkit/tests/integration_relation_paths.rs +++ b/crates/dbkit/tests/integration_relation_paths.rs @@ -396,6 +396,25 @@ async fn related_ordering_runs_before_limit_and_keeps_missing_rows() -> Result<( Ok(()) } +#[tokio::test] +async fn automatic_left_join_for_update_returns_all_base_rows() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + // No owner filter: PostgreSQL must preserve the outer join and its missing-owner rows. + let rows: Vec = Record::query() + .order_by(Order::asc(Record::owner.id)) + .order_by(Order::asc(Record::id)) + .for_update() + .all(&tx) + .await?; + + assert_eq!(rows.iter().map(|row| row.id).collect::>(), [1, 3, 2, 4, 7, 5, 6]); + tx.rollback().await?; + Ok(()) +} + #[tokio::test] async fn count_exists_one_and_paginate_use_the_same_relation_filter() -> Result<(), Error> { let db = Database::connect(&db_url()).await?; diff --git a/crates/dbkit/tests/relation_paths_sql.rs b/crates/dbkit/tests/relation_paths_sql.rs index aef91a1..6f32402 100644 --- a/crates/dbkit/tests/relation_paths_sql.rs +++ b/crates/dbkit/tests/relation_paths_sql.rs @@ -114,6 +114,20 @@ fn order_only_paths_add_joins_before_pagination() { assert!(compiled.binds.is_empty()); } +#[test] +fn automatic_left_join_for_update_scopes_lock_to_base_table() { + // Ordering preserves the outer join; a filter could let PostgreSQL simplify it to an inner join. + let compiled = Record::query().order_by(Order::asc(Record::owner.id)).for_update().compile(); + + only_alias(&compiled.sql, "path_members"); + assert!(compiled.sql.contains("LEFT JOIN path_members")); + assert!( + compiled.sql.ends_with("FOR UPDATE OF path_records"), + "automatic left joins must scope the lock to the base table: {}", + compiled.sql + ); +} + #[test] fn projection_only_paths_add_joins_and_keep_output_aliases() { let compiled = Record::query() From 0d5fa9d4a62158a746fc4d2106988d2756a6eb5d Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Mon, 7 Sep 2026 13:22:48 +0200 Subject: [PATCH 06/22] Fix automatic join lock scoping --- crates/dbkit-core/src/path.rs | 10 ++++++++++ crates/dbkit-core/src/query.rs | 7 +------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/crates/dbkit-core/src/path.rs b/crates/dbkit-core/src/path.rs index 0c5134f..1622c84 100644 --- a/crates/dbkit-core/src/path.rs +++ b/crates/dbkit-core/src/path.rs @@ -406,6 +406,16 @@ impl JoinPlan { .collect() } + pub(crate) fn has_left_join(&self) -> bool { + self.joins.iter().any(|join| { + let kind = match join { + PlannedJoin::Declared(join) => join.kind, + PlannedJoin::Related { kind, .. } => *kind, + }; + matches!(kind, crate::JoinKind::Left) + }) + } + pub(crate) fn write(&self, builder: &mut crate::compile::SqlBuilder) { use crate::compile::ToSql; for join in &self.joins { diff --git a/crates/dbkit-core/src/query.rs b/crates/dbkit-core/src/query.rs index 73790f9..05098d3 100644 --- a/crates/dbkit-core/src/query.rs +++ b/crates/dbkit-core/src/query.rs @@ -519,12 +519,7 @@ impl Select Date: Mon, 7 Sep 2026 13:36:56 +0200 Subject: [PATCH 07/22] Test aliased self-relation joins --- .../dbkit/tests/integration_relation_paths.rs | 24 +++++++++++++++++++ crates/dbkit/tests/relation_paths_sql.rs | 18 ++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/crates/dbkit/tests/integration_relation_paths.rs b/crates/dbkit/tests/integration_relation_paths.rs index f1856f8..e359778 100644 --- a/crates/dbkit/tests/integration_relation_paths.rs +++ b/crates/dbkit/tests/integration_relation_paths.rs @@ -613,6 +613,30 @@ async fn self_relations_support_finite_depth_missing_parents_and_cycles() -> Res Ok(()) } +#[tokio::test] +async fn aliased_self_relation_filters_by_each_base_rows_parent() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let table = Node::TABLE.with_alias("n"); + let base_id = dbkit::Column::::new(table, "id"); + let mut results = Vec::new(); + for label in ["branch", "cycle"] { + let rows: Vec = dbkit::Select::new(table) + .filter(Node::parent.label.eq(label)) + .order_by(Order::asc(base_id)) + .all(&tx) + .await?; + results.push((label, rows.iter().map(|row| row.id).collect::>())); + } + + // A parent-to-itself join loses the leaf and makes the cycle match every base row. + assert_eq!(results, [("branch", vec![3]), ("cycle", vec![5])]); + tx.rollback().await?; + Ok(()) +} + #[tokio::test] async fn explicit_inner_joins_and_existing_table_column_filters_still_work() -> Result<(), Error> { let db = Database::connect(&db_url()).await?; diff --git a/crates/dbkit/tests/relation_paths_sql.rs b/crates/dbkit/tests/relation_paths_sql.rs index 6f32402..88f83c6 100644 --- a/crates/dbkit/tests/relation_paths_sql.rs +++ b/crates/dbkit/tests/relation_paths_sql.rs @@ -298,6 +298,24 @@ fn self_paths_keep_the_base_parent_and_grandparent_distinct() { ); } +#[test] +fn aliased_self_relation_join_keeps_the_source_key_on_the_base_row() { + let compiled = dbkit::Select::::new(Node::TABLE.with_alias("n")) + .filter(Node::parent.label.eq("branch")) + .compile(); + let parent = only_alias(&compiled.sql, "path_nodes"); + + assert_ne!(parent, "n"); + // The new relation rewrite must not turn the source key into the parent's own key. + assert!( + compiled.sql.contains(&format!("({parent}.id = n.parent_id)")), + "self-relation must join to the aliased base row: {}", + compiled.sql + ); + assert!(!compiled.sql.contains(&format!("({parent}.id = {parent}.parent_id)"))); + assert_eq!(compiled.binds, vec![Value::String("branch".into())]); +} + #[test] fn subquery_paths_stay_in_the_subquery_and_preserve_correlated_base_columns() { let compiled = Organization::query() From 8a8e8511f9e69fd84197ece9e262cf7916b24678 Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Mon, 7 Sep 2026 13:39:48 +0200 Subject: [PATCH 08/22] Fix aliased self-relation joins --- crates/dbkit-core/src/compile.rs | 8 ++++++++ crates/dbkit-core/src/path.rs | 6 +++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/dbkit-core/src/compile.rs b/crates/dbkit-core/src/compile.rs index e59b2b0..3c00b33 100644 --- a/crates/dbkit-core/src/compile.rs +++ b/crates/dbkit-core/src/compile.rs @@ -112,6 +112,14 @@ impl SqlBuilder { } pub fn push_related_column(&mut self, col: ColumnRef, path: &[crate::Relation]) { + if path.is_empty() { + let table = self + .base_table + .filter(|base| base.name == col.table.name && base.schema == col.table.schema) + .unwrap_or(col.table); + self.sql.push_str(&ColumnRef { table, ..col }.qualified_name()); + return; + } let (_, alias) = self .relation_aliases .iter() diff --git a/crates/dbkit-core/src/path.rs b/crates/dbkit-core/src/path.rs index 1622c84..0b05f89 100644 --- a/crates/dbkit-core/src/path.rs +++ b/crates/dbkit-core/src/path.rs @@ -260,7 +260,11 @@ pub fn join_on(path: &[Relation]) -> Expr { Expr::new(ExprNode::Binary { left: Box::new(column(target, path)), op: BinaryOp::Eq, - right: Box::new(column(source, previous)), + // An empty path still identifies the source row; it is not an unscoped model column. + right: Box::new(ExprNode::RelatedColumn { + column: source, + path: previous.to_vec(), + }), }) } From 9813cb200d0f87ffcc66582e1e59b88f4363fd2b Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Mon, 7 Sep 2026 19:09:21 +0200 Subject: [PATCH 09/22] Test correlations to outer relation joins --- .../dbkit/tests/integration_relation_paths.rs | 57 +++++++++++++++++++ crates/dbkit/tests/relation_paths_sql.rs | 57 +++++++++++++++++++ 2 files changed, 114 insertions(+) diff --git a/crates/dbkit/tests/integration_relation_paths.rs b/crates/dbkit/tests/integration_relation_paths.rs index e359778..c541fd5 100644 --- a/crates/dbkit/tests/integration_relation_paths.rs +++ b/crates/dbkit/tests/integration_relation_paths.rs @@ -265,6 +265,63 @@ async fn correlated_sibling_paths_keep_their_local_comparison_and_outer_identity Ok(()) } +#[tokio::test] +async fn correlated_exists_matches_the_outer_relation_join() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let records: Vec = Record::query() + .join(Record::owner) + .where_exists(Assignment::query().filter(Assignment::first_id.eq_col(Member::id))) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + // Owner 4 has no matching assignment; NULL and dangling owners cannot join. + assert_eq!(records.iter().map(|row| row.id).collect::>(), [1, 2, 3, 4]); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn correlated_not_exists_preserves_missing_outer_relation_joins() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let records: Vec = Record::query() + .left_join(Record::owner) + .where_not_exists(Assignment::query().filter(Assignment::first_id.eq_col(Member::id))) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + // A NULL owner, a dangling owner, and an owner without assignments all survive. + assert_eq!(records.iter().map(|row| row.id).collect::>(), [5, 6, 7]); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn correlated_exists_matches_the_outer_joined_loading_relation() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let records: Vec>> = Record::query() + .with(Record::owner.joined()) + .where_exists(Assignment::query().filter(Assignment::first_id.eq_col(Member::id))) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + assert_eq!(records.iter().map(|row| row.id).collect::>(), [1, 2, 3, 4]); + assert_eq!( + records.iter().map(|row| row.owner.as_ref().unwrap().id).collect::>(), + [1, 2, 1, 3] + ); + tx.rollback().await?; + Ok(()) +} + #[tokio::test] async fn filtering_is_independent_of_loading_strategy() -> Result<(), Error> { let db = Database::connect(&db_url()).await?; diff --git a/crates/dbkit/tests/relation_paths_sql.rs b/crates/dbkit/tests/relation_paths_sql.rs index 88f83c6..d354d2e 100644 --- a/crates/dbkit/tests/relation_paths_sql.rs +++ b/crates/dbkit/tests/relation_paths_sql.rs @@ -357,6 +357,43 @@ fn correlated_exists_preserves_outer_columns_with_automatic_and_declared_relatio } } +#[test] +fn correlated_exists_uses_the_outer_relation_join_alias() { + for outer in [Record::query().join(Record::owner), Record::query().left_join(Record::owner)] { + let compiled = outer + .where_exists(Assignment::query().filter(Assignment::first_id.eq_col(Member::id))) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + assert!( + compiled.sql.contains(&format!("(path_assignments.first_id = {owner}.id)")), + "{}", + compiled.sql + ); + } +} + +#[test] +fn correlated_nested_exists_uses_the_enclosing_relation_join_alias() { + let compiled = Record::query() + .join(Record::owner) + .where_exists( + Organization::query().where_exists( + Assignment::query() + .filter(Assignment::first_id.eq_col(Member::id)) + .filter(Member::organization_id.eq_col(Organization::id)), + ), + ) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + assert_eq!(compiled.sql.matches("EXISTS (").count(), 2); + for expected in [ + format!("(path_assignments.first_id = {owner}.id)"), + format!("({owner}.organization_id = path_organizations.id)"), + ] { + assert!(compiled.sql.contains(&expected), "missing {expected}: {}", compiled.sql); + } +} + #[test] fn correlated_not_exists_keeps_outer_filters_and_bind_order() { let compiled = Member::query() @@ -595,6 +632,26 @@ impl Executor for CaptureExecutor { } } +#[tokio::test] +async fn correlated_exists_uses_the_outer_joined_loading_alias() -> Result<(), Error> { + let ex = CaptureExecutor::default(); + let _: Vec>> = Record::query() + .with(Record::owner.joined()) + .where_exists(Assignment::query().filter(Assignment::first_id.eq_col(Member::id))) + .all(&ex) + .await?; + let sqls = ex.0.lock().unwrap(); + assert_eq!(sqls.len(), 1); + let owner = only_alias(&sqls[0], "path_members"); + assert!(sqls[0].contains(&format!("{owner}.label AS "))); + assert!( + sqls[0].contains(&format!("(path_assignments.first_id = {owner}.id)")), + "{}", + sqls[0] + ); + Ok(()) +} + #[tokio::test] async fn filtering_and_joined_loading_share_one_join_regardless_of_builder_order() -> Result<(), Error> { let ex = CaptureExecutor::default(); From b6853f52b7b35135e1d94f3b83173607399ce4e5 Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Mon, 7 Sep 2026 19:38:58 +0200 Subject: [PATCH 10/22] Fix correlations to outer relation joins --- crates/dbkit-core/src/compile.rs | 50 +++++++++++++++--------- crates/dbkit/tests/relation_paths_sql.rs | 44 +++++++++++++++++++++ 2 files changed, 75 insertions(+), 19 deletions(-) diff --git a/crates/dbkit-core/src/compile.rs b/crates/dbkit-core/src/compile.rs index 3c00b33..0f42a03 100644 --- a/crates/dbkit-core/src/compile.rs +++ b/crates/dbkit-core/src/compile.rs @@ -10,7 +10,7 @@ pub struct CompiledSql { /// Enclosing table bindings and SQL qualifiers visible to a subquery. #[derive(Debug, Default, Clone)] pub(crate) struct QueryScope { - tables: Vec, + bindings: Vec<(crate::Table, String)>, pub(crate) qualifiers: Vec, } @@ -93,21 +93,29 @@ impl SqlBuilder { self.push_related_column(col, &path.steps()); return; } - // An enclosing table binding takes precedence over the legacy shorthand - // for a local relation target. Explicit paths always identify local joins. - let mut matches = self.relation_aliases.iter().filter(|(path, _)| { - Some(col.table) != self.base_table - && !self.declared_tables.contains(&col.table) - && !self.outer_scope.tables.contains(&col.table) - && path.last().is_some_and(|rel| rel.join_table() == col.table) - }); - let alias = matches.next().filter(|_| matches.next().is_none()); - if let Some((_, alias)) = alias { - self.sql.push_str(alias); - self.sql.push('.'); - self.sql.push_str(col.name); + let qualifier = self.column_qualifier(col.table).to_owned(); + self.sql.push_str(&qualifier); + self.sql.push('.'); + self.sql.push_str(col.name); + } + + fn column_qualifier(&self, table: crate::Table) -> &str { + if Some(table) == self.base_table || self.declared_tables.contains(&table) { + return table.qualifier(); + } + // Enclosing bindings precede the legacy shorthand for a local relation + // target. Explicit paths always identify local joins. + if let Some((_, qualifier)) = self.outer_scope.bindings.iter().rev().find(|(bound, _)| *bound == table) { + return qualifier; + } + let mut matches = self + .relation_aliases + .iter() + .filter(|(path, _)| path.last().is_some_and(|rel| rel.join_table() == table)); + if let Some((_, alias)) = matches.next().filter(|_| matches.next().is_none()) { + alias } else { - self.sql.push_str(&col.qualified_name()); + table.qualifier() } } @@ -133,12 +141,16 @@ impl SqlBuilder { fn push_subquery(&mut self, subquery: &crate::query::Select<()>) { let mut scope = self.outer_scope.clone(); for table in self.base_table.iter().chain(&self.declared_tables) { - scope.tables.push(*table); + scope.bindings.push((*table, table.qualifier().to_owned())); scope.qualifiers.push(table.qualifier().to_owned()); } - scope - .qualifiers - .extend(self.relation_aliases.iter().map(|(_, alias)| alias.clone())); + for (path, alias) in &self.relation_aliases { + let table = path.last().expect("relation joins have a nonempty path").join_table(); + if !scope.bindings.iter().any(|(bound, _)| *bound == table) { + scope.bindings.push((table, self.column_qualifier(table).to_owned())); + } + scope.qualifiers.push(alias.clone()); + } self.push_compiled_sql(&subquery.compile_for_exists(scope)); } diff --git a/crates/dbkit/tests/relation_paths_sql.rs b/crates/dbkit/tests/relation_paths_sql.rs index d354d2e..17386e9 100644 --- a/crates/dbkit/tests/relation_paths_sql.rs +++ b/crates/dbkit/tests/relation_paths_sql.rs @@ -372,6 +372,50 @@ fn correlated_exists_uses_the_outer_relation_join_alias() { } } +#[test] +fn local_table_bindings_shadow_outer_relation_join_aliases() { + for compiled in [ + Record::query() + .join(Record::owner) + .where_exists(Member::query().filter(Member::label.eq("Atlas"))) + .compile(), + Record::query() + .join(Record::owner) + .where_exists( + Organization::query() + .join_on(Member::TABLE, Member::organization_id.eq_col(Organization::id)) + .where_exists(Assignment::query().filter(Assignment::first_id.eq_col(Member::id))) + .filter(Member::label.eq("Atlas")), + ) + .compile(), + ] { + assert!(compiled.sql.contains("(path_members.label = $1)"), "{}", compiled.sql); + if compiled.sql.contains("path_assignments") { + assert!( + compiled.sql.contains("(path_assignments.first_id = path_members.id)"), + "{}", + compiled.sql + ); + } + } +} + +#[test] +fn correlated_outer_join_alias_stays_distinct_from_an_inner_relation_path() { + let compiled = Record::query() + .join(Record::owner) + .where_exists(Assignment::query().filter(Assignment::first.id.eq(Member::id))) + .compile(); + let members = aliases(&compiled.sql, "path_members"); + assert_eq!(members.len(), 2); + assert_ne!(members[0], members[1]); + assert!( + compiled.sql.contains(&format!("({}.id = {}.id)", members[1], members[0])), + "{}", + compiled.sql + ); +} + #[test] fn correlated_nested_exists_uses_the_enclosing_relation_join_alias() { let compiled = Record::query() From b7a0f68cf4c2bae93398294923700bf371ca49f7 Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Mon, 7 Sep 2026 20:45:59 +0200 Subject: [PATCH 11/22] Test custom joins with dynamic relation columns --- .../dbkit/tests/integration_relation_paths.rs | 40 ++++++++++++++++ crates/dbkit/tests/relation_paths_sql.rs | 48 +++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/crates/dbkit/tests/integration_relation_paths.rs b/crates/dbkit/tests/integration_relation_paths.rs index c541fd5..ab6af72 100644 --- a/crates/dbkit/tests/integration_relation_paths.rs +++ b/crates/dbkit/tests/integration_relation_paths.rs @@ -736,6 +736,46 @@ async fn explicit_inner_joins_and_existing_table_column_filters_still_work() -> Ok(()) } +#[tokio::test] +async fn custom_left_join_with_a_dynamic_relation_column_preserves_matches_and_missing_owners() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let enabled: dbkit::Expr> = dbkit::Expr::new(dbkit::path::column(Member::enabled.as_ref(), &[Record::owner.descriptor()])); + let records: Vec = Record::query() + .left_join_on(Member::TABLE, enabled.eq(true)) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + // An enabled owner matches all four rows of the custom join. Disabled, + // NULL, and dangling owners survive once because this is a LEFT JOIN. + assert_eq!( + records.iter().map(|row| row.id).collect::>(), + [1, 1, 1, 1, 2, 3, 3, 3, 3, 4, 4, 4, 4, 5, 6, 7] + ); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn custom_inner_join_with_a_dynamic_relation_key_uses_the_requested_source_column() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let owner_id: dbkit::Expr> = dbkit::Expr::new(dbkit::path::column(Member::id.as_ref(), &[Record::owner.descriptor()])); + let records: Vec = Record::query() + .join_on(Member::TABLE, owner_id.eq_col(Record::id)) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + // Only records 1 and 2 have owner.id == record.id; each matches all four members. + assert_eq!(records.iter().map(|row| row.id).collect::>(), [1, 1, 1, 1, 2, 2, 2, 2]); + tx.rollback().await?; + Ok(()) +} + #[tokio::test] async fn grouped_projections_and_correlated_exists_resolve_paths_in_their_query() -> Result<(), Error> { let db = Database::connect(&db_url()).await?; diff --git a/crates/dbkit/tests/relation_paths_sql.rs b/crates/dbkit/tests/relation_paths_sql.rs index 17386e9..51e667d 100644 --- a/crates/dbkit/tests/relation_paths_sql.rs +++ b/crates/dbkit/tests/relation_paths_sql.rs @@ -197,6 +197,54 @@ fn custom_table_join_keeps_its_columns_when_a_relation_path_uses_the_same_table( assert_eq!(compiled.sql.matches("JOIN path_members ").count(), 2); } +#[test] +fn dynamic_relation_column_keeps_custom_join_predicates_and_binds() { + let enabled: dbkit::Expr> = dbkit::Expr::new(dbkit::path::column(Member::enabled.as_ref(), &[Record::owner.descriptor()])); + // Both public ways of expressing the path must preserve the custom ON clause. + for (actual, expected) in [ + ( + Record::query().join_on(Member::TABLE, enabled.clone().eq(true)).compile(), + Record::query().join_on(Member::TABLE, Record::owner.enabled.eq(true)).compile(), + ), + ( + Record::query().left_join_on(Member::TABLE, enabled.eq(true)).compile(), + Record::query() + .left_join_on(Member::TABLE, Record::owner.enabled.eq(true)) + .compile(), + ), + ] { + assert_eq!(actual.binds, vec![Value::Bool(true)], "{}", actual.sql); + assert_eq!(actual, expected); + assert_eq!(actual.sql.matches("JOIN path_members ").count(), 2); + } +} + +#[test] +fn dynamic_relation_key_compared_to_a_literal_is_a_custom_join() { + let owner_id: dbkit::Expr> = dbkit::Expr::new(dbkit::path::column(Member::id.as_ref(), &[Record::owner.descriptor()])); + let compiled = Record::query().join_on(Member::TABLE, owner_id.eq(1_i64)).compile(); + // Even the relation's target key is not a foreign-key join when the RHS is a value. + assert_eq!(compiled.binds, vec![Value::I64(1)], "{}", compiled.sql); + assert_eq!( + compiled, + Record::query().join_on(Member::TABLE, Record::owner.id.eq(1_i64)).compile() + ); + assert_eq!(compiled.sql.matches("JOIN path_members ").count(), 2); +} + +#[test] +fn dynamic_relation_key_compared_to_another_source_column_keeps_that_column() { + let owner_id: dbkit::Expr> = dbkit::Expr::new(dbkit::path::column(Member::id.as_ref(), &[Record::owner.descriptor()])); + let compiled = Record::query().join_on(Member::TABLE, owner_id.eq_col(Record::id)).compile(); + // The caller chose the record's ID, not its owner_id foreign key. + assert_eq!( + compiled, + Record::query().join_on(Member::TABLE, Record::owner.id.eq(Record::id)).compile() + ); + assert_eq!(compiled.sql.matches("JOIN path_members ").count(), 2); + assert!(compiled.binds.is_empty()); +} + #[test] fn nested_paths_share_their_prefix_and_join_in_dependency_order() { let compiled = Record::query() From afc63e20107f72092bbdfff5949801598d0ce0af Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Mon, 7 Sep 2026 20:48:39 +0200 Subject: [PATCH 12/22] Fix custom joins with dynamic relation columns --- crates/dbkit-core/src/path.rs | 43 +++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/crates/dbkit-core/src/path.rs b/crates/dbkit-core/src/path.rs index 0b05f89..00e006d 100644 --- a/crates/dbkit-core/src/path.rs +++ b/crates/dbkit-core/src/path.rs @@ -326,6 +326,36 @@ pub(crate) struct JoinPlan { reserved: Vec, } +fn relation_join_path(join: &crate::Join) -> Option<&[Relation]> { + let ExprNode::Binary { + left, + op: BinaryOp::Eq, + right, + } = &join.on.node + else { + return None; + }; + let ExprNode::RelatedColumn { column: target, path } = &**left else { + return None; + }; + let ExprNode::RelatedColumn { + column: source, + path: source_path, + } = &**right + else { + return None; + }; + let (relation, previous) = path.split_last()?; + let (target_key, source_key) = match relation.kind { + crate::RelationKind::BelongsTo => (relation.parent_key, relation.child_key), + crate::RelationKind::HasMany => (relation.child_key, relation.parent_key), + crate::RelationKind::ManyToMany => return None, + }; + // Only the complete predicate emitted by join_on can be replaced by a planned relation join. + (join.table == relation.join_table() && *target == target_key && *source == source_key && source_path == previous) + .then_some(path.as_slice()) +} + impl JoinPlan { pub(crate) fn new(base: Table, declared: &[crate::Join], extra: &[crate::Join], outer_qualifiers: &[String]) -> Self { let mut plan = Self { @@ -337,16 +367,9 @@ impl JoinPlan { }; for (joins, explicit) in [(declared, true), (extra, false)] { for join in joins { - if let ExprNode::Binary { - left, op: BinaryOp::Eq, .. - } = &join.on.node - { - if let ExprNode::RelatedColumn { path, column } = &**left { - if column.table == join.table { - plan.require(path, explicit.then_some(join.kind)); - continue; - } - } + if let Some(path) = relation_join_path(join) { + plan.require(path, explicit.then_some(join.kind)); + continue; } join.on.node.visit_paths(&mut |path| plan.require(path, None)); plan.joins.push(PlannedJoin::Declared(join.clone())); From 796dd120e7e584c16f65fa2be4ca74188bf336e5 Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Mon, 7 Sep 2026 23:07:50 +0200 Subject: [PATCH 13/22] Test nearer relation bindings in nested subqueries --- .../dbkit/tests/integration_relation_paths.rs | 47 +++++++++++++++++ crates/dbkit/tests/relation_paths_sql.rs | 52 +++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/crates/dbkit/tests/integration_relation_paths.rs b/crates/dbkit/tests/integration_relation_paths.rs index ab6af72..a86dd0f 100644 --- a/crates/dbkit/tests/integration_relation_paths.rs +++ b/crates/dbkit/tests/integration_relation_paths.rs @@ -265,6 +265,53 @@ async fn correlated_sibling_paths_keep_their_local_comparison_and_outer_identity Ok(()) } +#[tokio::test] +async fn nested_exists_matches_the_nearer_owner_instead_of_the_outer_member() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let members: Vec = Member::query() + .where_exists( + Record::query() + .join(Record::owner) + .filter(Record::id.eq(1_i64)) + .where_exists(Assignment::query().filter(Assignment::first_id.eq_col(Member::id))), + ) + .order_by(Order::asc(Member::id)) + .all(&tx) + .await?; + // Record 1's owner has assignments, independently of the outer member. + // Resolving the innermost ID to the outer member incorrectly excludes member 4. + assert_eq!(members.iter().map(|row| row.id).collect::>(), [1, 2, 3, 4]); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn nested_not_exists_keeps_a_missing_nearer_relation_instead_of_using_an_outer_relation() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let records: Vec = Record::query() + .join(Record::owner) + .where_exists( + Assignment::query() + .left_join(Assignment::first) + .filter(Assignment::id.eq(4_i64)) + .where_not_exists(Organization::query().filter(Organization::id.eq_col(Member::organization_id))), + ) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + // Assignment 4 has no first member, so NOT EXISTS succeeds for every joined + // record. Falling back to the record's owner incorrectly leaves only record 7. + assert_eq!(records.iter().map(|row| row.id).collect::>(), [1, 2, 3, 4, 7]); + tx.rollback().await?; + Ok(()) +} + #[tokio::test] async fn correlated_exists_matches_the_outer_relation_join() -> Result<(), Error> { let db = Database::connect(&db_url()).await?; diff --git a/crates/dbkit/tests/relation_paths_sql.rs b/crates/dbkit/tests/relation_paths_sql.rs index 51e667d..1c1b919 100644 --- a/crates/dbkit/tests/relation_paths_sql.rs +++ b/crates/dbkit/tests/relation_paths_sql.rs @@ -464,6 +464,58 @@ fn correlated_outer_join_alias_stays_distinct_from_an_inner_relation_path() { ); } +#[test] +fn nested_subquery_uses_the_nearer_relation_instead_of_the_outer_base_table() { + for middle in [ + Record::query().join(Record::owner), + Record::query().left_join(Record::owner), + Record::query().filter(Record::owner.enabled.eq(true)), + ] { + let compiled = Member::query() + .where_exists( + middle + .filter(Record::owner.id.eq(Member::id)) + .where_exists(Assignment::query().filter(Assignment::first_id.eq_col(Member::id))), + ) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + // The middle query still correlates to the outer member. Its child sees + // the middle query's owner as the nearest enclosing member binding. + assert!( + compiled.sql.contains(&format!("({owner}.id = path_members.id)")), + "{}", + compiled.sql + ); + assert!( + compiled.sql.contains(&format!("(path_assignments.first_id = {owner}.id)")), + "{}", + compiled.sql + ); + } +} + +#[test] +fn nested_subquery_uses_the_nearer_relation_instead_of_an_outer_relation() { + let compiled = Record::query() + .join(Record::owner) + .where_exists( + Assignment::query() + .left_join(Assignment::first) + .where_not_exists(Organization::query().filter(Organization::id.eq_col(Member::organization_id))), + ) + .compile(); + let members = aliases(&compiled.sql, "path_members"); + assert_eq!(members.len(), 2); + assert_ne!(members[0], members[1]); + assert!( + compiled + .sql + .contains(&format!("(path_organizations.id = {}.organization_id)", members[1])), + "{}", + compiled.sql + ); +} + #[test] fn correlated_nested_exists_uses_the_enclosing_relation_join_alias() { let compiled = Record::query() From 2c7d1f729d10f8d234da90e0c1c8f8f082272b3d Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Mon, 7 Sep 2026 23:18:24 +0200 Subject: [PATCH 14/22] Fix nearer relation bindings in nested subqueries --- crates/dbkit-core/src/compile.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/dbkit-core/src/compile.rs b/crates/dbkit-core/src/compile.rs index 0f42a03..6253c35 100644 --- a/crates/dbkit-core/src/compile.rs +++ b/crates/dbkit-core/src/compile.rs @@ -108,6 +108,10 @@ impl SqlBuilder { if let Some((_, qualifier)) = self.outer_scope.bindings.iter().rev().find(|(bound, _)| *bound == table) { return qualifier; } + self.relation_qualifier(table) + } + + fn relation_qualifier(&self, table: crate::Table) -> &str { let mut matches = self .relation_aliases .iter() @@ -140,14 +144,16 @@ impl SqlBuilder { fn push_subquery(&mut self, subquery: &crate::query::Select<()>) { let mut scope = self.outer_scope.clone(); + let local_start = scope.bindings.len(); for table in self.base_table.iter().chain(&self.declared_tables) { scope.bindings.push((*table, table.qualifier().to_owned())); scope.qualifiers.push(table.qualifier().to_owned()); } for (path, alias) in &self.relation_aliases { let table = path.last().expect("relation joins have a nonempty path").join_table(); - if !scope.bindings.iter().any(|(bound, _)| *bound == table) { - scope.bindings.push((table, self.column_qualifier(table).to_owned())); + // A child sees this query's bindings before those inherited from farther scopes. + if !scope.bindings[local_start..].iter().any(|(bound, _)| *bound == table) { + scope.bindings.push((table, self.relation_qualifier(table).to_owned())); } scope.qualifiers.push(alias.clone()); } From 421eb51372412a3834b54e86c918c3f353ec0958 Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Mon, 7 Sep 2026 23:39:19 +0200 Subject: [PATCH 15/22] Fix relation join ordering for custom joins --- crates/dbkit-core/src/compile.rs | 2 +- crates/dbkit-core/src/path.rs | 55 +++++++++++++--- .../dbkit/tests/integration_relation_paths.rs | 58 ++++++++++++++++ crates/dbkit/tests/relation_paths_sql.rs | 66 +++++++++++++++++++ 4 files changed, 171 insertions(+), 10 deletions(-) diff --git a/crates/dbkit-core/src/compile.rs b/crates/dbkit-core/src/compile.rs index 6253c35..512b4ba 100644 --- a/crates/dbkit-core/src/compile.rs +++ b/crates/dbkit-core/src/compile.rs @@ -99,7 +99,7 @@ impl SqlBuilder { self.sql.push_str(col.name); } - fn column_qualifier(&self, table: crate::Table) -> &str { + pub(crate) fn column_qualifier(&self, table: crate::Table) -> &str { if Some(table) == self.base_table || self.declared_tables.contains(&table) { return table.qualifier(); } diff --git a/crates/dbkit-core/src/path.rs b/crates/dbkit-core/src/path.rs index 00e006d..8c3d4cb 100644 --- a/crates/dbkit-core/src/path.rs +++ b/crates/dbkit-core/src/path.rs @@ -270,22 +270,28 @@ pub fn join_on(path: &[Relation]) -> Expr { impl ExprNode { pub(crate) fn visit_paths(&self, visit: &mut impl FnMut(&[Relation])) { + self.visit_columns(&mut |_, path| visit(path)); + } + + fn visit_columns(&self, visit: &mut impl FnMut(crate::ColumnRef, &[Relation])) { match self { Self::Column(col) => { if let Some(path) = col.path { - visit(&path.steps()); + visit(*col, &path.steps()); + } else { + visit(*col, &[]); } } - Self::RelatedColumn { path, .. } => visit(path), + Self::RelatedColumn { column, path } => visit(*column, path), Self::Row { values } | Self::Func { args: values, .. } => { for value in values { - value.visit_paths(visit); + value.visit_columns(visit); } } Self::Trim { expr, characters, .. } => { - expr.visit_paths(visit); + expr.visit_columns(visit); if let Some(chars) = characters { - chars.visit_paths(visit); + chars.visit_columns(visit); } } Self::AggregateFilter { @@ -295,8 +301,8 @@ impl ExprNode { | Self::VectorBinary { left, right, .. } | Self::Binary { left, right, .. } | Self::Bool { left, right, .. } => { - left.visit_paths(visit); - right.visit_paths(visit); + left.visit_columns(visit); + right.visit_columns(visit); } Self::Normalize { expr, .. } | Self::MakeInterval { value: expr, .. } @@ -305,7 +311,7 @@ impl ExprNode { | Self::In { expr, .. } | Self::RowIn { expr, .. } | Self::IsNull { expr, .. } - | Self::Like { expr, .. } => expr.visit_paths(visit), + | Self::Like { expr, .. } => expr.visit_columns(visit), // Subqueries discover their own paths when compiled in their enclosing scope. Self::Value(_) | Self::Exists { .. } => {} } @@ -321,6 +327,25 @@ enum PlannedJoin { }, } +impl PlannedJoin { + fn depends_on(&self, path: &[Relation], alias: &str, builder: &crate::compile::SqlBuilder) -> bool { + match self { + Self::Related { path: child, .. } => child.starts_with(path), + Self::Declared(join) => { + let mut depends = false; + join.on.node.visit_columns(&mut |column, column_path| { + depends |= if column_path.is_empty() { + builder.column_qualifier(column.table) == alias + } else { + column_path.starts_with(path) + }; + }); + depends + } + } + } +} + pub(crate) struct JoinPlan { joins: Vec, reserved: Vec, @@ -445,7 +470,19 @@ impl JoinPlan { pub(crate) fn write(&self, builder: &mut crate::compile::SqlBuilder) { use crate::compile::ToSql; - for join in &self.joins { + let mut joins: Vec<_> = self.joins.iter().collect(); + // Children are planned after parents. Moving them first lets each parent + // follow its earliest dependent without reordering custom joins. + for join in self.joins.iter().rev() { + if let PlannedJoin::Related { path, alias, .. } = join { + let index = joins.iter().position(|other| std::ptr::eq(*other, join)).unwrap(); + if let Some(before) = joins[..index].iter().position(|other| other.depends_on(path, alias, builder)) { + joins.remove(index); + joins.insert(before, join); + } + } + } + for join in joins { let (table, alias, kind, on) = match join { PlannedJoin::Declared(join) => (join.table, join.table.alias, join.kind, join.on.clone()), PlannedJoin::Related { path, alias, kind } => { diff --git a/crates/dbkit/tests/integration_relation_paths.rs b/crates/dbkit/tests/integration_relation_paths.rs index a86dd0f..424adf0 100644 --- a/crates/dbkit/tests/integration_relation_paths.rs +++ b/crates/dbkit/tests/integration_relation_paths.rs @@ -783,6 +783,64 @@ async fn explicit_inner_joins_and_existing_table_column_filters_still_work() -> Ok(()) } +#[tokio::test] +async fn custom_join_can_use_a_relation_discovered_in_a_filter_in_either_builder_order() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let on = Organization::id + .eq_col(Member::organization_id) + .and(Organization::label.eq("north")); + for query in [ + Record::query() + .join_on(Organization::TABLE, on.clone()) + .filter(Record::owner.enabled.eq(true)), + Record::query() + .filter(Record::owner.enabled.eq(true)) + .join_on(Organization::TABLE, on), + ] { + let records: Vec = query.order_by(Order::asc(Record::id)).all(&tx).await?; + // Keep both the custom organization condition and the related owner filter. + assert_eq!(records.iter().map(|row| row.id).collect::>(), [1, 3]); + } + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn custom_left_join_can_use_a_projection_relation_and_preserve_missing_targets() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let rows: Vec<(i64, Option, Option)> = Record::query() + .left_join_on(Organization::TABLE, Organization::id.eq_col(Member::organization_id)) + .select_only() + .column(Record::id) + .column(Record::owner.label) + .column(Organization::label) + .order_by(Order::asc(Record::id)) + .into_model() + .all(&tx) + .await?; + // Missing owners and an owner with a dangling organization must survive both LEFT JOINs. + assert_eq!( + rows, + [ + (1, Some("Atlas".into()), Some("north".into())), + (2, Some("Birch".into()), Some("south".into())), + (3, Some("Atlas".into()), Some("north".into())), + (4, Some("Cedar".into()), Some("south".into())), + (5, None, None), + (6, None, None), + (7, Some("Delta".into()), None), + ] + ); + tx.rollback().await?; + Ok(()) +} + #[tokio::test] async fn custom_left_join_with_a_dynamic_relation_column_preserves_matches_and_missing_owners() -> Result<(), Error> { let db = Database::connect(&db_url()).await?; diff --git a/crates/dbkit/tests/relation_paths_sql.rs b/crates/dbkit/tests/relation_paths_sql.rs index 1c1b919..cd03b2a 100644 --- a/crates/dbkit/tests/relation_paths_sql.rs +++ b/crates/dbkit/tests/relation_paths_sql.rs @@ -197,6 +197,72 @@ fn custom_table_join_keeps_its_columns_when_a_relation_path_uses_the_same_table( assert_eq!(compiled.sql.matches("JOIN path_members ").count(), 2); } +#[test] +fn filter_relation_join_precedes_a_custom_join_that_uses_its_alias() { + let on = Organization::id + .eq_col(Member::organization_id) + .and(Organization::label.eq("north")); + for query in [ + Record::query() + .join_on(Organization::TABLE, on.clone()) + .filter(Record::owner.enabled.eq(true)), + Record::query() + .filter(Record::owner.enabled.eq(true)) + .join_on(Organization::TABLE, on), + ] { + let compiled = query.compile(); + let owner = only_alias(&compiled.sql, "path_members"); + let owner_join = compiled.sql.find(&format!("LEFT JOIN path_members {owner} ON ")).unwrap(); + let organization_join = compiled.sql.find("JOIN path_organizations ON ").unwrap(); + // The shorthand in ON is rewritten to the owner alias, which must already be in scope. + assert!(owner_join < organization_join, "{}", compiled.sql); + assert!(compiled.sql.contains(&format!("(path_organizations.id = {owner}.organization_id)"))); + assert!(compiled.sql.contains("(path_organizations.label = $1)")); + assert_eq!(compiled.binds, vec![Value::String("north".into()), Value::Bool(true)]); + } +} + +#[test] +fn projection_relation_join_precedes_a_custom_left_join_that_uses_its_alias() { + let compiled = Record::query() + .left_join_on(Organization::TABLE, Organization::id.eq_col(Member::organization_id)) + .select_only() + .column(Record::id) + .column(Record::owner.label) + .column(Organization::label) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + let owner_join = compiled.sql.find(&format!("LEFT JOIN path_members {owner} ON ")).unwrap(); + let organization_join = compiled.sql.find("LEFT JOIN path_organizations ON ").unwrap(); + assert!(owner_join < organization_join, "{}", compiled.sql); + assert!(compiled.sql.contains(&format!("(path_organizations.id = {owner}.organization_id)"))); + assert!(compiled.binds.is_empty()); +} + +#[test] +fn nested_join_dependencies_move_together_without_reordering_custom_joins() { + let compiled = Record::query() + .join_on(Node::TABLE, Node::id.eq_col(Record::id)) + .join_on(Assignment::TABLE, Assignment::first_id.eq_col(Organization::id)) + .filter(Record::owner.organization.label.eq("north")) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + let organization = only_alias(&compiled.sql, "path_organizations"); + let unrelated_join = compiled.sql.find("JOIN path_nodes ON ").unwrap(); + let owner_join = compiled.sql.find(&format!("LEFT JOIN path_members {owner} ON ")).unwrap(); + let organization_join = compiled + .sql + .find(&format!("LEFT JOIN path_organizations {organization} ON ")) + .unwrap(); + let dependent_join = compiled.sql.find("JOIN path_assignments ON ").unwrap(); + assert!( + unrelated_join < owner_join && owner_join < organization_join && organization_join < dependent_join, + "{}", + compiled.sql + ); + assert!(compiled.sql.contains(&format!("(path_assignments.first_id = {organization}.id)"))); +} + #[test] fn dynamic_relation_column_keeps_custom_join_predicates_and_binds() { let enabled: dbkit::Expr> = dbkit::Expr::new(dbkit::path::column(Member::enabled.as_ref(), &[Record::owner.descriptor()])); From e412e07931948df332aa3d9e4fb001d9d2120256 Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Mon, 7 Sep 2026 23:57:59 +0200 Subject: [PATCH 16/22] Test scalar operators with relation columns --- crates/dbkit/tests/derive_ui.rs | 2 + .../dbkit/tests/integration_relation_paths.rs | 104 ++++++++++++++++++ crates/dbkit/tests/relation_paths_sql.rs | 53 +++++++++ .../tests/ui/fail_relation_path_operators.rs | 23 ++++ .../tests/ui/pass_relation_path_operators.rs | 89 +++++++++++++++ 5 files changed, 271 insertions(+) create mode 100644 crates/dbkit/tests/ui/fail_relation_path_operators.rs create mode 100644 crates/dbkit/tests/ui/pass_relation_path_operators.rs diff --git a/crates/dbkit/tests/derive_ui.rs b/crates/dbkit/tests/derive_ui.rs index 956e1a5..7975e46 100644 --- a/crates/dbkit/tests/derive_ui.rs +++ b/crates/dbkit/tests/derive_ui.rs @@ -70,6 +70,8 @@ fn main() -> ui_test::color_eyre::Result<()> { "pass_relation_state_into_generic_names.rs".into(), "pass_relation_state_into_shadowed_traits.rs".into(), "pass_relation_paths.rs".into(), + "pass_relation_path_operators.rs".into(), + "fail_relation_path_operators.rs".into(), "pass_relation_paths_graphs.rs".into(), "pass_relation_paths_field_names.rs".into(), "fail_relation_paths_value_types.rs".into(), diff --git a/crates/dbkit/tests/integration_relation_paths.rs b/crates/dbkit/tests/integration_relation_paths.rs index 424adf0..c1d61de 100644 --- a/crates/dbkit/tests/integration_relation_paths.rs +++ b/crates/dbkit/tests/integration_relation_paths.rs @@ -65,6 +65,110 @@ async fn setup_correlated(ex: &(impl Executor + Send + Sync)) -> Result<(), Erro Ok(()) } +#[tokio::test] +async fn scalar_left_arithmetic_evaluates_in_order_and_preserves_missing_relations() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let rows: Vec<(i64, Option, Option, Option, Option, Option)> = Record::query() + .select_only() + .column(Record::id) + .column(1_i32 + Record::owner.score) + .column(100_i32 - Record::owner.score) + .column(2_i32 * Record::owner.score) + .column(15_f64 / Record::owner.score) + .column(10_i64 - Record::owner.organization.id) + .order_by(Order::asc(Record::id)) + .into_model() + .all(&tx) + .await?; + assert_eq!( + rows, + [ + (1, Some(31), Some(70), Some(60), Some(0.5), Some(9)), + (2, Some(11), Some(90), Some(20), Some(1.5), Some(8)), + (3, Some(31), Some(70), Some(60), Some(0.5), Some(9)), + (4, Some(21), Some(80), Some(40), Some(0.75), Some(8)), + (5, None, None, None, None, None), + (6, None, None, None, None, None), + (7, Some(6), Some(95), Some(10), Some(3.0), None), + ] + ); + let filtered: Vec = Record::query() + .filter((100_i32 - Record::owner.score).gt(70_i32)) + .order_by(Order::asc(100_i32 - Record::owner.score)) + .all(&tx) + .await?; + assert_eq!(filtered.iter().map(|row| row.id).collect::>(), [4, 2, 7]); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn scalar_left_bitwise_and_shift_operators_preserve_values_and_nulls() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let rows: Vec<(i64, Option, Option, Option, Option, Option)> = Record::query() + .select_only() + .column(Record::id) + .column(7_i32 & Record::owner.score) + .column(8_i32 | Record::owner.score) + .column(3_i32 ^ Record::owner.score) + .column(1_i64 << Record::owner.score) + .column(1024_i64 >> Record::owner.score) + .order_by(Order::asc(Record::id)) + .into_model() + .all(&tx) + .await?; + assert_eq!( + rows, + [ + (1, Some(6), Some(30), Some(29), Some(1_073_741_824), Some(0)), + (2, Some(2), Some(10), Some(9), Some(1024), Some(1)), + (3, Some(6), Some(30), Some(29), Some(1_073_741_824), Some(0)), + (4, Some(4), Some(28), Some(23), Some(1_048_576), Some(0)), + (5, None, None, None, None, None), + (6, None, None, None, None, None), + (7, Some(5), Some(13), Some(6), Some(32), Some(32)), + ] + ); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn scalar_left_operators_preserve_null_fields_on_existing_self_relations() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let rows: Vec<(i64, Option, Option)> = Node::query() + .select_only() + .column(Node::id) + .column(1_i64 + Node::parent.parent_id) + .column(7_i64 & Node::parent.parent_id) + .order_by(Order::asc(Node::id)) + .into_model() + .all(&tx) + .await?; + // Node 2 has a parent whose parent_id is NULL; nodes 1 and 4 have no parent row. + assert_eq!( + rows, + [ + (1, None, None), + (2, None, None), + (3, Some(2), Some(1)), + (4, None, None), + (5, Some(6), Some(5)) + ] + ); + tx.rollback().await?; + Ok(()) +} + #[tokio::test] async fn correlated_exists_returns_only_members_with_matching_records() -> Result<(), Error> { let db = Database::connect(&db_url()).await?; diff --git a/crates/dbkit/tests/relation_paths_sql.rs b/crates/dbkit/tests/relation_paths_sql.rs index cd03b2a..322eb3d 100644 --- a/crates/dbkit/tests/relation_paths_sql.rs +++ b/crates/dbkit/tests/relation_paths_sql.rs @@ -55,6 +55,59 @@ fn filter_adds_a_left_join_without_selecting_or_loading_the_relation() { assert_eq!(compiled.binds, vec![Value::Bool(true)]); } +#[test] +fn scalar_left_arithmetic_keeps_operand_order_binds_and_nested_relation_paths() { + let compiled = Record::query() + .select_only() + .column(1_i32 + Record::owner.score) + .column(100_i32 - Record::owner.score) + .column(2_i32 * Record::owner.score) + .column(60_f64 / Record::owner.score) + .column(10_i64 - Record::owner.organization.id) + .filter((100_i32 - Record::owner.score).gt(70_i32)) + .order_by(Order::asc(100_i32 - Record::owner.score)) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + let organization = only_alias(&compiled.sql, "path_organizations"); + assert!(compiled.sql.starts_with(&format!( + "SELECT ($1 + {owner}.score), ($2 - {owner}.score), ($3 * {owner}.score), ($4 / {owner}.score), ($5 - {organization}.id) FROM " + ))); + assert_eq!( + compiled.binds, + vec![ + Value::I32(1), + Value::I32(100), + Value::I32(2), + Value::F64(60.0), + Value::I64(10), + Value::I32(70) + ] + ); + assert!(compiled + .sql + .ends_with(&format!("WHERE (($2 - {owner}.score) > $6) ORDER BY ($2 - {owner}.score) ASC"))); +} + +#[test] +fn scalar_left_bitwise_and_shift_operators_share_the_relation_join() { + let compiled = Record::query() + .select_only() + .column(7_i32 & Record::owner.score) + .column(8_i32 | Record::owner.score) + .column(3_i32 ^ Record::owner.score) + .column(1_i64 << Record::owner.score) + .column(1024_i64 >> Record::owner.score) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + assert!(compiled.sql.starts_with(&format!( + "SELECT ($1 & {owner}.score), ($2 | {owner}.score), ($3 # {owner}.score), ($4 << {owner}.score), ($5 >> {owner}.score) FROM " + ))); + assert_eq!( + compiled.binds, + vec![Value::I32(7), Value::I32(8), Value::I32(3), Value::I64(1), Value::I64(1024)] + ); +} + #[test] fn repeated_columns_and_computed_expressions_reuse_the_path() { let compiled = Record::query() diff --git a/crates/dbkit/tests/ui/fail_relation_path_operators.rs b/crates/dbkit/tests/ui/fail_relation_path_operators.rs new file mode 100644 index 0000000..de2ec11 --- /dev/null +++ b/crates/dbkit/tests/ui/fail_relation_path_operators.rs @@ -0,0 +1,23 @@ +#[path = "../support/relation_paths.rs"] +mod relation_paths; +use relation_paths::Record; + +fn main() { + // Forwarding an operator must retain the ordinary column's operand restrictions. + let _ = 1_i32 + Record::owner.label; //~ E0277 + let _ = 1_i32 - Record::owner.enabled; //~ E0277 + let _ = 1_u32 + Record::owner.score; //~ E0277 + let _ = 1_i16 + Record::owner.score; //~ E0277 + let _ = 1_f64 * Record::owner.score; //~ E0277 + let _ = 1_f64 & Record::owner.score; //~ E0277 + let _ = true | Record::owner.enabled; //~ E0277 + let _ = 1_i32 ^ Record::owner.note; //~ E0277 + + // PostgreSQL shift counts accept SMALLINT/INTEGER, not BIGINT or text. + let _ = 1_i32 << Record::owner.id; //~ E0277 + let _ = 1_i64 >> Record::owner.organization.id; //~ E0277 + let _ = 1_i32 << Record::owner.label; //~ E0277 + + // A scalar on the left cannot remove the path's outer-join nullability. + let _: dbkit::Expr = 1_i32 + Record::owner.score; //~ E0308 +} diff --git a/crates/dbkit/tests/ui/pass_relation_path_operators.rs b/crates/dbkit/tests/ui/pass_relation_path_operators.rs new file mode 100644 index 0000000..c2eebea --- /dev/null +++ b/crates/dbkit/tests/ui/pass_relation_path_operators.rs @@ -0,0 +1,89 @@ +//@check-pass +#![allow(non_upper_case_globals)] +use dbkit::{model, Expr, IntoExpr}; + +#[model(table = "numeric_samples")] +struct Sample { + #[key] + id: i64, + small: i16, + integer: i32, + real: f32, + double: f64, + nullable: Option, + parent_id: Option, + #[belongs_to(key = parent_id, references = id)] + parent: dbkit::BelongsTo, +} + +fn nullable(_: Expr>) {} + +macro_rules! arithmetic { + ($literal:expr, $column:expr, $output:ty) => { + arithmetic!($literal, $column, $output, $output); + }; + ($literal:expr, $column:expr, $output:ty, $quotient:ty) => { + nullable::<$output>($literal + $column); + nullable::<$output>($literal - $column); + nullable::<$output>($literal * $column); + nullable::<$quotient>($literal / $column); + }; +} + +macro_rules! bitwise { + ($literal:expr, $column:expr, $output:ty) => { + nullable::<$output>($literal & $column); + nullable::<$output>($literal | $column); + nullable::<$output>($literal ^ $column); + }; +} + +macro_rules! shifts { + ($literal:expr, $output:ty) => { + nullable::<$output>($literal << Sample::parent.small); + nullable::<$output>($literal >> Sample::parent.small); + nullable::<$output>($literal << Sample::parent.integer); + nullable::<$output>($literal >> Sample::parent.integer); + }; +} + +fn main() { + // Every built-in scalar type supported with ordinary columns also accepts paths. + // SMALLINT +, -, and * widen to INTEGER; SMALLINT / SMALLINT stays SMALLINT. + arithmetic!(12_i16, Sample::parent.small, i32, i16); + arithmetic!(12_i32, Sample::parent.integer, i32); + arithmetic!(12_i64, Sample::parent.id, i64); + arithmetic!(12_f32, Sample::parent.real, f32); + arithmetic!(12_f64, Sample::parent.double, f64); + // Division supports mixed numeric types; +, -, and * retain their existing restrictions. + nullable::(12_i16 / Sample::parent.integer); + nullable::(12_i32 / Sample::parent.id); + nullable::(12_f32 / Sample::parent.integer); + nullable::(12_i32 / Sample::parent.real); + + bitwise!(7_i16, Sample::parent.small, i16); + bitwise!(7_i32, Sample::parent.integer, i32); + bitwise!(7_i64, Sample::parent.id, i64); + bitwise!(7_i16, Sample::parent.integer, i32); + bitwise!(7_i32, Sample::parent.id, i64); + bitwise!(7_i64, Sample::parent.small, i64); + shifts!(1_i16, i16); + shifts!(1_i32, i32); + shifts!(1_i64, i64); + + // Existing nullable fields stay singly nullable through nested paths and operators. + let column = Sample::parent.parent.nullable; + arithmetic!(12_i32, column, i32); + bitwise!(7_i32, column, i32); + nullable::(1_i64 << column); + nullable::(64_i64 >> column); + + // Scalar, column, expression, and relation operands remain composable. + nullable::((1_i32 + column) * (Sample::parent.integer + 2_i32)); + nullable::(Sample::integer + column); + nullable::(Sample::integer.into_expr() - column); + nullable::(Sample::parent.integer / column); + nullable::(Sample::integer & column); + nullable::(Sample::integer.into_expr() | column); + nullable::(Sample::parent.integer ^ column); +} From d54db77fec3d3172dc4a062bf739c016304a4c90 Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Tue, 8 Sep 2026 00:01:33 +0200 Subject: [PATCH 17/22] Support scalar operators with relation columns --- crates/dbkit-core/src/path.rs | 27 ++++++++++++------- .../tests/ui/fail_relation_path_operators.rs | 2 +- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/crates/dbkit-core/src/path.rs b/crates/dbkit-core/src/path.rs index 8c3d4cb..5693aac 100644 --- a/crates/dbkit-core/src/path.rs +++ b/crates/dbkit-core/src/path.rs @@ -210,24 +210,31 @@ impl RelatedColumn { } macro_rules! forward_operator { - ($($trait:ident::$method:ident),* $(,)?) => {$( + ($($trait:ident::$method:ident => [$($scalar:ty),+]),* $(,)?) => {$( impl $trait for RelatedColumn where Column: $trait { type Output = as $trait>::Output; fn $method(self, rhs: R) -> Self::Output { self.0.$method(rhs) } } + $( + impl $trait> for $scalar + where $scalar: $trait> { + type Output = <$scalar as $trait>>::Output; + fn $method(self, rhs: RelatedColumn) -> Self::Output { self.$method(rhs.0) } + } + )+ )*}; } forward_operator!( - Add::add, - Sub::sub, - Mul::mul, - Div::div, - BitAnd::bitand, - BitOr::bitor, - BitXor::bitxor, - Shl::shl, - Shr::shr + Add::add => [i16, i32, i64, f32, f64], + Sub::sub => [i16, i32, i64, f32, f64], + Mul::mul => [i16, i32, i64, f32, f64], + Div::div => [i16, i32, i64, f32, f64], + BitAnd::bitand => [i16, i32, i64], + BitOr::bitor => [i16, i32, i64], + BitXor::bitxor => [i16, i32, i64], + Shl::shl => [i16, i32, i64], + Shr::shr => [i16, i32, i64] ); impl Not for RelatedColumn where diff --git a/crates/dbkit/tests/ui/fail_relation_path_operators.rs b/crates/dbkit/tests/ui/fail_relation_path_operators.rs index de2ec11..5e6503a 100644 --- a/crates/dbkit/tests/ui/fail_relation_path_operators.rs +++ b/crates/dbkit/tests/ui/fail_relation_path_operators.rs @@ -9,7 +9,7 @@ fn main() { let _ = 1_u32 + Record::owner.score; //~ E0277 let _ = 1_i16 + Record::owner.score; //~ E0277 let _ = 1_f64 * Record::owner.score; //~ E0277 - let _ = 1_f64 & Record::owner.score; //~ E0277 + let _ = 1_f64 & Record::owner.score; //~ E0369 let _ = true | Record::owner.enabled; //~ E0277 let _ = 1_i32 ^ Record::owner.note; //~ E0277 From 7caf7fc24d8315ed3fcf05519f100206cb8cdc8d Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Tue, 8 Sep 2026 13:49:47 +0200 Subject: [PATCH 18/22] Test ambiguous relation scope bindings --- .../dbkit/tests/integration_relation_paths.rs | 54 +++++++++++++++++++ crates/dbkit/tests/relation_paths_sql.rs | 45 ++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/crates/dbkit/tests/integration_relation_paths.rs b/crates/dbkit/tests/integration_relation_paths.rs index c1d61de..7af8241 100644 --- a/crates/dbkit/tests/integration_relation_paths.rs +++ b/crates/dbkit/tests/integration_relation_paths.rs @@ -416,6 +416,60 @@ async fn nested_not_exists_keeps_a_missing_nearer_relation_instead_of_using_an_o Ok(()) } +#[tokio::test] +async fn ambiguous_middle_relations_correlate_to_the_outer_owner() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let records: Vec = Record::query() + .join(Record::owner) + .where_exists( + Assignment::query() + .join(Assignment::first) + .join(Assignment::second) + .filter(Assignment::id.eq(1_i64)) + .where_exists( + Organization::query() + .filter(Organization::id.eq_col(Member::organization_id)) + .filter(Organization::label.eq("north")), + ), + ) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + // Assignment 1's siblings belong to different organizations. Choosing its first + // member would return every joined record; choosing its second would return none. + assert_eq!(records.iter().map(|row| row.id).collect::>(), [1, 3]); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn ambiguous_middle_relations_preserve_nulls_in_the_outer_binding() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let records: Vec = Record::query() + .left_join(Record::owner) + .where_exists( + Assignment::query() + .left_join(Assignment::first) + .left_join(Assignment::second) + .filter(Assignment::id.eq(4_i64)) + .where_not_exists(Organization::query().filter(Organization::id.eq_col(Member::organization_id))), + ) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + // A missing sibling row does not make the other sibling a unique binding. + // Only records with no owner or no owner organization satisfy NOT EXISTS. + assert_eq!(records.iter().map(|row| row.id).collect::>(), [5, 6, 7]); + tx.rollback().await?; + Ok(()) +} + #[tokio::test] async fn correlated_exists_matches_the_outer_relation_join() -> Result<(), Error> { let db = Database::connect(&db_url()).await?; diff --git a/crates/dbkit/tests/relation_paths_sql.rs b/crates/dbkit/tests/relation_paths_sql.rs index 322eb3d..4a752fc 100644 --- a/crates/dbkit/tests/relation_paths_sql.rs +++ b/crates/dbkit/tests/relation_paths_sql.rs @@ -635,6 +635,51 @@ fn nested_subquery_uses_the_nearer_relation_instead_of_an_outer_relation() { ); } +#[test] +fn ambiguous_middle_relations_preserve_the_outer_relation_binding() { + for middle in [ + Assignment::query().join(Assignment::first).join(Assignment::second), + Assignment::query().join(Assignment::second).join(Assignment::first), + Assignment::query().filter(Assignment::first.score.gt(Assignment::second.score)), + ] { + let compiled = Record::query() + .join(Record::owner) + .where_exists(middle.where_exists(Organization::query().filter(Organization::id.eq_col(Member::organization_id)))) + .compile(); + let members = aliases(&compiled.sql, "path_members"); + assert_eq!(members.len(), 3); + // Neither sibling identifies a unique Member binding. Keep the outer owner, + // regardless of sibling declaration order or automatic join discovery. + assert!( + compiled + .sql + .contains(&format!("(path_organizations.id = {}.organization_id)", members[0])), + "{}", + compiled.sql + ); + } +} + +#[test] +fn ambiguous_nested_relation_targets_preserve_the_outer_nested_binding() { + let compiled = Record::query() + .filter(Record::owner.organization.label.eq("north")) + .where_exists( + Assignment::query() + .filter(Assignment::first.organization.id.eq(Assignment::second.organization.id)) + .where_exists(Node::query().filter(Node::id.eq_col(Organization::id))), + ) + .compile(); + let organizations = aliases(&compiled.sql, "path_organizations"); + assert_eq!(organizations.len(), 3); + assert!( + compiled.sql.contains(&format!("(path_nodes.id = {}.id)", organizations[0])), + "{}", + compiled.sql + ); + assert_eq!(compiled.binds, vec![Value::String("north".into())]); +} + #[test] fn correlated_nested_exists_uses_the_enclosing_relation_join_alias() { let compiled = Record::query() From 18e88f54ec6a43677cb316e8f8745644ada8f0ca Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Tue, 8 Sep 2026 14:46:52 +0200 Subject: [PATCH 19/22] Fix ambiguous relation scope bindings --- crates/dbkit-core/src/compile.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/crates/dbkit-core/src/compile.rs b/crates/dbkit-core/src/compile.rs index 512b4ba..c3e5d7e 100644 --- a/crates/dbkit-core/src/compile.rs +++ b/crates/dbkit-core/src/compile.rs @@ -108,19 +108,15 @@ impl SqlBuilder { if let Some((_, qualifier)) = self.outer_scope.bindings.iter().rev().find(|(bound, _)| *bound == table) { return qualifier; } - self.relation_qualifier(table) + self.relation_alias(table).unwrap_or(table.qualifier()) } - fn relation_qualifier(&self, table: crate::Table) -> &str { + fn relation_alias(&self, table: crate::Table) -> Option<&str> { let mut matches = self .relation_aliases .iter() .filter(|(path, _)| path.last().is_some_and(|rel| rel.join_table() == table)); - if let Some((_, alias)) = matches.next().filter(|_| matches.next().is_none()) { - alias - } else { - table.qualifier() - } + matches.next().filter(|_| matches.next().is_none()).map(|(_, alias)| alias.as_str()) } pub fn push_related_column(&mut self, col: ColumnRef, path: &[crate::Relation]) { @@ -151,9 +147,11 @@ impl SqlBuilder { } for (path, alias) in &self.relation_aliases { let table = path.last().expect("relation joins have a nonempty path").join_table(); - // A child sees this query's bindings before those inherited from farther scopes. + // Only unique relation targets shadow farther bindings in a child scope. if !scope.bindings[local_start..].iter().any(|(bound, _)| *bound == table) { - scope.bindings.push((table, self.relation_qualifier(table).to_owned())); + if let Some(qualifier) = self.relation_alias(table) { + scope.bindings.push((table, qualifier.to_owned())); + } } scope.qualifiers.push(alias.clone()); } From 66aa68a2c5934f81b3b813c3631543631ba5d58d Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Tue, 8 Sep 2026 14:51:34 +0200 Subject: [PATCH 20/22] Test correlations in join-condition subqueries --- .../dbkit/tests/integration_relation_paths.rs | 65 ++++++++++++++++++ crates/dbkit/tests/relation_paths_sql.rs | 66 +++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/crates/dbkit/tests/integration_relation_paths.rs b/crates/dbkit/tests/integration_relation_paths.rs index 7af8241..966302a 100644 --- a/crates/dbkit/tests/integration_relation_paths.rs +++ b/crates/dbkit/tests/integration_relation_paths.rs @@ -966,6 +966,71 @@ async fn custom_join_can_use_a_relation_discovered_in_a_filter_in_either_builder Ok(()) } +#[tokio::test] +async fn custom_join_exists_can_correlate_to_an_automatically_joined_owner() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let records: Vec = Record::query() + .join_on( + Organization::TABLE, + Organization::id.eq(1_i64).and(func::exists( + Assignment::query() + .filter(Assignment::first_id.eq_col(Member::id)) + .filter(Assignment::second_code.eq("b")), + )), + ) + .filter(Record::owner.enabled.eq(true)) + .order_by(Order::asc(Record::id)) + .all(&tx) + .await?; + // Owners 1 and 3 are enabled and have matching assignments. Restricting the + // joined organization to one row also checks that EXISTS does not duplicate records. + assert_eq!(records.iter().map(|row| row.id).collect::>(), [1, 3, 4]); + tx.rollback().await?; + Ok(()) +} + +#[tokio::test] +async fn custom_left_join_not_exists_preserves_missing_owner_rows() -> Result<(), Error> { + let db = Database::connect(&db_url()).await?; + let tx = db.begin().await?; + setup(&tx).await?; + + let rows: Vec<(i64, Option, Option)> = Record::query() + .left_join_on( + Organization::TABLE, + Organization::id + .eq(1_i64) + .and(func::exists(Assignment::query().filter(Assignment::first_id.eq_col(Member::id))).not()), + ) + .select_only() + .column(Record::id) + .column(Record::owner.id) + .column(Organization::id) + .order_by(Order::asc(Record::id)) + .into_model() + .all(&tx) + .await?; + // The custom join matches when the owner is absent or has no assignments. + // Owners with assignments still survive, with a NULL joined organization. + assert_eq!( + rows, + [ + (1, Some(1), None), + (2, Some(2), None), + (3, Some(1), None), + (4, Some(3), None), + (5, None, Some(1)), + (6, None, Some(1)), + (7, Some(4), Some(1)), + ] + ); + tx.rollback().await?; + Ok(()) +} + #[tokio::test] async fn custom_left_join_can_use_a_projection_relation_and_preserve_missing_targets() -> Result<(), Error> { let db = Database::connect(&db_url()).await?; diff --git a/crates/dbkit/tests/relation_paths_sql.rs b/crates/dbkit/tests/relation_paths_sql.rs index 4a752fc..0838afa 100644 --- a/crates/dbkit/tests/relation_paths_sql.rs +++ b/crates/dbkit/tests/relation_paths_sql.rs @@ -292,6 +292,72 @@ fn projection_relation_join_precedes_a_custom_left_join_that_uses_its_alias() { assert!(compiled.binds.is_empty()); } +#[test] +fn relation_join_precedes_a_correlated_exists_in_a_custom_join_condition() { + let on = Organization::id.eq(1_i64).and(func::exists( + Assignment::query() + .filter(Assignment::first_id.eq_col(Member::id)) + .filter(Assignment::second_code.eq("b")), + )); + for query in [ + Record::query() + .join_on(Organization::TABLE, on.clone()) + .filter(Record::owner.enabled.eq(true)), + Record::query() + .filter(Record::owner.enabled.eq(true)) + .join_on(Organization::TABLE, on), + ] { + let compiled = query.compile(); + let owner = only_alias(&compiled.sql, "path_members"); + let owner_join = compiled.sql.find(&format!("LEFT JOIN path_members {owner} ON ")).unwrap(); + let custom_join = compiled.sql.find("JOIN path_organizations ON ").unwrap(); + assert!(owner_join < custom_join, "{}", compiled.sql); + assert!(compiled.sql.contains(&format!("(path_assignments.first_id = {owner}.id)"))); + assert_eq!(compiled.binds, vec![Value::I64(1), Value::String("b".into()), Value::Bool(true)]); + } +} + +#[test] +fn relation_join_precedes_a_correlation_through_nested_join_subqueries() { + let compiled = Record::query() + .left_join_on( + Organization::TABLE, + func::exists(Assignment::query().where_exists(Node::query().filter(Node::id.eq_col(Member::id)))), + ) + .select_only() + .column(Record::id) + .column(Record::owner.score) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + let owner_join = compiled.sql.find(&format!("LEFT JOIN path_members {owner} ON ")).unwrap(); + let custom_join = compiled.sql.find("LEFT JOIN path_organizations ON ").unwrap(); + assert!(owner_join < custom_join, "{}", compiled.sql); + assert!(compiled.sql.contains(&format!("(path_nodes.id = {owner}.id)"))); + assert_eq!(compiled.sql.matches("EXISTS (").count(), 2); +} + +#[test] +fn uncorrelated_join_subquery_keeps_its_relation_paths_local() { + let compiled = Record::query() + .join_on( + Organization::TABLE, + func::exists(Member::query().filter(Member::organization.label.eq("north"))), + ) + .select_only() + .column(Record::id) + .column(Record::owner.score) + .compile(); + let owner = only_alias(&compiled.sql, "path_members"); + let owner_join = compiled.sql.find(&format!("LEFT JOIN path_members {owner} ON ")).unwrap(); + let custom_join = compiled.sql.find("JOIN path_organizations ON ").unwrap(); + // Member belongs to the subquery here; its organization path must stay there. + assert!(custom_join < owner_join, "{}", compiled.sql); + assert!(compiled + .sql + .contains("EXISTS (SELECT path_members.* FROM path_members LEFT JOIN path_organizations ")); + assert_eq!(compiled.sql.matches("LEFT JOIN path_organizations ").count(), 1); +} + #[test] fn nested_join_dependencies_move_together_without_reordering_custom_joins() { let compiled = Record::query() From 301377ca66acc1af64345424e6846adb4d66b492 Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Tue, 8 Sep 2026 14:58:55 +0200 Subject: [PATCH 21/22] Fix correlations in join-condition subqueries --- crates/dbkit-core/src/compile.rs | 46 ++++++++++++++++++++- crates/dbkit-core/src/path.rs | 69 ++++++++++++++------------------ crates/dbkit-core/src/query.rs | 8 ++-- 3 files changed, 79 insertions(+), 44 deletions(-) diff --git a/crates/dbkit-core/src/compile.rs b/crates/dbkit-core/src/compile.rs index c3e5d7e..aa40c12 100644 --- a/crates/dbkit-core/src/compile.rs +++ b/crates/dbkit-core/src/compile.rs @@ -22,6 +22,7 @@ pub struct SqlBuilder { relation_aliases: Vec<(Vec, String)>, declared_tables: Vec, outer_scope: QueryScope, + referenced_qualifiers: Vec, } impl SqlBuilder { @@ -97,9 +98,10 @@ impl SqlBuilder { self.sql.push_str(&qualifier); self.sql.push('.'); self.sql.push_str(col.name); + self.referenced_qualifiers.push(qualifier); } - pub(crate) fn column_qualifier(&self, table: crate::Table) -> &str { + fn column_qualifier(&self, table: crate::Table) -> &str { if Some(table) == self.base_table || self.declared_tables.contains(&table) { return table.qualifier(); } @@ -126,6 +128,7 @@ impl SqlBuilder { .filter(|base| base.name == col.table.name && base.schema == col.table.schema) .unwrap_or(col.table); self.sql.push_str(&ColumnRef { table, ..col }.qualified_name()); + self.referenced_qualifiers.push(table.qualifier().to_owned()); return; } let (_, alias) = self @@ -136,6 +139,28 @@ impl SqlBuilder { self.sql.push_str(alias); self.sql.push('.'); self.sql.push_str(col.name); + self.referenced_qualifiers.push(alias.clone()); + } + + pub(crate) fn compile_expression(&self, expr: &ExprNode) -> Self { + let mut builder = Self { + base_table: self.base_table, + relation_aliases: self.relation_aliases.clone(), + declared_tables: self.declared_tables.clone(), + outer_scope: self.outer_scope.clone(), + ..Self::default() + }; + expr.to_sql(&mut builder); + builder + } + + pub(crate) fn references_qualifier(&self, qualifier: &str) -> bool { + self.referenced_qualifiers.iter().any(|reference| reference == qualifier) + } + + pub(crate) fn push_expression(&mut self, mut expression: Self) { + self.referenced_qualifiers.append(&mut expression.referenced_qualifiers); + self.push_compiled_sql(&expression.finish()); } fn push_subquery(&mut self, subquery: &crate::query::Select<()>) { @@ -155,7 +180,24 @@ impl SqlBuilder { } scope.qualifiers.push(alias.clone()); } - self.push_compiled_sql(&subquery.compile_for_exists(scope)); + let subquery = subquery.compile_for_exists(scope); + // Only correlations escape a subquery. Its own tables and aliases may + // shadow enclosing names and cannot create dependencies in the parent. + self.referenced_qualifiers.extend( + subquery + .referenced_qualifiers + .iter() + .filter(|qualifier| { + !subquery + .base_table + .iter() + .chain(&subquery.declared_tables) + .any(|table| table.qualifier() == *qualifier) + && !subquery.relation_aliases.iter().any(|(_, alias)| alias == *qualifier) + }) + .cloned(), + ); + self.push_compiled_sql(&subquery.finish()); } pub fn push_compiled_sql(&mut self, compiled: &CompiledSql) { diff --git a/crates/dbkit-core/src/path.rs b/crates/dbkit-core/src/path.rs index 5693aac..e6e88c2 100644 --- a/crates/dbkit-core/src/path.rs +++ b/crates/dbkit-core/src/path.rs @@ -277,28 +277,22 @@ pub fn join_on(path: &[Relation]) -> Expr { impl ExprNode { pub(crate) fn visit_paths(&self, visit: &mut impl FnMut(&[Relation])) { - self.visit_columns(&mut |_, path| visit(path)); - } - - fn visit_columns(&self, visit: &mut impl FnMut(crate::ColumnRef, &[Relation])) { match self { Self::Column(col) => { if let Some(path) = col.path { - visit(*col, &path.steps()); - } else { - visit(*col, &[]); + visit(&path.steps()); } } - Self::RelatedColumn { column, path } => visit(*column, path), + Self::RelatedColumn { path, .. } => visit(path), Self::Row { values } | Self::Func { args: values, .. } => { for value in values { - value.visit_columns(visit); + value.visit_paths(visit); } } Self::Trim { expr, characters, .. } => { - expr.visit_columns(visit); + expr.visit_paths(visit); if let Some(chars) = characters { - chars.visit_columns(visit); + chars.visit_paths(visit); } } Self::AggregateFilter { @@ -308,8 +302,8 @@ impl ExprNode { | Self::VectorBinary { left, right, .. } | Self::Binary { left, right, .. } | Self::Bool { left, right, .. } => { - left.visit_columns(visit); - right.visit_columns(visit); + left.visit_paths(visit); + right.visit_paths(visit); } Self::Normalize { expr, .. } | Self::MakeInterval { value: expr, .. } @@ -318,7 +312,7 @@ impl ExprNode { | Self::In { expr, .. } | Self::RowIn { expr, .. } | Self::IsNull { expr, .. } - | Self::Like { expr, .. } => expr.visit_columns(visit), + | Self::Like { expr, .. } => expr.visit_paths(visit), // Subqueries discover their own paths when compiled in their enclosing scope. Self::Value(_) | Self::Exists { .. } => {} } @@ -335,20 +329,10 @@ enum PlannedJoin { } impl PlannedJoin { - fn depends_on(&self, path: &[Relation], alias: &str, builder: &crate::compile::SqlBuilder) -> bool { + fn depends_on(&self, path: &[Relation], alias: &str, on: &crate::compile::SqlBuilder) -> bool { match self { Self::Related { path: child, .. } => child.starts_with(path), - Self::Declared(join) => { - let mut depends = false; - join.on.node.visit_columns(&mut |column, column_path| { - depends |= if column_path.is_empty() { - builder.column_qualifier(column.table) == alias - } else { - column_path.starts_with(path) - }; - }); - depends - } + Self::Declared(_) => on.references_qualifier(alias), } } } @@ -476,25 +460,32 @@ impl JoinPlan { } pub(crate) fn write(&self, builder: &mut crate::compile::SqlBuilder) { - use crate::compile::ToSql; - let mut joins: Vec<_> = self.joins.iter().collect(); + let mut joins: Vec<_> = self + .joins + .iter() + .map(|join| { + let on = match join { + PlannedJoin::Declared(join) => builder.compile_expression(&join.on.node), + PlannedJoin::Related { path, .. } => builder.compile_expression(&join_on(path).node), + }; + (join, on) + }) + .collect(); // Children are planned after parents. Moving them first lets each parent // follow its earliest dependent without reordering custom joins. for join in self.joins.iter().rev() { if let PlannedJoin::Related { path, alias, .. } = join { - let index = joins.iter().position(|other| std::ptr::eq(*other, join)).unwrap(); - if let Some(before) = joins[..index].iter().position(|other| other.depends_on(path, alias, builder)) { - joins.remove(index); - joins.insert(before, join); + let index = joins.iter().position(|(other, _)| std::ptr::eq(*other, join)).unwrap(); + if let Some(before) = joins[..index].iter().position(|(other, on)| other.depends_on(path, alias, on)) { + let entry = joins.remove(index); + joins.insert(before, entry); } } } - for join in joins { - let (table, alias, kind, on) = match join { - PlannedJoin::Declared(join) => (join.table, join.table.alias, join.kind, join.on.clone()), - PlannedJoin::Related { path, alias, kind } => { - (path.last().unwrap().join_table(), Some(alias.as_str()), *kind, join_on(path)) - } + for (join, on) in joins { + let (table, alias, kind) = match join { + PlannedJoin::Declared(join) => (join.table, join.table.alias, join.kind), + PlannedJoin::Related { path, alias, kind } => (path.last().unwrap().join_table(), Some(alias.as_str()), *kind), }; builder.push_sql(match kind { crate::JoinKind::Inner => " JOIN ", @@ -506,7 +497,7 @@ impl JoinPlan { builder.push_sql(alias); } builder.push_sql(" ON "); - on.node.to_sql(builder); + builder.push_expression(on); } } } diff --git a/crates/dbkit-core/src/query.rs b/crates/dbkit-core/src/query.rs index 05098d3..4508a52 100644 --- a/crates/dbkit-core/src/query.rs +++ b/crates/dbkit-core/src/query.rs @@ -369,12 +369,13 @@ impl Select CompiledSql { + pub(crate) fn compile_for_exists(&self, scope: QueryScope) -> SqlBuilder { self.compile_inner_with((&[], &[]), true, true, true, scope) } pub fn compile_with_extra(&self, extra_columns: &[SelectItem], extra_joins: &[Join]) -> CompiledSql { self.compile_inner_with((extra_columns, extra_joins), true, true, true, QueryScope::default()) + .finish() } fn compile_inner(&self, include_order: bool, include_pagination: bool, include_locking: bool) -> CompiledSql { @@ -385,6 +386,7 @@ impl Select Select CompiledSql { + ) -> SqlBuilder { let (extra_columns, extra_joins) = extra; let mut plan = crate::path::JoinPlan::new(self.table, &self.joins, extra_joins, &scope.qualifiers); for item in self.columns.iter().flatten().chain(extra_columns) { @@ -530,7 +532,7 @@ impl Select String { From ce7437880ae4569ce1cfb4189f590976325a251a Mon Sep 17 00:00:00 2001 From: Alexander Eichhorn Date: Tue, 8 Sep 2026 15:13:30 +0200 Subject: [PATCH 22/22] Reserve internal model field names --- crates/dbkit-derive/src/lib.rs | 8 +++ crates/dbkit/tests/derive_ui.rs | 2 + .../tests/ui/fail_reserved_model_fields.rs | 53 +++++++++++++++++++ .../tests/ui/pass_model_field_prefixes.rs | 37 +++++++++++++ 4 files changed, 100 insertions(+) create mode 100644 crates/dbkit/tests/ui/fail_reserved_model_fields.rs create mode 100644 crates/dbkit/tests/ui/pass_model_field_prefixes.rs diff --git a/crates/dbkit-derive/src/lib.rs b/crates/dbkit-derive/src/lib.rs index f6133ff..95e6b97 100644 --- a/crates/dbkit-derive/src/lib.rs +++ b/crates/dbkit-derive/src/lib.rs @@ -1,5 +1,6 @@ use proc_macro::TokenStream; use quote::{format_ident, quote}; +use syn::ext::IdentExt; use syn::parse::Parser; use syn::{parse_macro_input, Attribute, Field, Fields, Ident, ItemStruct, Meta, Type}; @@ -108,6 +109,13 @@ fn expand_model(args: ModelArgs, input: ItemStruct) -> syn::Result .clone() .ok_or_else(|| syn::Error::new_spanned(&field, "dbkit: unnamed field"))?; + if field_ident.unraw().to_string().starts_with("__dbkit_") { + return Err(syn::Error::new_spanned( + field_ident, + "dbkit: field names starting with `__dbkit_` are reserved for dbkit internals", + )); + } + let is_relation = has_attr(&field.attrs, "has_many") || has_attr(&field.attrs, "belongs_to") || has_attr(&field.attrs, "many_to_many"); diff --git a/crates/dbkit/tests/derive_ui.rs b/crates/dbkit/tests/derive_ui.rs index 7975e46..646129d 100644 --- a/crates/dbkit/tests/derive_ui.rs +++ b/crates/dbkit/tests/derive_ui.rs @@ -74,6 +74,8 @@ fn main() -> ui_test::color_eyre::Result<()> { "fail_relation_path_operators.rs".into(), "pass_relation_paths_graphs.rs".into(), "pass_relation_paths_field_names.rs".into(), + "fail_reserved_model_fields.rs".into(), + "pass_model_field_prefixes.rs".into(), "fail_relation_paths_value_types.rs".into(), "fail_relation_paths_nullability.rs".into(), "fail_relation_paths_assignment.rs".into(), diff --git a/crates/dbkit/tests/ui/fail_reserved_model_fields.rs b/crates/dbkit/tests/ui/fail_reserved_model_fields.rs new file mode 100644 index 0000000..2a18e29 --- /dev/null +++ b/crates/dbkit/tests/ui/fail_reserved_model_fields.rs @@ -0,0 +1,53 @@ +use dbkit::model; + +#[model(table = "marker_fields")] +struct MarkerField { + #[key] + id: i64, + __dbkit_marker: String, //~ ERROR: field names starting with `__dbkit_` are reserved for dbkit internals +} + +#[model(table = "future_fields")] +struct FutureField { + #[key] + id: i64, + __dbkit_future: bool, //~ ERROR: field names starting with `__dbkit_` are reserved for dbkit internals +} + +#[model(table = "renamed_fields")] +struct RenamedField { + #[key] + id: i64, + #[dbkit(column = "public_label")] + __dbkit_label: String, //~ ERROR: field names starting with `__dbkit_` are reserved for dbkit internals +} + +#[model(table = "raw_fields")] +struct RawField { + #[key] + id: i64, + r#__dbkit_raw: String, //~ ERROR: field names starting with `__dbkit_` are reserved for dbkit internals +} + +#[model(table = "reserved_keys")] +struct ReservedKey { + #[key] + __dbkit_id: i64, //~ ERROR: field names starting with `__dbkit_` are reserved for dbkit internals +} + +#[model(table = "targets")] +struct Target { + #[key] + id: i64, +} + +#[model(table = "reserved_relations")] +struct ReservedRelation { + #[key] + id: i64, + owner_id: i64, + #[belongs_to(key = owner_id, references = id)] + __dbkit_owner: dbkit::BelongsTo, //~ ERROR: field names starting with `__dbkit_` are reserved for dbkit internals +} + +fn main() {} diff --git a/crates/dbkit/tests/ui/pass_model_field_prefixes.rs b/crates/dbkit/tests/ui/pass_model_field_prefixes.rs new file mode 100644 index 0000000..5f52825 --- /dev/null +++ b/crates/dbkit/tests/ui/pass_model_field_prefixes.rs @@ -0,0 +1,37 @@ +//@check-pass +#![allow(non_upper_case_globals)] +use dbkit::model; + +#[model(table = "accepted_fields")] +struct Accepted { + #[key] + id: i64, + dbkit_value: i32, + _dbkit_value: i32, + __dbkit: i32, + __dbkitx_value: i32, + // Only Rust field names are reserved; existing database column names stay usable. + #[dbkit(column = "__dbkit_marker")] + label: String, +} + +#[model(table = "sources")] +struct Source { + #[key] + id: i64, + target_id: i64, + #[belongs_to(key = target_id, references = id)] + target: dbkit::BelongsTo, +} + +fn main() { + let _ = Accepted::query() + .filter(Accepted::dbkit_value.eq(1_i32)) + .filter(Accepted::_dbkit_value.eq(2_i32)) + .filter(Accepted::__dbkit.eq(3_i32)) + .filter(Accepted::__dbkitx_value.eq(4_i32)) + .filter(Accepted::label.eq("value")); + let _ = Source::query() + .filter(Source::target.__dbkit.eq(3_i32)) + .filter(Source::target.label.eq("value")); +}