Skip to content

Commit

Permalink
breaking(sqlite): always use i64 as intermediate when decoding (#3184)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@abonander
abonander authored Apr 13, 2024
1 parent 6a4f61e commit 25efb2f
Show file tree
Hide file tree
Showing 13 changed files with 145 additions and 88 deletions.
4 changes: 2 additions & 2 deletions sqlx-sqlite/src/any.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
41 changes: 21 additions & 20 deletions sqlx-sqlite/src/connection/explain.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool> {
Expand All @@ -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),
},
}
Expand Down Expand Up @@ -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,
Expand All @@ -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,
}
Expand Down Expand Up @@ -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),
}),
);
Expand Down Expand Up @@ -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),
}),
);
Expand Down Expand Up @@ -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),
}),
);
Expand Down Expand Up @@ -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
}),
);
Expand Down Expand Up @@ -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),
}),
);
Expand All @@ -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();
Expand Down Expand Up @@ -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),
}),
);
Expand Down Expand Up @@ -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),
}),
);
Expand Down Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand Down
5 changes: 5 additions & 0 deletions sqlx-sqlite/src/type_checking.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 == <T>::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,
Expand Down
42 changes: 25 additions & 17 deletions sqlx-sqlite/src/type_info.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -87,15 +95,15 @@ impl FromStr for DataType {
fn from_str(s: &str) -> Result<Self, Self::Err> {
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,

Expand All @@ -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()?);
Expand Down
4 changes: 2 additions & 2 deletions sqlx-sqlite/src/types/bool.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ impl Type<Sqlite> 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)
}
}

Expand All @@ -25,6 +25,6 @@ impl<'q> Encode<'q, Sqlite> for bool {

impl<'r> Decode<'r, Sqlite> for bool {
fn decode(value: SqliteValueRef<'r>) -> Result<bool, BoxDynError> {
Ok(value.int() != 0)
Ok(value.int64() != 0)
}
}
8 changes: 6 additions & 2 deletions sqlx-sqlite/src/types/chrono.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@ impl Type<Sqlite> 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
)
}
}
Expand Down Expand Up @@ -105,7 +109,7 @@ impl<'r> Decode<'r, Sqlite> for DateTime<FixedOffset> {
fn decode_datetime(value: SqliteValueRef<'_>) -> Result<DateTime<FixedOffset>, 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,
Expand Down
Loading

0 comments on commit 25efb2f

Please sign in to comment.