Skip to content

Commit bbea5f5

Browse files
sgrifsgrif
committed
Remove the Copy constraint from Identifiable
I had originally wanted to returned owned values here because I'm thinking that when we eventually add support for composite primary keys, I'll need to return a tuple of two fields from this method. I've gone back and forth on a bunch of different options, but ultimately I think I'm going to change this to return `Cow<'a, Self::Id>`. However, I don't want to make that change pre-emptively, as I'm still not sure exactly what composite primary keys will look like. Other alternatives: - Implement `Identifiable` on `&'a T` -- makes reasoning about it a pain - Change `Identifiable` to be `Identifiable<'a>` -- makes referencing it a pain - Change `id` to `with_id`, and have it take a `FnOnce(&Self::Id)`. This would work, but would be really weird and just returning `Cow<'a, Self::Id>` makes way more sense.
1 parent d9e680a commit bbea5f5

12 files changed

Lines changed: 83 additions & 37 deletions

File tree

‎diesel/src/associations/belongs_to.rs‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use super::Identifiable;
88
pub trait BelongsTo<Parent: Identifiable> {
99
type ForeignKeyColumn: Column;
1010

11-
fn foreign_key(&self) -> Parent::Id;
11+
fn foreign_key(&self) -> &Parent::Id;
1212
fn foreign_key_column() -> Self::ForeignKeyColumn;
1313
}
1414

@@ -27,56 +27,56 @@ impl<Parent, Child, Iter> GroupedBy<Parent> for Iter where
2727
let id_indices: HashMap<_, _> = parents.iter().enumerate().map(|(i, u)| (u.id(), i)).collect();
2828
let mut result = parents.iter().map(|_| Vec::new()).collect::<Vec<_>>();
2929
for child in self {
30-
let index = id_indices[&child.foreign_key()];
30+
let index = id_indices[child.foreign_key()];
3131
result[index].push(child);
3232
}
3333
result
3434
}
3535
}
3636

