Skip to content

Commit 1b7f668

Browse files
committed
Fix panics on unknown Postgres type oid when decoding
Postgres arrays and records do not fully support custom types. When encountering an unknown OID, they currently default to using `PgTypeInfo::with_oid`. This is invalid as it breaks the invariant that decoding only uses resolved types, leading to panics. This commit returns an error instead of panicking. This is merely a mitigation: a proper fix would actually add full support for custom Postgres types. Full support involves more work, so it may still be useful to fix this immediate issue. Related issues: - #1672 - #1797
1 parent fa5c436 commit 1b7f668

File tree

4 files changed

+166
-5
lines changed

4 files changed

+166
-5
lines changed

sqlx-core/src/postgres/types/array.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,12 @@ where
134134
let element_type_oid = Oid(buf.get_u32());
135135
let element_type_info: PgTypeInfo = PgTypeInfo::try_from_oid(element_type_oid)
136136
.or_else(|| value.type_info.try_array_element().map(Cow::into_owned))
137-
.unwrap_or_else(|| PgTypeInfo(PgType::DeclareWithOid(element_type_oid)));
137+
.ok_or_else(|| {
138+
BoxDynError::from(format!(
139+
"failed to resolve array element type for oid {}",
140+
element_type_oid.0
141+
))
142+
})?;
138143

139144
// length of the array axis
140145
let len = buf.get_i32();

sqlx-core/src/postgres/types/record.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -125,16 +125,17 @@ impl<'r> PgRecordDecoder<'r> {
125125
}
126126
};
127127

128-
self.ind += 1;
129-
130128
if let Some(ty) = &element_type_opt {
131129
if !ty.is_null() && !T::compatible(ty) {
132130
return Err(mismatched_types::<Postgres, T>(ty));
133131
}
134132
}
135133

136134
let element_type =
137-
element_type_opt.unwrap_or_else(|| PgTypeInfo::with_oid(element_type_oid));
135+
element_type_opt
136+
.ok_or_else(|| BoxDynError::from(format!("custom types in records are not fully supported yet: failed to retrieve type info for field {} with type oid {}", self.ind, element_type_oid.0)))?;
137+
138+
self.ind += 1;
138139

139140
T::decode(PgValueRef::get(&mut self.buf, self.fmt, element_type))
140141
}

sqlx-core/src/query_builder.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ where
224224
/// // e.g. collect it to a `Vec` first.
225225
/// b.push_bind(user.id)
226226
/// .push_bind(user.username)
227-
/// .push_bind(user.email)
227+
/// .push_bind(user.email)
228228
/// .push_bind(user.password);
229229
/// });
230230
///
@@ -310,6 +310,7 @@ where
310310
/// A wrapper around `QueryBuilder` for creating comma(or other token)-separated lists.
311311
///
312312
/// See [`QueryBuilder::separated()`] for details.
313+
#[allow(explicit_outlives_requirements)]
313314
pub struct Separated<'qb, 'args: 'qb, DB, Sep>
314315
where
315316
DB: Database,

tests/postgres/postgres.rs

+154
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use sqlx::postgres::{
55
PgPoolOptions, PgRow, PgSeverity, Postgres,
66
};
77
use sqlx::{Column, Connection, Executor, Row, Statement, TypeInfo};
8+
use sqlx_core::error::{BoxDynError, Error};
89
use sqlx_test::{new, pool, setup_if_needed};
910
use std::env;
1011
use std::sync::Arc;
@@ -1200,6 +1201,159 @@ VALUES
12001201
Ok(())
12011202
}
12021203

