From 0e5798f3e0cebe5b9ff05d68d9146469c5175d88 Mon Sep 17 00:00:00 2001 From: jacobs Date: Tue, 4 Nov 2025 21:40:14 +0000 Subject: [PATCH 1/2] Correctly handle rowid alias columns --- sqlx-sqlite/src/connection/explain.rs | 10 +++++++--- sqlx-sqlite/src/statement/handle.rs | 28 ++++++++++++++++++++++----- 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index edd65ece49..b66fefbd7c 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -43,6 +43,7 @@ const OP_IDX_GE: &str = "IdxGE"; const OP_IDX_GT: &str = "IdxGT"; const OP_IDX_LE: &str = "IdxLE"; const OP_IDX_LT: &str = "IdxLT"; +const OP_IDX_ROWID: &str = "IdxRowid"; const OP_IF: &str = "If"; const OP_IF_NO_HOPE: &str = "IfNoHope"; const OP_IF_NOT: &str = "IfNot"; @@ -361,7 +362,9 @@ fn opcode_to_type(op: &str) -> DataType { OP_REAL => DataType::Float, OP_BLOB => DataType::Blob, OP_AND | OP_OR => DataType::Bool, - OP_NEWROWID | OP_ROWID | OP_COUNT | OP_INT64 | OP_INTEGER => DataType::Integer, + OP_NEWROWID | OP_IDX_ROWID | OP_ROWID | OP_COUNT | OP_INT64 | OP_INTEGER => { + DataType::Integer + } OP_STRING8 => DataType::Text, OP_COLUMN | _ => DataType::Null, } @@ -676,7 +679,7 @@ pub(super) fn explain( //nobranch if maybe not null let might_not_branch = match state.mem.r.get(&p1) { - Some(r_p1) => !matches!(r_p1.map_to_datatype(), DataType::Null), + Some(r_p1) => r_p1.map_to_nullable() != Some(false), _ => false, }; @@ -1379,7 +1382,8 @@ pub(super) fn explain( state.mem.r.insert(p2, RegDataType::Int(p1)); } - OP_BLOB | OP_COUNT | OP_REAL | OP_STRING8 | OP_ROWID | OP_NEWROWID => { + OP_BLOB | OP_COUNT | OP_REAL | OP_STRING8 | OP_ROWID | OP_IDX_ROWID + | OP_NEWROWID => { // r[p2] = state.mem.r.insert( p2, diff --git a/sqlx-sqlite/src/statement/handle.rs b/sqlx-sqlite/src/statement/handle.rs index 7985ff9d36..1ceb00d2d4 100644 --- a/sqlx-sqlite/src/statement/handle.rs +++ b/sqlx-sqlite/src/statement/handle.rs @@ -198,10 +198,15 @@ impl StatementHandle { } } + /// Use sqlite3_column_metadata to determine if a specific column is nullable. + /// + /// Returns None in the case of INTEGER PRIMARY KEYs + /// This is because this column is an alias to rowid if the table does not use a compound + /// primary key. In this case the row is not nullable, and the output of + /// sqlite3_column_metadata may be incorrect. pub(crate) fn column_nullable(&self, index: usize) -> Result, Error> { unsafe { let index = check_col_idx!(index); - // https://sqlite.org/c3ref/column_database_name.html // // ### Note @@ -212,12 +217,13 @@ impl StatementHandle { let db_name = sqlite3_column_database_name(self.0.as_ptr(), index); let table_name = sqlite3_column_table_name(self.0.as_ptr(), index); let origin_name = sqlite3_column_origin_name(self.0.as_ptr(), index); - if db_name.is_null() || table_name.is_null() || origin_name.is_null() { return Ok(None); } let mut not_null: c_int = 0; + let mut datatype: *const c_char = ptr::null(); + let mut primary_key: c_int = 0; // https://sqlite.org/c3ref/table_column_metadata.html let status = sqlite3_table_column_metadata( @@ -225,11 +231,11 @@ impl StatementHandle { db_name, table_name, origin_name, + &mut datatype, // function docs state to provide NULL for return values you don't care about ptr::null_mut(), - ptr::null_mut(), &mut not_null, - ptr::null_mut(), + &mut primary_key, ptr::null_mut(), ); @@ -245,7 +251,19 @@ impl StatementHandle { return Err(SqliteError::new(self.db_handle()).into()); } - Ok(Some(not_null == 0)) + let datatype = CStr::from_ptr(datatype); + + Ok( + if primary_key != 0 + && datatype + .to_bytes() + .eq_ignore_ascii_case("integer".as_bytes()) + { + None + } else { + Some(not_null == 0) + }, + ) } } From 70d4157bea5e25e9f8ef6d50b9c4e9449d5942a9 Mon Sep 17 00:00:00 2001 From: jacobs Date: Tue, 4 Nov 2025 21:57:58 +0000 Subject: [PATCH 2/2] Added test for rowid aliasing --- sqlx-sqlite/src/connection/explain.rs | 48 +++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index b66fefbd7c..33b31f9af8 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -1782,3 +1782,51 @@ fn test_root_block_columns_has_types() { ); } } + +#[test] +fn test_explain() { + use crate::SqliteConnectOptions; + use std::str::FromStr; + let conn_options = SqliteConnectOptions::from_str("sqlite::memory:").unwrap(); + let mut conn = super::EstablishParams::from_options(&conn_options) + .unwrap() + .establish() + .unwrap(); + + assert!(execute::iter( + &mut conn, + r"CREATE TABLE an_alias(a INTEGER PRIMARY KEY);", + None, + false + ) + .unwrap() + .next() + .is_some()); + + assert!(execute::iter( + &mut conn, + r"CREATE TABLE not_an_alias(a INT PRIMARY KEY);", + None, + false + ) + .unwrap() + .next() + .is_some()); + + assert!( + if let Ok((ty, nullable)) = explain(&mut conn, "SELECT * FROM an_alias") { + ty.as_slice() == &[SqliteTypeInfo(DataType::Integer)] + && nullable.as_slice() == &[Some(false)] + } else { + false + } + ); + assert!( + if let Ok((ty, nullable)) = explain(&mut conn, "SELECT * FROM not_an_alias") { + ty.as_slice() == &[SqliteTypeInfo(DataType::Integer)] + && nullable.as_slice() == &[Some(true)] + } else { + false + } + ); +}