37-
impl<Parent, Child> BelongingToDsl<Parent> for Child where
37+
impl<'a, Parent, Child> BelongingToDsl<&'a Parent> for Child where
3838
Parent: Identifiable,
3939
Child: Identifiable + BelongsTo<Parent>,
40-
Parent::Id: AsExpression<<Child::ForeignKeyColumn as Expression>::SqlType>,
41-
<Child as Identifiable>::Table: FilterDsl<Eq<Child::ForeignKeyColumn, Parent::Id>>,
40+
&'a Parent::Id: AsExpression<<Child::ForeignKeyColumn as Expression>::SqlType>,
41+
<Child as Identifiable>::Table: FilterDsl<Eq<Child::ForeignKeyColumn, &'a Parent::Id>>,
4242
{
4343
type Output = FindBy<
4444
Child::Table,
4545
Child::ForeignKeyColumn,
46-
Parent::Id,
46+
&'a Parent::Id,
4747
>;
4848

49-
fn belonging_to(parent: &Parent) -> Self::Output {
49+
fn belonging_to(parent: &'a Parent) -> Self::Output {
5050
Child::table().filter(Child::foreign_key_column().eq(parent.id()))
5151
}
5252
}
5353

54-
impl<Parent, Child> BelongingToDsl<[Parent]> for Child where
54+
impl<'a, Parent, Child> BelongingToDsl<&'a [Parent]> for Child where
5555
Parent: Identifiable,
5656
Child: Identifiable + BelongsTo<Parent>,
57-
Vec<Parent::Id>: AsInExpression<<Child::ForeignKeyColumn as Expression>::SqlType>,
58-
<Child as Identifiable>::Table: FilterDsl<EqAny<Child::ForeignKeyColumn, Vec<Parent::Id>>>,
57+
Vec<&'a Parent::Id>: AsInExpression<<Child::ForeignKeyColumn as Expression>::SqlType>,
58+
<Child as Identifiable>::Table: FilterDsl<EqAny<Child::ForeignKeyColumn, Vec<&'a Parent::Id>>>,
5959
{
6060
type Output = Filter<
6161
Child::Table,
6262
EqAny<
6363
Child::ForeignKeyColumn,
64-
Vec<Parent::Id>,
64+
Vec<&'a Parent::Id>,
6565
>,
6666
>;
6767

68-
fn belonging_to(parents: &[Parent]) -> Self::Output {
68+
fn belonging_to(parents: &'a [Parent]) -> Self::Output {
6969
let ids = parents.iter().map(Parent::id).collect::<Vec<_>>();
7070
Child::table().filter(Child::foreign_key_column().eq_any(ids))
7171
}
7272
}
7373

74-
impl<Parent, Child> BelongingToDsl<Vec<Parent>> for Child where
75-
Child: BelongingToDsl<[Parent]>,
74+
impl<'a, Parent, Child> BelongingToDsl<&'a Vec<Parent>> for Child where
75+
Child: BelongingToDsl<&'a [Parent]>,
7676
{
7777
type Output = Child::Output;
7878

79-
fn belonging_to(parents: &Vec<Parent>) -> Self::Output {
79+
fn belonging_to(parents: &'a Vec<Parent>) -> Self::Output {
8080
Self::belonging_to(&**parents)
8181
}
8282
}

‎diesel/src/associations/mod.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,9 @@ use query_source::Table;
126126
pub use self::belongs_to::{BelongsTo, GroupedBy};
127127

128128
pub trait Identifiable {
129-
type Id: Hash + Eq + Copy;
130-
type Table: Table + FindDsl<Self::Id>;
129+
type Id: Hash + Eq;
130+
type Table: Table + for<'a> FindDsl<&'a Self::Id>;
131131

132132
fn table() -> Self::Table;
133-
fn id(&self) -> Self::Id;
133+
fn id(&self) -> &Self::Id;
134134
}

‎diesel/src/macros/associations/belongs_to.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ macro_rules! BelongsTo {
116116
impl $crate::associations::BelongsTo<$parent_struct> for $struct_name {
117117
type ForeignKeyColumn = $child_table_name::$foreign_key_name;
118118

119-
fn foreign_key(&self) -> <$parent_struct as $crate::associations::Identifiable>::Id {
120-
self.$foreign_key_name
119+
fn foreign_key(&self) -> &<$parent_struct as $crate::associations::Identifiable>::Id {
120+
&self.$foreign_key_name
121121
}
122122

123123
fn foreign_key_column() -> Self::ForeignKeyColumn {

‎diesel/src/macros/identifiable.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ macro_rules! Identifiable {
9494
$table_name::table
9595
}
9696

97-
fn id(&self) -> Self::Id {
98-
self.id
97+
fn id(&self) -> &Self::Id {
98+
&self.id
9999
}
100100
}
101101
};
@@ -165,8 +165,8 @@ fn derive_identifiable_on_simple_struct() {
165165

166166
let foo1 = Foo { id: 1, foo: 2 };
167167
let foo2 = Foo { id: 2, foo: 3 };
168-
assert_eq!(1, foo1.id());
169-
assert_eq!(2, foo2.id());
168+
assert_eq!(&1, foo1.id());
169+
assert_eq!(&2, foo2.id());
170170
}
171171

172172
#[test]
@@ -189,8 +189,8 @@ fn derive_identifiable_when_id_is_not_first_field() {
189189

190190
let foo1 = Foo { id: 1, foo: 2 };
191191
let foo2 = Foo { id: 2, foo: 3 };
192-
assert_eq!(1, foo1.id());
193-
assert_eq!(2, foo2.id());
192+
assert_eq!(&1, foo1.id());
193+
assert_eq!(&2, foo2.id());
194194
}
195195

196196
#[test]
@@ -213,6 +213,6 @@ fn derive_identifiable_on_struct_with_non_integer_pk() {
213213

214214
let foo1 = Foo { id: "hi", foo: 2 };
215215
let foo2 = Foo { id: "there", foo: 3 };
216-
assert_eq!("hi", foo1.id());
217-
assert_eq!("there", foo2.id());
216+
assert_eq!(&"hi", foo1.id());
217+
assert_eq!(&"there", foo2.id());
218218
}

‎diesel/src/query_builder/update_statement/target.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pub trait IntoUpdateTarget {
1818
}
1919

2020
impl<'a, T: Identifiable, V> IntoUpdateTarget for &'a T where
21-
<T::Table as FindDsl<T::Id>>::Output: IntoUpdateTarget<Table=T::Table, WhereClause=V>,
21+
<T::Table as FindDsl<&'a T::Id>>::Output: IntoUpdateTarget<Table=T::Table, WhereClause=V>,
2222
{
2323
type Table = T::Table;
2424
type WhereClause = V;

‎diesel/src/query_dsl/belonging_to_dsl.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use query_builder::AsQuery;
22

3-
pub trait BelongingToDsl<T: ?Sized> {
3+
pub trait BelongingToDsl<T> {
44
type Output: AsQuery;
55

6-
fn belonging_to(other: &T) -> Self::Output;
6+
fn belonging_to(other: T) -> Self::Output;
77
}

‎diesel/src/query_dsl/save_changes_dsl.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ impl<'a, T, ST> SaveChangesDsl<SqliteConnection, ST> for &'a T where
4242
T::Table: AsQuery<SqlType=ST>,
4343
&'a T: AsChangeset<Target=T::Table> + IntoUpdateTarget<Table=T::Table>,
4444
Update<&'a T, &'a T>: ExecuteDsl<SqliteConnection>,
45-
Find<T::Table, T::Id>: LoadDsl<SqliteConnection, SqlType=ST>,
45+
Find<T::Table, &'a T::Id>: LoadDsl<SqliteConnection, SqlType=ST>,
4646
{
4747
fn save_changes<U>(self, conn: &SqliteConnection) -> QueryResult<U> where
4848
U: Queryable<ST, Sqlite>,

‎diesel/src/types/impls/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ macro_rules! expression_impls {
2020
}
2121
}
2222

23-
impl<'a: 'expr, 'expr> $crate::expression::AsExpression<types::$Source> for &'expr $Target {
23+
impl<'a, 'expr> $crate::expression::AsExpression<types::$Source> for &'expr $Target {
2424
type Expression = $crate::expression::bound::Bound<types::$Source, Self>;
2525

2626
fn as_expression(self) -> Self::Expression {
@@ -36,7 +36,7 @@ macro_rules! expression_impls {
3636
}
3737
}
3838

39-
impl<'a: 'expr, 'expr> $crate::expression::AsExpression<$crate::types::Nullable<types::$Source>> for &'a $Target {
39+
impl<'a, 'expr> $crate::expression::AsExpression<$crate::types::Nullable<types::$Source>> for &'a $Target {
4040
type Expression = $crate::expression::bound::Bound<$crate::types::Nullable<types::$Source>, Self>;
4141

4242
fn as_expression(self) -> Self::Expression {

‎diesel/src/types/impls/tuples.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,8 @@ macro_rules! tuple_impls {
286286
{
287287
type ForeignKeyColumn = A::ForeignKeyColumn;
288288

289-
fn foreign_key(&self) -> Parent::Id {
290-
self.0.foreign_key()
289+
fn foreign_key(&self) -> &Parent::Id {
290+
&self.0.foreign_key()
291291
}
292292

293293
fn foreign_key_column() -> Self::ForeignKeyColumn {

‎diesel_tests/tests/associations.rs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,52 @@ fn eager_loading_associations_for_multiple_records() {
3434
assert_eq!(expected_data, users_and_posts);
3535
}
3636

37+
mod eager_loading_with_string_keys {
38+
use diesel::*;
39+
use diesel::connection::SimpleConnection;
40+
use schema::connection;
41+
42+
table! { users { id -> Text, } }
43+
table! { posts { id -> Text, user_id -> Text, } }
44+
45+
#[derive(Queryable, Identifiable, Debug, PartialEq, Clone)]
46+
pub struct User {
47+
id: String,
48+
}
49+
50+
#[derive(Queryable, Identifiable, Debug, PartialEq, Clone)]
51+
#[belongs_to(User)]
52+
pub struct Post {
53+
id: String,
54+
user_id: String,
55+
}
56+
57+
#[test]
58+
fn eager_loading_associations_for_multiple_records() {
59+
let connection = connection();
60+
connection.batch_execute(r#"
61+
DROP TABLE users;
62+
DROP TABLE posts;
63+
CREATE TABLE users (id TEXT PRIMARY KEY NOT NULL);
64+
CREATE TABLE posts (id TEXT PRIMARY KEY NOT NULL, user_id TEXT NOT NULL);
65+
INSERT INTO users (id) VALUES ('Sean'), ('Tess');
66+
INSERT INTO posts (id, user_id) VALUES ('Hello', 'Sean'), ('World', 'Sean'), ('Hello 2', 'Tess');
67+
"#).unwrap();
68+
let sean = User { id: "Sean".into() };
69+
let tess = User { id: "Tess".into() };
70+
71+
let users = vec![sean.clone(), tess.clone()];
72+
let posts = Post::belonging_to(&users).load::<Post>(&connection).unwrap()
73+
.grouped_by(&users);
74+
let users_and_posts = users.into_iter().zip(posts).collect::<Vec<_>>();
75+
76+
let seans_posts = Post::belonging_to(&sean).load(&connection).unwrap();
77+
let tess_posts = Post::belonging_to(&tess).load(&connection).unwrap();
78+
let expected_data = vec![(sean, seans_posts), (tess, tess_posts)];
79+
assert_eq!(expected_data, users_and_posts);
80+
}
81+
}
82+
3783
#[test]
3884
fn grouping_associations_maintains_ordering() {
3985
let (connection, sean, tess, _) = conn_with_test_data();

0 commit comments

Comments
 (0)