Skip to content

Commit 9301591

Browse files
weiznichweiznich
authored
Merge pull request diesel-rs#2628 from weiznich/fix/2203
Fix diesel-rs#2203
2 parents 8f149f9 + efe2566 commit 9301591

13 files changed

Lines changed: 266 additions & 74 deletions

File tree

‎diesel/src/connection/statement_cache.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ where
215215
None => {
216216
let sql = Self::construct_sql(source)?;
217217
Ok(StatementCacheKey::Sql {
218-
sql: sql,
218+
sql,
219219
bind_types: bind_types.into(),
220220
})
221221
}

‎diesel/src/pg/backend.rs‎

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,26 +6,80 @@ use super::query_builder::PgQueryBuilder;
66
use super::{PgMetadataLookup, PgValue};
77
use crate::backend::*;
88
use crate::deserialize::Queryable;
9+
use crate::pg::metadata_lookup::PgMetadataCacheKey;
910
use crate::query_builder::bind_collector::RawBytesBindCollector;
1011
use crate::sql_types::TypeMetadata;
1112

1213
/// The PostgreSQL backend
1314
#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)]
1415
pub struct Pg;
1516

17+
#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, Queryable)]
18+
pub(in crate::pg) struct InnerPgTypeMetadata {
19+
pub oid: u32,
20+
pub array_oid: u32,
21+
}
22+
23+
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
24+
pub(in crate::pg) struct FailedToLookupTypeError(Box<PgMetadataCacheKey<'static>>);
25+
26+
impl FailedToLookupTypeError {
27+
pub(crate) fn new(cache_key: PgMetadataCacheKey<'static>) -> Self {
28+
Self(Box::new(cache_key))
29+
}
30+
}
31+
32+
impl std::error::Error for FailedToLookupTypeError {}
33+
34+
impl std::fmt::Display for FailedToLookupTypeError {
35+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
36+
if let Some(schema) = self.0.schema.as_ref() {
37+
write!(
38+
f,
39+
"Failed to find a type oid for `{}`.`{}`",
40+
schema, self.0.type_name
41+
)
42+
} else {
43+
write!(f, "Failed to find a type oid for `{}`", self.0.type_name)
44+
}
45+
}
46+
}
47+
1648
/// The [OIDs] for a SQL type
1749
///
1850
/// [OIDs]: https://www.postgresql.org/docs/current/static/datatype-oid.html
19-
#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, Default, Queryable)]
20-
pub struct PgTypeMetadata {
51+
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
52+
#[must_use]
53+
pub struct PgTypeMetadata(pub(in crate::pg) Result<InnerPgTypeMetadata, FailedToLookupTypeError>);
54+
55+
impl PgTypeMetadata {
56+
/// Create a new instance of this type based on known constant [OIDs].
57+
///
58+
/// Please refer to [PgMetadataLookup] for a way to query [OIDs]
59+
/// of custom types at run time
60+
///
61+
/// [OIDs]: https://www.postgresql.org/docs/current/static/datatype-oid.html
62+
/// [PgMetadataLookup]: struct.PgMetadataLookup.html
63+
pub fn new(type_oid: u32, array_oid: u32) -> Self {
64+
Self(Ok(InnerPgTypeMetadata {
65+
oid: type_oid,
66+
array_oid,
67+
}))
68+
}
69+
2170
/// The [OID] of `T`
2271
///
2372
/// [OID]: https://www.postgresql.org/docs/current/static/datatype-oid.html
24-
pub oid: u32,
73+
pub fn oid(&self) -> Result<u32, impl std::error::Error + Send + Sync> {
74+
self.0.as_ref().map(|i| i.oid).map_err(Clone::clone)
75+
}
76+
2577
/// The [OID] of `T[]`
2678
///
2779
/// [OID]: https://www.postgresql.org/docs/current/static/datatype-oid.html
28-
pub array_oid: u32,
80+
pub fn array_oid(self) -> Result<u32, impl std::error::Error + Send + Sync> {
81+
self.0.as_ref().map(|i| i.array_oid).map_err(Clone::clone)
82+
}
2983
}
3084

3185
impl Backend for Pg {

‎diesel/src/pg/connection/stmt/mod.rs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,11 @@ impl Statement {
5757
) -> QueryResult<Self> {
5858
let name = CString::new(name.unwrap_or(""))?;
5959
let sql = CString::new(sql)?;
60-
let param_types_vec = param_types.iter().map(|x| x.oid).collect();
60+
let param_types_vec = param_types
61+
.iter()
62+
.map(|x| x.oid())
63+
.collect::<Result<Vec<_>, _>>()
64+
.map_err(|e| crate::result::Error::SerializationError(Box::new(e)))?;
6165

6266
let internal_result = unsafe {
6367
conn.raw_connection.prepare(

‎diesel/src/pg/metadata_lookup.rs‎

Lines changed: 81 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
#![allow(unused_parens)] // FIXME: Remove this attribute once false positive is resolved.
22

3+
use super::backend::{FailedToLookupTypeError, InnerPgTypeMetadata};
34
use super::{PgConnection, PgTypeMetadata};
45
use crate::prelude::*;
56

7+
use std::borrow::Cow;
68
use std::cell::RefCell;
79
use std::collections::HashMap;
810

@@ -24,26 +26,78 @@ impl PgMetadataLookup {
2426
/// This function should only be used for user defined types, or types which
2527
/// come from an extension. This function may perform a SQL query to look
2628
/// up the type. For built-in types, a static OID should be preferred.
27-
pub fn lookup_type(&self, type_name: &str) -> PgTypeMetadata {
29+
pub fn lookup_type(&self, type_name: &str, schema: Option<&str>) -> PgTypeMetadata {
2830
let metadata_cache = self.conn.get_metadata_cache();
29-
metadata_cache.lookup_type(type_name).unwrap_or_else(|| {
30-
use self::pg_type::dsl::*;
31-
32-
let type_metadata = pg_type
33-
.select((oid, typarray))
34-
.filter(typname.eq(type_name))
35-
.first(&self.conn)
36-
.unwrap_or_default();
37-
metadata_cache.store_type(type_name, type_metadata);
38-
type_metadata
31+
let cache_key = PgMetadataCacheKey {
32+
schema: schema.map(Cow::Borrowed),
33+
type_name: Cow::Borrowed(type_name),
34+
};
35+
36+
metadata_cache.lookup_type(&cache_key).unwrap_or_else(|| {
37+
let r = lookup_type(&cache_key, &self.conn);
38+
39+
match r {
40+
Ok(type_metadata) => {
41+
metadata_cache.store_type(cache_key, type_metadata);
42+
PgTypeMetadata(Ok(type_metadata))
43+
}
44+
Err(_e) => {
45+
PgTypeMetadata(Err(FailedToLookupTypeError::new(cache_key.into_owned())))
46+
}
47+
}
3948
})
4049
}
4150
}
4251

52+
fn lookup_type(
53+
cache_key: &PgMetadataCacheKey<'_>,
54+
conn: &PgConnection,
55+
) -> QueryResult<InnerPgTypeMetadata> {
56+
let search_path: String;
57+
58+
let search_schema = if let Some(schema) = cache_key.schema.as_ref() {
59+
vec![schema.as_ref()]
60+
} else {
61+
search_path = crate::dsl::sql("SHOW search_path").get_result::<String>(conn)?;
62+
63+
search_path
64+
.split(',')
65+
// skip the `$user` entry for now
66+
.filter(|f| !f.starts_with("\"$"))
67+
.map(|s| s.trim())
68+
.collect()
69+
};
70+
71+
let metadata = pg_type::table
72+
.inner_join(pg_namespace::table)
73+
.filter(pg_namespace::nspname.eq(crate::dsl::any(search_schema)))
74+
.filter(pg_type::typname.eq(&cache_key.type_name))
75+
.select((pg_type::oid, pg_type::typarray))
76+
.first(conn)?;
77+
78+
Ok(metadata)
79+
}
80+
81+
#[derive(Hash, PartialEq, Eq, Debug, Clone)]
82+
pub(in crate::pg) struct PgMetadataCacheKey<'a> {
83+
pub(crate) schema: Option<Cow<'a, str>>,
84+
pub(crate) type_name: Cow<'a, str>,
85+
}
86+
87+
impl<'a> PgMetadataCacheKey<'a> {
88+
fn into_owned(self) -> PgMetadataCacheKey<'static> {
89+
let PgMetadataCacheKey { schema, type_name } = self;
90+
PgMetadataCacheKey {
91+
schema: schema.map(|s| Cow::Owned(s.into_owned())),
92+
type_name: Cow::Owned(type_name.into_owned()),
93+
}
94+
}
95+
}
96+
4397
/// Stores a cache for the OID of custom types
4498
#[allow(missing_debug_implementations)]
4599
pub struct PgMetadataCache {
46-
cache: RefCell<HashMap<String, PgTypeMetadata>>,
100+
cache: RefCell<HashMap<PgMetadataCacheKey<'static>, InnerPgTypeMetadata>>,
47101
}
48102

49103
impl PgMetadataCache {
@@ -53,14 +107,14 @@ impl PgMetadataCache {
53107
}
54108
}
55109

56-
fn lookup_type(&self, type_name: &str) -> Option<PgTypeMetadata> {
57-
Some(*self.cache.borrow().get(type_name)?)
110+
fn lookup_type(&self, type_name: &PgMetadataCacheKey) -> Option<PgTypeMetadata> {
111+
Some(PgTypeMetadata(Ok(*self.cache.borrow().get(type_name)?)))
58112
}
59113

60-
fn store_type(&self, type_name: &str, type_metadata: PgTypeMetadata) {
114+
fn store_type(&self, type_name: PgMetadataCacheKey, type_metadata: InnerPgTypeMetadata) {
61115
self.cache
62116
.borrow_mut()
63-
.insert(type_name.to_owned(), type_metadata);
117+
.insert(type_name.into_owned(), type_metadata);
64118
}
65119
}
66120

@@ -69,5 +123,16 @@ table! {
69123
oid -> Oid,
70124
typname -> Text,
71125
typarray -> Oid,
126+
typnamespace -> Oid,
127+
}
128+
}
129+
130+
table! {
131+
pg_namespace (oid) {
132+
oid -> Oid,
133+
nspname -> Text,
72134
}
73135
}
136+
137+
joinable!(pg_type -> pg_namespace(typnamespace));
138+
allow_tables_to_appear_in_same_query!(pg_type, pg_namespace);

‎diesel/src/pg/types/array.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ where
1212
Pg: HasSqlType<T>,
1313
{
1414
fn metadata(lookup: &PgMetadataLookup) -> PgTypeMetadata {
15-
PgTypeMetadata {
16-
oid: <Pg as HasSqlType<T>>::metadata(lookup).array_oid,
17-
array_oid: 0,
15+
match <Pg as HasSqlType<T>>::metadata(lookup).0 {
16+
Ok(tpe) => PgTypeMetadata::new(tpe.array_oid, 0),
17+
c @ Err(_) => PgTypeMetadata(c),
1818
}
1919
}
2020
}
@@ -91,7 +91,7 @@ where
9191
out.write_i32::<NetworkEndian>(num_dimensions)?;
9292
let flags = 0;
9393
out.write_i32::<NetworkEndian>(flags)?;
94-
let element_oid = Pg::metadata(out.metadata_lookup()).oid;
94+
let element_oid = Pg::metadata(out.metadata_lookup()).oid()?;
9595
out.write_u32::<NetworkEndian>(element_oid)?;
9696
out.write_i32::<NetworkEndian>(self.len() as i32)?;
9797
let lower_bound = 1;

‎diesel/src/pg/types/ranges.rs‎

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -160,54 +160,36 @@ where
160160

161161
impl HasSqlType<Int4range> for Pg {
162162
fn metadata(_: &PgMetadataLookup) -> PgTypeMetadata {
163-
PgTypeMetadata {
164-
oid: 3904,
165-
array_oid: 3905,
166-
}
163+
PgTypeMetadata::new(3904, 3905)
167164
}
168165
}
169166

170167
impl HasSqlType<Numrange> for Pg {
171168
fn metadata(_: &PgMetadataLookup) -> PgTypeMetadata {
172-
PgTypeMetadata {
173-
oid: 3906,
174-
array_oid: 3907,
175-
}
169+
PgTypeMetadata::new(3906, 3907)
176170
}
177171
}
178172

179173
impl HasSqlType<Tsrange> for Pg {
180174
fn metadata(_: &PgMetadataLookup) -> PgTypeMetadata {
181-
PgTypeMetadata {
182-
oid: 3908,
183-
array_oid: 3909,
184-
}
175+
PgTypeMetadata::new(3908, 3909)
185176
}
186177
}
187178

188179
impl HasSqlType<Tstzrange> for Pg {
189180
fn metadata(_: &PgMetadataLookup) -> PgTypeMetadata {
190-
PgTypeMetadata {
191-
oid: 3910,
192-
array_oid: 3911,
193-
}
181+
PgTypeMetadata::new(3910, 3911)
194182
}
195183
}
196184

197185
impl HasSqlType<Daterange> for Pg {
198186
fn metadata(_: &PgMetadataLookup) -> PgTypeMetadata {
199-
PgTypeMetadata {
200-
oid: 3912,
201-
array_oid: 3913,
202-
}
187+
PgTypeMetadata::new(3912, 3913)
203188
}
204189
}
205190

206191
impl HasSqlType<Int8range> for Pg {
207192
fn metadata(_: &PgMetadataLookup) -> PgTypeMetadata {
208-
PgTypeMetadata {
209-
oid: 3926,
210-
array_oid: 3927,
211-
}
193+
PgTypeMetadata::new(3926, 3927)
212194
}
213195
}

‎diesel/src/pg/types/record.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ macro_rules! tuple_impls {
103103
out.write_i32::<NetworkEndian>($Tuple)?;
104104

105105
$(
106-
let oid = <Pg as HasSqlType<$ST>>::metadata(out.metadata_lookup()).oid;
106+
let oid = <Pg as HasSqlType<$ST>>::metadata(out.metadata_lookup()).oid()?;
107107
out.write_u32::<NetworkEndian>(oid)?;
108108
let is_null = self.$idx.to_sql(&mut buffer)?;
109109

‎diesel_compile_tests/tests/fail/array_expressions_must_be_same_type.stderr‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ error[E0277]: the trait bound `{integer}: diesel::SelectableExpression<()>` is n
106106
<(A, B) as diesel::SelectableExpression<QS>>
107107
<(A, B, C) as diesel::SelectableExpression<QS>>
108108
<(A, B, C, D) as diesel::SelectableExpression<QS>>
109-
and 108 others
109+
and 124 others
110110
= note: required because of the requirements on the impl of `diesel::SelectableExpression<()>` for `({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>)`
111111
= note: required because of the requirements on the impl of `diesel::SelectableExpression<()>` for `diesel::pg::expression::array::ArrayLiteral<({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>), diesel::sql_types::Double>`
112112
= note: required because of the requirements on the impl of `diesel::query_dsl::select_dsl::SelectDsl<diesel::pg::expression::array::ArrayLiteral<({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>), diesel::sql_types::Double>>` for `diesel::query_builder::SelectStatement<()>`
@@ -127,7 +127,7 @@ error[E0277]: the trait bound `{integer}: diesel::expression::ValidGrouping<()>`
127127
<(A, B) as diesel::expression::ValidGrouping<__GroupByClause>>
128128
<(A, B, C) as diesel::expression::ValidGrouping<__GroupByClause>>
129129
<(A, B, C, D) as diesel::expression::ValidGrouping<__GroupByClause>>
130-
and 111 others
130+
and 118 others
131131
= note: required because of the requirements on the impl of `diesel::expression::ValidGrouping<()>` for `({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>)`
132132
= note: required because of the requirements on the impl of `diesel::expression::ValidGrouping<()>` for `diesel::pg::expression::array::ArrayLiteral<({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>), diesel::sql_types::Double>`
133133
= note: required because of the requirements on the impl of `diesel::query_dsl::select_dsl::SelectDsl<diesel::pg::expression::array::ArrayLiteral<({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>), diesel::sql_types::Double>>` for `diesel::query_builder::SelectStatement<()>`
@@ -143,7 +143,7 @@ error[E0277]: the trait bound `{integer}: diesel::SelectableExpression<()>` is n
143143
<(A, B) as diesel::SelectableExpression<QS>>
144144
<(A, B, C) as diesel::SelectableExpression<QS>>
145145
<(A, B, C, D) as diesel::SelectableExpression<QS>>
146-
and 108 others
146+
and 124 others
147147
= note: required because of the requirements on the impl of `diesel::SelectableExpression<()>` for `({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>)`
148148
= note: required because of the requirements on the impl of `diesel::SelectableExpression<()>` for `diesel::pg::expression::array::ArrayLiteral<({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>), diesel::sql_types::Double>`
149149
= note: required because of the requirements on the impl of `diesel::query_builder::SelectClauseExpression<()>` for `diesel::query_builder::select_clause::SelectClause<diesel::pg::expression::array::ArrayLiteral<({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>), diesel::sql_types::Double>>`
@@ -161,7 +161,7 @@ error[E0277]: the trait bound `{integer}: diesel::expression::ValidGrouping<()>`
161161
<(A, B) as diesel::expression::ValidGrouping<__GroupByClause>>
162162
<(A, B, C) as diesel::expression::ValidGrouping<__GroupByClause>>
163163
<(A, B, C, D) as diesel::expression::ValidGrouping<__GroupByClause>>
164-
and 111 others
164+
and 118 others
165165
= note: required because of the requirements on the impl of `diesel::expression::ValidGrouping<()>` for `({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>)`
166166
= note: required because of the requirements on the impl of `diesel::expression::ValidGrouping<()>` for `diesel::pg::expression::array::ArrayLiteral<({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>), diesel::sql_types::Double>`
167167
= note: required because of the requirements on the impl of `diesel::query_builder::Query` for `diesel::query_builder::SelectStatement<(), diesel::query_builder::select_clause::SelectClause<diesel::pg::expression::array::ArrayLiteral<({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>), diesel::sql_types::Double>>>`
@@ -178,7 +178,7 @@ error[E0277]: the trait bound `{integer}: diesel::query_builder::QueryId` is not
178178
<() as diesel::query_builder::QueryId>
179179
<(A, B) as diesel::query_builder::QueryId>
180180
<(A, B, C) as diesel::query_builder::QueryId>
181-
and 180 others
181+
and 185 others
182182
= note: required because of the requirements on the impl of `diesel::query_builder::QueryId` for `({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>)`
183183
= note: required because of the requirements on the impl of `diesel::query_builder::QueryId` for `diesel::pg::expression::array::ArrayLiteral<({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>), diesel::sql_types::Double>`
184184
= note: required because of the requirements on the impl of `diesel::query_builder::QueryId` for `diesel::query_builder::select_clause::SelectClause<diesel::pg::expression::array::ArrayLiteral<({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>), diesel::sql_types::Double>>`
@@ -196,7 +196,7 @@ error[E0277]: the trait bound `{integer}: diesel::query_builder::QueryFragment<_
196196
<() as diesel::query_builder::QueryFragment<DB>>
197197
<(A, B) as diesel::query_builder::QueryFragment<__DB>>
198198
<(A, B, C) as diesel::query_builder::QueryFragment<__DB>>
199-
and 211 others
199+
and 215 others
200200
= note: required because of the requirements on the impl of `diesel::query_builder::QueryFragment<_>` for `({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>)`
201201
= note: required because of the requirements on the impl of `for<'a> diesel::query_builder::QueryFragment<_>` for `&'a ({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>)`
202202
= note: required because of the requirements on the impl of `diesel::query_builder::QueryFragment<_>` for `diesel::pg::expression::array::ArrayLiteral<({integer}, diesel::expression::bound::Bound<diesel::sql_types::Double, f64>), diesel::sql_types::Double>`
@@ -220,7 +220,7 @@ error[E0277]: the trait bound `{integer}: diesel::Expression` is not satisfied
220220
<(A, B) as diesel::Expression>
221221
<(A, B, C) as diesel::Expression>
222222
<(A, B, C, D) as diesel::Expression>
223-
and 96 others
223+
and 100 others
224224
= note: required because of the requirements on the impl of `diesel::expression::AsExpression<diesel::sql_types::Double>` for `{integer}`
225225
= note: required because of the requirements on the impl of `diesel::expression::AsExpressionList<diesel::sql_types::Double>` for `({integer}, f64)`
226226

0 commit comments

Comments
 (0)