From 25efb2f7f410e0f0aa3fee1d8467429066dbcdf8 Mon Sep 17 00:00:00 2001 From: Austin Bonander Date: Sat, 13 Apr 2024 16:59:13 -0700 Subject: [PATCH] breaking(sqlite): always use `i64` as intermediate when decoding (#3184) Breaking changes: * integer decoding will now loudly error on overflow instead of silently truncating. * some usages of the query!() macros might change an i32 to an i64. Also adds support for *decoding* `u64`s because there's no reason not to. Closes #3179 --- sqlx-sqlite/src/any.rs | 4 +-- sqlx-sqlite/src/connection/explain.rs | 41 +++++++++++++------------- sqlx-sqlite/src/type_checking.rs | 5 ++++ sqlx-sqlite/src/type_info.rs | 42 ++++++++++++++++----------- sqlx-sqlite/src/types/bool.rs | 4 +-- sqlx-sqlite/src/types/chrono.rs | 8 +++-- sqlx-sqlite/src/types/int.rs | 26 ++++++++++------- sqlx-sqlite/src/types/mod.rs | 19 +++++++----- sqlx-sqlite/src/types/time.rs | 6 ++-- sqlx-sqlite/src/types/uint.rs | 36 ++++++++++++++++++----- sqlx-sqlite/src/value.rs | 17 ++++------- tests/sqlite/any.rs | 17 +++++++++++ tests/sqlite/macros.rs | 8 ++--- 13 files changed, 145 insertions(+), 88 deletions(-) diff --git a/sqlx-sqlite/src/any.rs b/sqlx-sqlite/src/any.rs index dc84f6659..ea0f320e2 100644 --- a/sqlx-sqlite/src/any.rs +++ b/sqlx-sqlite/src/any.rs @@ -141,8 +141,8 @@ impl<'a> TryFrom<&'a SqliteTypeInfo> for AnyTypeInfo { Ok(AnyTypeInfo { kind: match &sqlite_type.0 { DataType::Null => AnyTypeInfoKind::Null, - DataType::Int => AnyTypeInfoKind::Integer, - DataType::Int64 => AnyTypeInfoKind::BigInt, + DataType::Int4 => AnyTypeInfoKind::Integer, + DataType::Integer => AnyTypeInfoKind::BigInt, DataType::Float => AnyTypeInfoKind::Double, DataType::Blob => AnyTypeInfoKind::Blob, DataType::Text => AnyTypeInfoKind::Text, diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index cb7ee62c6..7929ffb21 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -181,7 +181,7 @@ impl RegDataType { fn map_to_datatype(&self) -> DataType { match self { RegDataType::Single(d) => d.map_to_datatype(), - RegDataType::Int(_) => DataType::Int, + RegDataType::Int(_) => DataType::Integer, } } fn map_to_nullable(&self) -> Option { @@ -194,7 +194,7 @@ impl RegDataType { match self { RegDataType::Single(d) => d.clone(), RegDataType::Int(_) => ColumnType::Single { - datatype: DataType::Int, + datatype: DataType::Integer, nullable: Some(false), }, } @@ -311,7 +311,7 @@ impl CursorDataType { fn affinity_to_type(affinity: u8) -> DataType { match affinity { SQLITE_AFF_BLOB => DataType::Blob, - SQLITE_AFF_INTEGER => DataType::Int64, + SQLITE_AFF_INTEGER => DataType::Integer, SQLITE_AFF_NUMERIC => DataType::Numeric, SQLITE_AFF_REAL => DataType::Float, SQLITE_AFF_TEXT => DataType::Text, @@ -326,7 +326,7 @@ fn opcode_to_type(op: &str) -> DataType { OP_REAL => DataType::Float, OP_BLOB => DataType::Blob, OP_AND | OP_OR => DataType::Bool, - OP_ROWID | OP_COUNT | OP_INT64 | OP_INTEGER => DataType::Int64, + OP_ROWID | OP_COUNT | OP_INT64 | OP_INTEGER => DataType::Integer, OP_STRING8 => DataType::Text, OP_COLUMN | _ => DataType::Null, } @@ -649,7 +649,7 @@ pub(super) fn explain( state.mem.r.insert( p1, RegDataType::Single(ColumnType::Single { - datatype: DataType::Int64, + datatype: DataType::Integer, nullable: Some(false), }), ); @@ -843,7 +843,7 @@ pub(super) fn explain( state.mem.r.insert( p2, RegDataType::Single(ColumnType::Single { - datatype: DataType::Int64, + datatype: DataType::Integer, nullable: Some(false), }), ); @@ -999,7 +999,7 @@ pub(super) fn explain( state.mem.r.insert( p3, RegDataType::Single(ColumnType::Single { - datatype: DataType::Int64, + datatype: DataType::Integer, nullable: Some(false), }), ); @@ -1029,7 +1029,7 @@ pub(super) fn explain( state.mem.r.insert( p3, RegDataType::Single(ColumnType::Single { - datatype: DataType::Int64, + datatype: DataType::Integer, nullable: Some(p2 != 0), //never a null result if no argument provided }), ); @@ -1073,7 +1073,7 @@ pub(super) fn explain( state.mem.r.insert( p3, RegDataType::Single(ColumnType::Single { - datatype: DataType::Int64, + datatype: DataType::Integer, nullable: Some(false), }), ); @@ -1089,9 +1089,10 @@ pub(super) fn explain( } else if p4.starts_with("sum(") { if let Some(r_p2) = state.mem.r.get(&p2) { let datatype = match r_p2.map_to_datatype() { - DataType::Int64 => DataType::Int64, - DataType::Int => DataType::Int, - DataType::Bool => DataType::Int, + // The result of a `SUM()` can be arbitrarily large + DataType::Integer | DataType::Int4 | DataType::Bool => { + DataType::Integer + } _ => DataType::Float, }; let nullable = r_p2.map_to_nullable(); @@ -1130,7 +1131,7 @@ pub(super) fn explain( state.mem.r.insert( p1, RegDataType::Single(ColumnType::Single { - datatype: DataType::Int64, + datatype: DataType::Integer, nullable: Some(false), }), ); @@ -1275,7 +1276,7 @@ pub(super) fn explain( state.mem.r.insert( p2, RegDataType::Single(ColumnType::Single { - datatype: DataType::Int64, + datatype: DataType::Integer, nullable: Some(false), }), ); @@ -1471,7 +1472,7 @@ fn test_root_block_columns_has_types() { let table_db_block = table_block_nums["t"]; assert_eq!( ColumnType::Single { - datatype: DataType::Int64, + datatype: DataType::Integer, nullable: Some(true) //sqlite primary key columns are nullable unless declared not null }, root_block_cols[&table_db_block][&0] @@ -1496,7 +1497,7 @@ fn test_root_block_columns_has_types() { let table_db_block = table_block_nums["i1"]; assert_eq!( ColumnType::Single { - datatype: DataType::Int64, + datatype: DataType::Integer, nullable: Some(true) //sqlite primary key columns are nullable unless declared not null }, root_block_cols[&table_db_block][&0] @@ -1514,7 +1515,7 @@ fn test_root_block_columns_has_types() { let table_db_block = table_block_nums["i2"]; assert_eq!( ColumnType::Single { - datatype: DataType::Int64, + datatype: DataType::Integer, nullable: Some(true) //sqlite primary key columns are nullable unless declared not null }, root_block_cols[&table_db_block][&0] @@ -1532,7 +1533,7 @@ fn test_root_block_columns_has_types() { let table_db_block = table_block_nums["t2"]; assert_eq!( ColumnType::Single { - datatype: DataType::Int64, + datatype: DataType::Integer, nullable: Some(false) }, root_block_cols[&table_db_block][&0] @@ -1557,7 +1558,7 @@ fn test_root_block_columns_has_types() { let table_db_block = table_block_nums["t2i1"]; assert_eq!( ColumnType::Single { - datatype: DataType::Int64, + datatype: DataType::Integer, nullable: Some(false) }, root_block_cols[&table_db_block][&0] @@ -1575,7 +1576,7 @@ fn test_root_block_columns_has_types() { let table_db_block = table_block_nums["t2i2"]; assert_eq!( ColumnType::Single { - datatype: DataType::Int64, + datatype: DataType::Integer, nullable: Some(false) }, root_block_cols[&table_db_block][&0] diff --git a/sqlx-sqlite/src/type_checking.rs b/sqlx-sqlite/src/type_checking.rs index e1a86e241..e1ac3bc75 100644 --- a/sqlx-sqlite/src/type_checking.rs +++ b/sqlx-sqlite/src/type_checking.rs @@ -8,7 +8,12 @@ use crate::Sqlite; // For more info see: https://www.sqlite.org/datatype3.html#storage_classes_and_datatypes impl_type_checking!( Sqlite { + // Note that since the macro checks `column_type_info == ::type_info()` first, + // we can list `bool` without it being automatically picked for all integer types + // due to its `TypeInfo::compatible()` impl. bool, + // Since it returns `DataType::Int4` for `type_info()`, + // `i32` should only be chosen IFF the column decltype is `INT4` i32, i64, f64, diff --git a/sqlx-sqlite/src/type_info.rs b/sqlx-sqlite/src/type_info.rs index 9c95b5146..38a0fc416 100644 --- a/sqlx-sqlite/src/type_info.rs +++ b/sqlx-sqlite/src/type_info.rs @@ -11,19 +11,27 @@ pub(crate) use sqlx_core::type_info::*; #[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] #[cfg_attr(feature = "offline", derive(serde::Serialize, serde::Deserialize))] pub(crate) enum DataType { + // These variants should correspond to `SQLITE_*` type constants. Null, - Int, + /// Note: SQLite's type system has no notion of integer widths. + /// The `INTEGER` type affinity can store up to 8 byte integers, + /// making `i64` the only safe choice when mapping integer types to Rust. + Integer, Float, Text, Blob, - // TODO: Support NUMERIC + // Explicitly not supported: see documentation in `types/mod.rs` #[allow(dead_code)] Numeric, - // non-standard extensions + // non-standard extensions (chosen based on the column's declared type) + /// Chosen if the column's declared type is `BOOLEAN`. Bool, - Int64, + /// Chosen if the column's declared type is `INT4`; + /// instructs the macros to use `i32` instead of `i64`. + /// Legacy feature; no idea if this is actually used anywhere. + Int4, Date, Time, Datetime, @@ -51,7 +59,7 @@ impl TypeInfo for SqliteTypeInfo { DataType::Text => "TEXT", DataType::Float => "REAL", DataType::Blob => "BLOB", - DataType::Int | DataType::Int64 => "INTEGER", + DataType::Int4 | DataType::Integer => "INTEGER", DataType::Numeric => "NUMERIC", // non-standard extensions @@ -66,7 +74,7 @@ impl TypeInfo for SqliteTypeInfo { impl DataType { pub(crate) fn from_code(code: c_int) -> Self { match code { - SQLITE_INTEGER => DataType::Int, + SQLITE_INTEGER => DataType::Integer, SQLITE_FLOAT => DataType::Float, SQLITE_BLOB => DataType::Blob, SQLITE_NULL => DataType::Null, @@ -87,15 +95,15 @@ impl FromStr for DataType { fn from_str(s: &str) -> Result { let s = s.to_ascii_lowercase(); Ok(match &*s { - "int4" => DataType::Int, - "int8" => DataType::Int64, + "int4" => DataType::Int4, + "int8" => DataType::Integer, "boolean" | "bool" => DataType::Bool, "date" => DataType::Date, "time" => DataType::Time, "datetime" | "timestamp" => DataType::Datetime, - _ if s.contains("int") => DataType::Int64, + _ if s.contains("int") => DataType::Integer, _ if s.contains("char") || s.contains("clob") || s.contains("text") => DataType::Text, @@ -120,16 +128,16 @@ impl FromStr for DataType { #[test] fn test_data_type_from_str() -> Result<(), BoxDynError> { - assert_eq!(DataType::Int, "INT4".parse()?); + assert_eq!(DataType::Int4, "INT4".parse()?); - assert_eq!(DataType::Int64, "INT".parse()?); - assert_eq!(DataType::Int64, "INTEGER".parse()?); - assert_eq!(DataType::Int64, "INTBIG".parse()?); - assert_eq!(DataType::Int64, "MEDIUMINT".parse()?); + assert_eq!(DataType::Integer, "INT".parse()?); + assert_eq!(DataType::Integer, "INTEGER".parse()?); + assert_eq!(DataType::Integer, "INTBIG".parse()?); + assert_eq!(DataType::Integer, "MEDIUMINT".parse()?); - assert_eq!(DataType::Int64, "BIGINT".parse()?); - assert_eq!(DataType::Int64, "UNSIGNED BIG INT".parse()?); - assert_eq!(DataType::Int64, "INT8".parse()?); + assert_eq!(DataType::Integer, "BIGINT".parse()?); + assert_eq!(DataType::Integer, "UNSIGNED BIG INT".parse()?); + assert_eq!(DataType::Integer, "INT8".parse()?); assert_eq!(DataType::Text, "CHARACTER(20)".parse()?); assert_eq!(DataType::Text, "NCHAR(55)".parse()?); diff --git a/sqlx-sqlite/src/types/bool.rs b/sqlx-sqlite/src/types/bool.rs index 356ea6360..ce20ab787 100644 --- a/sqlx-sqlite/src/types/bool.rs +++ b/sqlx-sqlite/src/types/bool.rs @@ -11,7 +11,7 @@ impl Type for bool { } fn compatible(ty: &SqliteTypeInfo) -> bool { - matches!(ty.0, DataType::Bool | DataType::Int | DataType::Int64) + matches!(ty.0, DataType::Bool | DataType::Int4 | DataType::Integer) } } @@ -25,6 +25,6 @@ impl<'q> Encode<'q, Sqlite> for bool { impl<'r> Decode<'r, Sqlite> for bool { fn decode(value: SqliteValueRef<'r>) -> Result { - Ok(value.int() != 0) + Ok(value.int64() != 0) } } diff --git a/sqlx-sqlite/src/types/chrono.rs b/sqlx-sqlite/src/types/chrono.rs index 0782f8a5d..3e5aa93f3 100644 --- a/sqlx-sqlite/src/types/chrono.rs +++ b/sqlx-sqlite/src/types/chrono.rs @@ -32,7 +32,11 @@ impl Type for NaiveDateTime { fn compatible(ty: &SqliteTypeInfo) -> bool { matches!( ty.0, - DataType::Datetime | DataType::Text | DataType::Int64 | DataType::Int | DataType::Float + DataType::Datetime + | DataType::Text + | DataType::Integer + | DataType::Int4 + | DataType::Float ) } } @@ -105,7 +109,7 @@ impl<'r> Decode<'r, Sqlite> for DateTime { fn decode_datetime(value: SqliteValueRef<'_>) -> Result, BoxDynError> { let dt = match value.type_info().0 { DataType::Text => decode_datetime_from_text(value.text()?), - DataType::Int | DataType::Int64 => decode_datetime_from_int(value.int64()), + DataType::Int4 | DataType::Integer => decode_datetime_from_int(value.int64()), DataType::Float => decode_datetime_from_float(value.double()), _ => None, diff --git a/sqlx-sqlite/src/types/int.rs b/sqlx-sqlite/src/types/int.rs index f5ec40aa8..6176d9dea 100644 --- a/sqlx-sqlite/src/types/int.rs +++ b/sqlx-sqlite/src/types/int.rs @@ -7,11 +7,11 @@ use crate::{Sqlite, SqliteArgumentValue, SqliteTypeInfo, SqliteValueRef}; impl Type for i8 { fn type_info() -> SqliteTypeInfo { - SqliteTypeInfo(DataType::Int) + SqliteTypeInfo(DataType::Int4) } fn compatible(ty: &SqliteTypeInfo) -> bool { - matches!(ty.0, DataType::Int | DataType::Int64) + matches!(ty.0, DataType::Int4 | DataType::Integer) } } @@ -25,17 +25,21 @@ impl<'q> Encode<'q, Sqlite> for i8 { impl<'r> Decode<'r, Sqlite> for i8 { fn decode(value: SqliteValueRef<'r>) -> Result { - Ok(value.int().try_into()?) + // NOTE: using `sqlite3_value_int64()` here because `sqlite3_value_int()` silently truncates + // which leads to bugs, e.g.: + // https://github.com/launchbadge/sqlx/issues/3179 + // Similar bug in Postgres: https://github.com/launchbadge/sqlx/issues/3161 + Ok(value.int64().try_into()?) } } impl Type for i16 { fn type_info() -> SqliteTypeInfo { - SqliteTypeInfo(DataType::Int) + SqliteTypeInfo(DataType::Int4) } fn compatible(ty: &SqliteTypeInfo) -> bool { - matches!(ty.0, DataType::Int | DataType::Int64) + matches!(ty.0, DataType::Int4 | DataType::Integer) } } @@ -49,17 +53,17 @@ impl<'q> Encode<'q, Sqlite> for i16 { impl<'r> Decode<'r, Sqlite> for i16 { fn decode(value: SqliteValueRef<'r>) -> Result { - Ok(value.int().try_into()?) + Ok(value.int64().try_into()?) } } impl Type for i32 { fn type_info() -> SqliteTypeInfo { - SqliteTypeInfo(DataType::Int) + SqliteTypeInfo(DataType::Int4) } fn compatible(ty: &SqliteTypeInfo) -> bool { - matches!(ty.0, DataType::Int | DataType::Int64) + matches!(ty.0, DataType::Int4 | DataType::Integer) } } @@ -73,17 +77,17 @@ impl<'q> Encode<'q, Sqlite> for i32 { impl<'r> Decode<'r, Sqlite> for i32 { fn decode(value: SqliteValueRef<'r>) -> Result { - Ok(value.int()) + Ok(value.int64().try_into()?) } } impl Type for i64 { fn type_info() -> SqliteTypeInfo { - SqliteTypeInfo(DataType::Int64) + SqliteTypeInfo(DataType::Integer) } fn compatible(ty: &SqliteTypeInfo) -> bool { - matches!(ty.0, DataType::Int | DataType::Int64) + matches!(ty.0, DataType::Int4 | DataType::Integer) } } diff --git a/sqlx-sqlite/src/types/mod.rs b/sqlx-sqlite/src/types/mod.rs index 0bc62737d..85275273a 100644 --- a/sqlx-sqlite/src/types/mod.rs +++ b/sqlx-sqlite/src/types/mod.rs @@ -7,28 +7,33 @@ //! | `bool` | BOOLEAN | //! | `i8` | INTEGER | //! | `i16` | INTEGER | -//! | `i32` | INTEGER | +//! | `i32` | INTEGER, INT4 | //! | `i64` | BIGINT, INT8 | //! | `u8` | INTEGER | //! | `u16` | INTEGER | //! | `u32` | INTEGER | +//! | `u64` | INTEGER (Decode only; see note) | //! | `f32` | REAL | //! | `f64` | REAL | //! | `&str`, [`String`] | TEXT | //! | `&[u8]`, `Vec` | BLOB | //! //! #### Note: Unsigned Integers -//! The unsigned integer types `u8`, `u16` and `u32` are implemented by zero-extending to the -//! next-larger signed type. So `u8` becomes `i16`, `u16` becomes `i32`, and `u32` becomes `i64` -//! while still retaining their semantic values. +//! Decoding of unsigned integer types simply performs a checked conversion +//! to ensure that overflow does not occur. //! -//! Similarly, decoding performs a checked truncation to ensure that overflow does not occur. +//! Encoding of the unsigned integer types `u8`, `u16` and `u32` is implemented by zero-extending to +//! the next-larger signed type. So `u8` becomes `i16`, `u16` becomes `i32`, and `u32` becomes `i64` +//! while still retaining their semantic values. //! //! SQLite stores integers in a variable-width encoding and always handles them in memory as 64-bit //! signed values, so no space is wasted by this implicit widening. //! -//! However, there is no corresponding larger type for `u64` in SQLite (it would require a `i128`), -//! and so it is not supported. Bit-casting it to `i64` or storing it as `REAL`, `BLOB` or `TEXT` +//! However, there is no corresponding larger type for `u64` in SQLite +//! (it would require a native 16-byte integer, i.e. the equivalent of `i128`), +//! and so encoding is not supported for this type. +//! +//! Bit-casting `u64` to `i64`, or storing it as `REAL`, `BLOB` or `TEXT`, //! would change the semantics of the value in SQL and so violates the principle of least surprise. //! //! ### [`chrono`](https://crates.io/crates/chrono) diff --git a/sqlx-sqlite/src/types/time.rs b/sqlx-sqlite/src/types/time.rs index fa4e947f7..1adfdeddd 100644 --- a/sqlx-sqlite/src/types/time.rs +++ b/sqlx-sqlite/src/types/time.rs @@ -29,7 +29,7 @@ impl Type for PrimitiveDateTime { fn compatible(ty: &SqliteTypeInfo) -> bool { matches!( ty.0, - DataType::Datetime | DataType::Text | DataType::Int64 | DataType::Int + DataType::Datetime | DataType::Text | DataType::Integer | DataType::Int4 ) } } @@ -122,7 +122,7 @@ impl<'r> Decode<'r, Sqlite> for Time { fn decode_offset_datetime(value: SqliteValueRef<'_>) -> Result { let dt = match value.type_info().0 { DataType::Text => decode_offset_datetime_from_text(value.text()?), - DataType::Int | DataType::Int64 => { + DataType::Int4 | DataType::Integer => { Some(OffsetDateTime::from_unix_timestamp(value.int64())?) } @@ -155,7 +155,7 @@ fn decode_offset_datetime_from_text(value: &str) -> Option { fn decode_datetime(value: SqliteValueRef<'_>) -> Result { let dt = match value.type_info().0 { DataType::Text => decode_datetime_from_text(value.text()?), - DataType::Int | DataType::Int64 => { + DataType::Int4 | DataType::Integer => { let parsed = OffsetDateTime::from_unix_timestamp(value.int64()).unwrap(); Some(PrimitiveDateTime::new(parsed.date(), parsed.time())) } diff --git a/sqlx-sqlite/src/types/uint.rs b/sqlx-sqlite/src/types/uint.rs index 01a5b2a7d..4c4b4df48 100644 --- a/sqlx-sqlite/src/types/uint.rs +++ b/sqlx-sqlite/src/types/uint.rs @@ -7,11 +7,11 @@ use crate::{Sqlite, SqliteArgumentValue, SqliteTypeInfo, SqliteValueRef}; impl Type for u8 { fn type_info() -> SqliteTypeInfo { - SqliteTypeInfo(DataType::Int) + SqliteTypeInfo(DataType::Int4) } fn compatible(ty: &SqliteTypeInfo) -> bool { - matches!(ty.0, DataType::Int | DataType::Int64) + matches!(ty.0, DataType::Int4 | DataType::Integer) } } @@ -25,17 +25,21 @@ impl<'q> Encode<'q, Sqlite> for u8 { impl<'r> Decode<'r, Sqlite> for u8 { fn decode(value: SqliteValueRef<'r>) -> Result { - Ok(value.int().try_into()?) + // NOTE: using `sqlite3_value_int64()` here because `sqlite3_value_int()` silently truncates + // which leads to bugs, e.g.: + // https://github.com/launchbadge/sqlx/issues/3179 + // Similar bug in Postgres: https://github.com/launchbadge/sqlx/issues/3161 + Ok(value.int64().try_into()?) } } impl Type for u16 { fn type_info() -> SqliteTypeInfo { - SqliteTypeInfo(DataType::Int) + SqliteTypeInfo(DataType::Int4) } fn compatible(ty: &SqliteTypeInfo) -> bool { - matches!(ty.0, DataType::Int | DataType::Int64) + matches!(ty.0, DataType::Int4 | DataType::Integer) } } @@ -49,17 +53,17 @@ impl<'q> Encode<'q, Sqlite> for u16 { impl<'r> Decode<'r, Sqlite> for u16 { fn decode(value: SqliteValueRef<'r>) -> Result { - Ok(value.int().try_into()?) + Ok(value.int64().try_into()?) } } impl Type for u32 { fn type_info() -> SqliteTypeInfo { - SqliteTypeInfo(DataType::Int64) + SqliteTypeInfo(DataType::Integer) } fn compatible(ty: &SqliteTypeInfo) -> bool { - matches!(ty.0, DataType::Int | DataType::Int64) + matches!(ty.0, DataType::Int4 | DataType::Integer) } } @@ -76,3 +80,19 @@ impl<'r> Decode<'r, Sqlite> for u32 { Ok(value.int64().try_into()?) } } + +impl Type for u64 { + fn type_info() -> SqliteTypeInfo { + SqliteTypeInfo(DataType::Integer) + } + + fn compatible(ty: &SqliteTypeInfo) -> bool { + matches!(ty.0, DataType::Int4 | DataType::Integer) + } +} + +impl<'r> Decode<'r, Sqlite> for u64 { + fn decode(value: SqliteValueRef<'r>) -> Result { + Ok(value.int64().try_into()?) + } +} diff --git a/sqlx-sqlite/src/value.rs b/sqlx-sqlite/src/value.rs index efac04c42..1a4d8898a 100644 --- a/sqlx-sqlite/src/value.rs +++ b/sqlx-sqlite/src/value.rs @@ -6,8 +6,7 @@ use std::sync::Arc; use libsqlite3_sys::{ sqlite3_value, sqlite3_value_blob, sqlite3_value_bytes, sqlite3_value_double, - sqlite3_value_dup, sqlite3_value_free, sqlite3_value_int, sqlite3_value_int64, - sqlite3_value_type, SQLITE_NULL, + sqlite3_value_dup, sqlite3_value_free, sqlite3_value_int64, sqlite3_value_type, SQLITE_NULL, }; pub(crate) use sqlx_core::value::{Value, ValueRef}; @@ -27,12 +26,10 @@ impl<'r> SqliteValueRef<'r> { Self(SqliteValueData::Value(value)) } - pub(super) fn int(&self) -> i32 { - match self.0 { - SqliteValueData::Value(v) => v.int(), - } - } - + // NOTE: `int()` is deliberately omitted because it will silently truncate a wider value, + // which is likely to cause bugs: + // https://github.com/launchbadge/sqlx/issues/3179 + // (Similar bug in Postgres): https://github.com/launchbadge/sqlx/issues/3161 pub(super) fn int64(&self) -> i64 { match self.0 { SqliteValueData::Value(v) => v.int64(), @@ -114,10 +111,6 @@ impl SqliteValue { } } - fn int(&self) -> i32 { - unsafe { sqlite3_value_int(self.handle.0.as_ptr()) } - } - fn int64(&self) -> i64 { unsafe { sqlite3_value_int64(self.handle.0.as_ptr()) } } diff --git a/tests/sqlite/any.rs b/tests/sqlite/any.rs index f96274a3a..856db70c0 100644 --- a/tests/sqlite/any.rs +++ b/tests/sqlite/any.rs @@ -16,3 +16,20 @@ async fn it_encodes_bool_with_any() -> anyhow::Result<()> { Ok(()) } + +#[sqlx_macros::test] +async fn issue_3179() -> anyhow::Result<()> { + sqlx::any::install_default_drivers(); + + let mut conn = new::().await?; + + // 4294967297 = 2^32 + let number: i64 = sqlx::query_scalar("SELECT 4294967296") + .fetch_one(&mut conn) + .await?; + + // Previously, the decoding would use `i32` as an intermediate which would overflow to 0. + assert_eq!(number, 4294967296); + + Ok(()) +} diff --git a/tests/sqlite/macros.rs b/tests/sqlite/macros.rs index 0afad7583..74e689260 100644 --- a/tests/sqlite/macros.rs +++ b/tests/sqlite/macros.rs @@ -115,24 +115,24 @@ async fn test_query_scalar() -> anyhow::Result<()> { let mut conn = new::().await?; let id = sqlx::query_scalar!("select 1").fetch_one(&mut conn).await?; - assert_eq!(id, 1i32); + assert_eq!(id, 1i64); // invalid column names are ignored let id = sqlx::query_scalar!(r#"select 1 as "&foo""#) .fetch_one(&mut conn) .await?; - assert_eq!(id, 1i32); + assert_eq!(id, 1i64); let id = sqlx::query_scalar!(r#"select 1 as "foo!""#) .fetch_one(&mut conn) .await?; - assert_eq!(id, 1i32); + assert_eq!(id, 1i64); let id = sqlx::query_scalar!(r#"select 1 as "foo?""#) .fetch_one(&mut conn) .await?; - assert_eq!(id, Some(1i32)); + assert_eq!(id, Some(1i64)); let id = sqlx::query_scalar!(r#"select 1 as "foo: MyInt""#) .fetch_one(&mut conn)