From 6049f976f9ee23ccaa429e50608c0f069f37639e Mon Sep 17 00:00:00 2001 From: Ryan Leckey Date: Wed, 25 Mar 2020 04:25:38 -0700 Subject: [PATCH] opt out of compatible type check for null values --- .github/workflows/rust.yml | 16 +++++++------- sqlx-core/src/mysql/row.rs | 2 +- sqlx-core/src/mysql/value.rs | 12 +++++------ sqlx-core/src/postgres/types/json.rs | 4 ++-- sqlx-core/src/postgres/value.rs | 8 +++++-- sqlx-core/src/row.rs | 11 +++++++--- sqlx-core/src/sqlite/types/mod.rs | 2 -- sqlx-core/src/sqlite/value.rs | 22 ++++++++++---------- sqlx-core/src/value.rs | 2 +- tests/postgres.rs | 31 ++++++++++++++-------------- 10 files changed, 58 insertions(+), 52 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index df5d32901..b18a8ed3e 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -121,42 +121,42 @@ jobs: # ----------------------------------------------------- # integration test: async-std (chrono) - - run: cargo test --no-default-features --features 'runtime-async-std postgres macros uuid chrono bigdecimal ipnetwork tls' + - run: cargo test --no-default-features --features 'runtime-async-std postgres macros uuid chrono bigdecimal json ipnetwork tls' env: DATABASE_URL: postgres://postgres:postgres@localhost:${{ job.services.postgres.ports[5432] }}/postgres # integration test: async-std (time) - - run: cargo test --no-default-features --features 'runtime-async-std postgres macros uuid time bigdecimal ipnetwork tls' + - run: cargo test --no-default-features --features 'runtime-async-std postgres macros uuid time bigdecimal json ipnetwork tls' env: DATABASE_URL: postgres://postgres:postgres@localhost:${{ job.services.postgres.ports[5432] }}/postgres # integration test: async-std (time + chrono) - - run: cargo test --no-default-features --features 'runtime-async-std postgres macros uuid chrono time bigdecimal ipnetwork tls' + - run: cargo test --no-default-features --features 'runtime-async-std postgres macros uuid chrono time bigdecimal json ipnetwork tls' env: DATABASE_URL: postgres://postgres:postgres@localhost:${{ job.services.postgres.ports[5432] }}/postgres # integration test: tokio (chrono) - - run: cargo test --no-default-features --features 'runtime-tokio postgres macros uuid chrono bigdecimal ipnetwork tls' + - run: cargo test --no-default-features --features 'runtime-tokio postgres macros uuid chrono bigdecimal json ipnetwork tls' env: DATABASE_URL: postgres://postgres:postgres@localhost:${{ job.services.postgres.ports[5432] }}/postgres # integration test: tokio (time) - - run: cargo test --no-default-features --features 'runtime-tokio postgres macros uuid time bigdecimal ipnetwork tls' + - run: cargo test --no-default-features --features 'runtime-tokio postgres macros uuid time bigdecimal json ipnetwork tls' env: DATABASE_URL: postgres://postgres:postgres@localhost:${{ job.services.postgres.ports[5432] }}/postgres # integration test: tokio (time + chrono) - - run: cargo test --no-default-features --features 'runtime-tokio postgres macros uuid chrono time bigdecimal ipnetwork tls' + - run: cargo test --no-default-features --features 'runtime-tokio postgres macros uuid chrono time bigdecimal json ipnetwork tls' env: DATABASE_URL: postgres://postgres:postgres@localhost:${{ job.services.postgres.ports[5432] }}/postgres # UI feature gate tests: async-std - - run: cargo test --no-default-features --features 'runtime-async-std postgres macros bigdecimal ipnetwork tls' + - run: cargo test --no-default-features --features 'runtime-async-std postgres macros bigdecimal json ipnetwork tls' env: DATABASE_URL: postgres://postgres:postgres@localhost:${{ job.services.postgres.ports[5432] }}/postgres # UI feature gate tests: tokio - - run: cargo test --no-default-features --features 'runtime-tokio postgres macros bigdecimal ipnetwork tls' + - run: cargo test --no-default-features --features 'runtime-tokio postgres macros bigdecimal json ipnetwork tls' env: DATABASE_URL: postgres://postgres:postgres@localhost:${{ job.services.postgres.ports[5432] }}/postgres diff --git a/sqlx-core/src/mysql/row.rs b/sqlx-core/src/mysql/row.rs index 3a376663c..fa9c4583c 100644 --- a/sqlx-core/src/mysql/row.rs +++ b/sqlx-core/src/mysql/row.rs @@ -28,7 +28,7 @@ impl<'c> Row<'c> for MySqlRow<'c> { let column_ty = self.row.columns[index].clone(); let buffer = self.row.get(index); let value = match (self.row.binary, buffer) { - (_, None) => MySqlValue::null(column_ty), + (_, None) => MySqlValue::null(), (true, Some(buf)) => MySqlValue::binary(column_ty, buf), (false, Some(buf)) => MySqlValue::text(column_ty, buf), }; diff --git a/sqlx-core/src/mysql/value.rs b/sqlx-core/src/mysql/value.rs index c1148d6ba..7d92eaa12 100644 --- a/sqlx-core/src/mysql/value.rs +++ b/sqlx-core/src/mysql/value.rs @@ -10,7 +10,7 @@ pub enum MySqlData<'c> { #[derive(Debug)] pub struct MySqlValue<'c> { - type_info: MySqlTypeInfo, + type_info: Option, data: Option>, } @@ -31,23 +31,23 @@ impl<'c> MySqlValue<'c> { self.data } - pub(crate) fn null(type_info: MySqlTypeInfo) -> Self { + pub(crate) fn null() -> Self { Self { - type_info, + type_info: None, data: None, } } pub(crate) fn binary(type_info: MySqlTypeInfo, buf: &'c [u8]) -> Self { Self { - type_info, + type_info: Some(type_info), data: Some(MySqlData::Binary(buf)), } } pub(crate) fn text(type_info: MySqlTypeInfo, buf: &'c [u8]) -> Self { Self { - type_info, + type_info: Some(type_info), data: Some(MySqlData::Text(buf)), } } @@ -56,7 +56,7 @@ impl<'c> MySqlValue<'c> { impl<'c> RawValue<'c> for MySqlValue<'c> { type Database = MySql; - fn type_info(&self) -> MySqlTypeInfo { + fn type_info(&self) -> Option { self.type_info.clone() } } diff --git a/sqlx-core/src/postgres/types/json.rs b/sqlx-core/src/postgres/types/json.rs index 27a322d5a..2cdff3242 100644 --- a/sqlx-core/src/postgres/types/json.rs +++ b/sqlx-core/src/postgres/types/json.rs @@ -62,7 +62,7 @@ where T: Serialize, { fn encode(&self, buf: &mut Vec) { - // JSONB version (as of 2020-03-20) + // JSONB version (as of 2020-03-20 ) buf.put_u8(1); serde_json::to_writer(buf, &self.0) @@ -79,7 +79,7 @@ where (match value.try_get()? { PgData::Text(s) => serde_json::from_str(s), PgData::Binary(mut buf) => { - if value.type_info().id == TypeId::JSONB { + if value.type_info().as_ref().map(|info| info.id) == Some(TypeId::JSONB) { let version = buf.get_u8()?; assert_eq!( diff --git a/sqlx-core/src/postgres/value.rs b/sqlx-core/src/postgres/value.rs index a9926de2c..a1fa4e6c8 100644 --- a/sqlx-core/src/postgres/value.rs +++ b/sqlx-core/src/postgres/value.rs @@ -65,7 +65,11 @@ impl<'c> PgValue<'c> { impl<'c> RawValue<'c> for PgValue<'c> { type Database = Postgres; - fn type_info(&self) -> PgTypeInfo { - PgTypeInfo::with_oid(self.type_id.0) + fn type_info(&self) -> Option { + if self.data.is_some() { + Some(PgTypeInfo::with_oid(self.type_id.0)) + } else { + None + } } } diff --git a/sqlx-core/src/row.rs b/sqlx-core/src/row.rs index f3ccec802..91c9cb3d9 100644 --- a/sqlx-core/src/row.rs +++ b/sqlx-core/src/row.rs @@ -134,10 +134,15 @@ where T: Decode<'c, Self::Database>, { let value = self.try_get_raw(index)?; - let expected_ty = value.type_info(); - if !expected_ty.compatible(&T::type_info()) { - return Err(crate::Error::mismatched_types::(expected_ty)); + if let Some(expected_ty) = value.type_info() { + // NOTE: If there is no type, the value is NULL. This is fine. If the user tries + // to get this into a non-Option we catch that elsewhere and report as + // UnexpectedNullError. + + if !expected_ty.compatible(&T::type_info()) { + return Err(crate::Error::mismatched_types::(expected_ty)); + } } T::decode(value) diff --git a/sqlx-core/src/sqlite/types/mod.rs b/sqlx-core/src/sqlite/types/mod.rs index 79fadebe8..f505d3091 100644 --- a/sqlx-core/src/sqlite/types/mod.rs +++ b/sqlx-core/src/sqlite/types/mod.rs @@ -35,7 +35,6 @@ mod str; // https://www.sqlite.org/c3ref/c_blob.html #[derive(Debug, PartialEq, Clone, Copy)] pub(crate) enum SqliteType { - Null = 0, Integer = 1, Float = 2, Text = 3, @@ -78,7 +77,6 @@ impl Display for SqliteTypeInfo { SqliteType::Integer => "INTEGER", SqliteType::Float => "DOUBLE", SqliteType::Blob => "BLOB", - SqliteType::Null => "NULL", }) } } diff --git a/sqlx-core/src/sqlite/value.rs b/sqlx-core/src/sqlite/value.rs index 9357fb9a8..c8b1959ca 100644 --- a/sqlx-core/src/sqlite/value.rs +++ b/sqlx-core/src/sqlite/value.rs @@ -27,20 +27,20 @@ pub struct SqliteValue<'c> { impl<'c> SqliteValue<'c> { /// Returns true if the value should be intrepreted as NULL. pub(super) fn is_null(&self) -> bool { - self.r#type() == SqliteType::Null + self.r#type().is_none() } - fn r#type(&self) -> SqliteType { + fn r#type(&self) -> Option { #[allow(unsafe_code)] let type_code = unsafe { sqlite3_column_type(self.statement.handle(), self.index) }; // SQLITE_INTEGER, SQLITE_FLOAT, SQLITE_TEXT, SQLITE_BLOB, or SQLITE_NULL match type_code { - SQLITE_INTEGER => SqliteType::Integer, - SQLITE_FLOAT => SqliteType::Float, - SQLITE_TEXT => SqliteType::Text, - SQLITE_BLOB => SqliteType::Blob, - SQLITE_NULL => SqliteType::Null, + SQLITE_INTEGER => Some(SqliteType::Integer), + SQLITE_FLOAT => Some(SqliteType::Float), + SQLITE_TEXT => Some(SqliteType::Text), + SQLITE_BLOB => Some(SqliteType::Blob), + SQLITE_NULL => None, _ => unreachable!("received unexpected column type: {}", type_code), } @@ -111,10 +111,10 @@ impl<'c> SqliteValue<'c> { impl<'c> RawValue<'c> for SqliteValue<'c> { type Database = Sqlite; - fn type_info(&self) -> SqliteTypeInfo { - SqliteTypeInfo { - r#type: self.r#type(), + fn type_info(&self) -> Option { + Some(SqliteTypeInfo { + r#type: self.r#type()?, affinity: None, - } + }) } } diff --git a/sqlx-core/src/value.rs b/sqlx-core/src/value.rs index b757d31b2..70861776b 100644 --- a/sqlx-core/src/value.rs +++ b/sqlx-core/src/value.rs @@ -19,5 +19,5 @@ pub trait HasRawValue<'c> { pub trait RawValue<'c> { type Database: Database; - fn type_info(&self) -> ::TypeInfo; + fn type_info(&self) -> Option<::TypeInfo>; } diff --git a/tests/postgres.rs b/tests/postgres.rs index 214a5ef2d..7ee85b420 100644 --- a/tests/postgres.rs +++ b/tests/postgres.rs @@ -60,22 +60,21 @@ CREATE TEMPORARY TABLE users (id INTEGER PRIMARY KEY); async fn it_can_return_interleaved_nulls_issue_104() -> anyhow::Result<()> { let mut conn = new::().await?; - let tuple = - sqlx::query("SELECT NULL::INT, 10::INT, NULL, 20::INT, NULL, 40::INT, NULL, 80::INT") - .try_map(|row: PgRow| { - Ok(( - row.get::, _>(0), - row.get::, _>(1), - row.get::, _>(2), - row.get::, _>(3), - row.get::, _>(4), - row.get::, _>(5), - row.get::, _>(6), - row.get::, _>(7), - )) - }) - .fetch_one(&mut conn) - .await?; + let tuple = sqlx::query("SELECT NULL, 10::INT, NULL, 20::INT, NULL, 40::INT, NULL, 80::INT") + .try_map(|row: PgRow| { + Ok(( + row.get::, _>(0), + row.get::, _>(1), + row.get::, _>(2), + row.get::, _>(3), + row.get::, _>(4), + row.get::, _>(5), + row.get::, _>(6), + row.get::, _>(7), + )) + }) + .fetch_one(&mut conn) + .await?; assert_eq!(tuple.0, None); assert_eq!(tuple.1, Some(10));