1204+
#[sqlx_macros::test]
1205+
async fn it_resolves_custom_types_in_anonymous_records() -> anyhow::Result<()> {
1206+
// This request involves nested records and array types.
1207+
1208+
// Only supported in Postgres 11+
1209+
let mut conn = new::<Postgres>().await?;
1210+
if matches!(conn.server_version_num(), Some(version) if version < 110000) {
1211+
return Ok(());
1212+
}
1213+
1214+
// language=PostgreSQL
1215+
conn.execute(
1216+
r#"
1217+
DROP TABLE IF EXISTS repo_users;
1218+
DROP TABLE IF EXISTS repositories;
1219+
DROP TABLE IF EXISTS repo_memberships;
1220+
DROP TYPE IF EXISTS repo_member;
1221+
1222+
CREATE TABLE repo_users (
1223+
user_id INT4 NOT NULL,
1224+
username TEXT NOT NULL,
1225+
PRIMARY KEY (user_id)
1226+
);
1227+
CREATE TABLE repositories (
1228+
repo_id INT4 NOT NULL,
1229+
repo_name TEXT NOT NULL,
1230+
PRIMARY KEY (repo_id)
1231+
);
1232+
CREATE TABLE repo_memberships (
1233+
repo_id INT4 NOT NULL,
1234+
user_id INT4 NOT NULL,
1235+
permission TEXT NOT NULL,
1236+
PRIMARY KEY (repo_id, user_id)
1237+
);
1238+
CREATE TYPE repo_member AS (
1239+
user_id INT4,
1240+
permission TEXT
1241+
);
1242+
INSERT INTO repo_users(user_id, username)
1243+
VALUES
1244+
(101, 'alice'),
1245+
(102, 'bob'),
1246+
(103, 'charlie');
1247+
INSERT INTO repositories(repo_id, repo_name)
1248+
VALUES
1249+
(201, 'rust'),
1250+
(202, 'sqlx'),
1251+
(203, 'hello-world');
1252+
INSERT INTO repo_memberships(repo_id, user_id, permission)
1253+
VALUES
1254+
(201, 101, 'admin'),
1255+
(201, 102, 'write'),
1256+
(201, 103, 'read'),
1257+
(202, 102, 'admin');
1258+
"#,
1259+
)
1260+
.await?;
1261+
1262+
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
1263+
struct RepoMember {
1264+
user_id: i32,
1265+
permission: String,
1266+
}
1267+
1268+
impl sqlx::Type<Postgres> for RepoMember {
1269+
fn type_info() -> sqlx::postgres::PgTypeInfo {
1270+
sqlx::postgres::PgTypeInfo::with_name("repo_member")
1271+
}
1272+
}
1273+
1274+
impl<'r> sqlx::Decode<'r, Postgres> for RepoMember {
1275+
fn decode(
1276+
value: sqlx::postgres::PgValueRef<'r>,
1277+
) -> Result<Self, Box<dyn std::error::Error + 'static + Send + Sync>> {
1278+
let mut decoder = sqlx::postgres::types::PgRecordDecoder::new(value)?;
1279+
let user_id = decoder.try_decode::<i32>()?;
1280+
let permission = decoder.try_decode::<String>()?;
1281+
Ok(Self {
1282+
user_id,
1283+
permission,
1284+
})
1285+
}
1286+
}
1287+
1288+
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
1289+
struct RepoMemberArray(Vec<RepoMember>);
1290+
1291+
impl sqlx::Type<Postgres> for RepoMemberArray {
1292+
fn type_info() -> sqlx::postgres::PgTypeInfo {
1293+
// Array type name is the name of the element type prefixed with `_`
1294+
sqlx::postgres::PgTypeInfo::with_name("_repo_member")
1295+
}
1296+
}
1297+
1298+
impl<'r> sqlx::Decode<'r, Postgres> for RepoMemberArray {
1299+
fn decode(
1300+
value: sqlx::postgres::PgValueRef<'r>,
1301+
) -> Result<Self, Box<dyn std::error::Error + 'static + Send + Sync>> {
1302+
Ok(Self(Vec::<RepoMember>::decode(value)?))
1303+
}
1304+
}
1305+
1306+
let mut conn = new::<Postgres>().await?;
1307+
1308+
#[derive(Debug, sqlx::FromRow)]
1309+
struct Row {
1310+
count: i64,
1311+
items: Vec<(i32, String, RepoMemberArray)>,
1312+
}
1313+
// language=PostgreSQL
1314+
let row: Result<Row, Error> = sqlx::query_as::<_, Row>(
1315+
r"
1316+
WITH
1317+
members_by_repo AS (
1318+
SELECT repo_id,
1319+
ARRAY_AGG(ROW (user_id, permission)::repo_member) AS members
1320+
FROM repo_memberships
1321+
GROUP BY repo_id
1322+
),
1323+
repos AS (
1324+
SELECT repo_id, repo_name, COALESCE(members, '{}') AS members
1325+
FROM repositories
1326+
LEFT OUTER JOIN members_by_repo USING (repo_id)
1327+
ORDER BY repo_id
1328+
),
1329+
repo_array AS (
1330+
SELECT COALESCE(ARRAY_AGG(repos.*), '{}') AS items
1331+
FROM repos
1332+
),
1333+
repo_count AS (
1334+
SELECT COUNT(*) AS count
1335+
FROM repos
1336+
)
1337+
SELECT count, items
1338+
FROM repo_count, repo_array
1339+
;
1340+
",
1341+
)
1342+
.fetch_one(&mut conn)
1343+
.await;
1344+
1345+
// This test currently tests mitigations for `#1672` (use regular errors
1346+
// instead of panics). Once we fully support custom types, it should be
1347+
// updated accordingly.
1348+
match row {
1349+
Ok(_) => panic!("full support for custom types is not implemented yet"),
1350+
Err(e) => assert!(e
1351+
.to_string()
1352+
.contains("custom types in records are not fully supported yet")),
1353+
}
1354+
Ok(())
1355+
}
1356+
12031357
#[sqlx_macros::test]
12041358
async fn test_pg_server_num() -> anyhow::Result<()> {
12051359
let conn = new::<Postgres>().await?;

0 commit comments

Comments
 (0)