diff --git a/sqlx-sqlite/src/connection/describe.rs b/sqlx-sqlite/src/connection/describe.rs index 400c671d96..070aee0724 100644 --- a/sqlx-sqlite/src/connection/describe.rs +++ b/sqlx-sqlite/src/connection/describe.rs @@ -7,7 +7,6 @@ use crate::type_info::DataType; use crate::{Sqlite, SqliteColumn}; use sqlx_core::sql_str::SqlStr; use sqlx_core::Either; -use std::convert::identity; pub(crate) fn describe( conn: &mut ConnectionState, @@ -78,9 +77,8 @@ pub(crate) fn describe( ty }; - // check explain let col_nullable = stmt.handle.column_nullable(col)?; - let exp_nullable = fallback_nullable.get(col).copied().and_then(identity); + let exp_nullable = fallback_nullable.get(col).copied().flatten(); nullable.push(exp_nullable.or(col_nullable)); diff --git a/sqlx-sqlite/src/connection/explain.rs b/sqlx-sqlite/src/connection/explain.rs index 550f21557e..3a3807545f 100644 --- a/sqlx-sqlite/src/connection/explain.rs +++ b/sqlx-sqlite/src/connection/explain.rs @@ -373,9 +373,9 @@ fn opcode_to_type(op: &str) -> DataType { fn root_block_columns( conn: &mut ConnectionState, ) -> Result>, Error> { - let table_block_columns: Vec<(i64, i64, i64, String, bool)> = execute::iter( + let table_block_columns: Vec<(i64, i64, i64, String, bool, i64)> = execute::iter( conn, - "SELECT s.dbnum, s.rootpage, col.cid as colnum, col.type, col.\"notnull\" + "SELECT s.dbnum, s.rootpage, col.cid as colnum, col.type, col.\"notnull\", col.pk FROM ( select 1 dbnum, tss.* from temp.sqlite_schema tss UNION ALL select 0 dbnum, mss.* from main.sqlite_schema mss @@ -383,7 +383,7 @@ fn root_block_columns( JOIN pragma_table_info(s.name) AS col WHERE s.type = 'table' UNION ALL - SELECT s.dbnum, s.rootpage, idx.seqno as colnum, col.type, col.\"notnull\" + SELECT s.dbnum, s.rootpage, idx.seqno as colnum, col.type, col.\"notnull\", col.pk FROM ( select 1 dbnum, tss.* from temp.sqlite_schema tss UNION ALL select 0 dbnum, mss.* from main.sqlite_schema mss @@ -400,13 +400,13 @@ fn root_block_columns( .collect::, Error>>()?; let mut row_info: HashMap<(i64, i64), IntMap> = HashMap::new(); - for (dbnum, block, colnum, datatype, notnull) in table_block_columns { + for (dbnum, block, colnum, datatype, notnull, pk) in table_block_columns { let row_info = row_info.entry((dbnum, block)).or_default(); row_info.insert( colnum, ColumnType::Single { datatype: datatype.parse().unwrap_or(DataType::Null), - nullable: Some(!notnull), + nullable: Some(!(notnull || (pk > 0 && datatype.to_lowercase() == "integer"))), }, ); } @@ -1640,7 +1640,7 @@ fn test_root_block_columns_has_types() { assert_eq!( Some(&ColumnType::Single { datatype: DataType::Integer, - nullable: Some(true) //sqlite primary key columns are nullable unless declared not null + nullable: Some(false) }), root_block_cols[&table_db_block].get(&0) ); @@ -1665,7 +1665,7 @@ fn test_root_block_columns_has_types() { assert_eq!( Some(&ColumnType::Single { datatype: DataType::Integer, - nullable: Some(true) //sqlite primary key columns are nullable unless declared not null + nullable: Some(false) }), root_block_cols[&table_db_block].get(&0) ); @@ -1683,7 +1683,7 @@ fn test_root_block_columns_has_types() { assert_eq!( Some(&ColumnType::Single { datatype: DataType::Integer, - nullable: Some(true) //sqlite primary key columns are nullable unless declared not null + nullable: Some(false) }), root_block_cols[&table_db_block].get(&0) ); diff --git a/sqlx-sqlite/src/statement/handle.rs b/sqlx-sqlite/src/statement/handle.rs index c78ce98414..810fb1776b 100644 --- a/sqlx-sqlite/src/statement/handle.rs +++ b/sqlx-sqlite/src/statement/handle.rs @@ -212,9 +212,9 @@ impl StatementHandle { /// Use sqlite3_column_metadata to determine if a specific column is nullable. /// - /// Returns None in the case of INTEGER PRIMARY KEYs + /// Returns Some(false) 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 + /// primary key. In this case the column is not nullable, and the output of /// sqlite3_column_metadata may be incorrect. pub(crate) fn column_nullable(&self, index: usize) -> Result, Error> { unsafe { @@ -271,7 +271,7 @@ impl StatementHandle { .to_bytes() .eq_ignore_ascii_case("integer".as_bytes()) { - None + Some(false) } else { Some(not_null == 0) }, diff --git a/tests/sqlite/macros.rs b/tests/sqlite/macros.rs index 0ab4dd56bf..cca0b19593 100644 --- a/tests/sqlite/macros.rs +++ b/tests/sqlite/macros.rs @@ -359,3 +359,33 @@ async fn test_column_override_exact_nullable() -> anyhow::Result<()> { } // we don't emit bind parameter typechecks for SQLite so testing the overrides is redundant + +// Regression test: INTEGER PRIMARY KEY (a rowid alias) must not be nullable. +// Before the fix, `query!` would infer `id` as `Option` and the +// assignment `let _: i64 = row.id` would fail to compile. +#[sqlx_macros::test] +async fn test_returning_primary_key_is_not_nullable() -> anyhow::Result<()> { + let mut conn = new::().await?; + + let row = + sqlx::query!(r#"INSERT INTO accounts_no_not_null ( name ) VALUES ( 'test' ) RETURNING id"#) + .fetch_one(&mut conn) + .await?; + + let _: i64 = row.id; + + Ok(()) +} + +#[sqlx_macros::test] +async fn test_returning_with_foreign_key_is_not_nullable() -> anyhow::Result<()> { + let mut conn = new::().await?; + + let _id: i64 = sqlx::query_scalar!( + r#"INSERT INTO packages ( project_id ) VALUES ( 1 ) RETURNING package_id"# + ) + .fetch_one(&mut conn) + .await?; + + Ok(()) +} diff --git a/tests/sqlite/setup.sql b/tests/sqlite/setup.sql index 8f7d77c46d..c7efca7919 100644 --- a/tests/sqlite/setup.sql +++ b/tests/sqlite/setup.sql @@ -23,6 +23,10 @@ CREATE TABLE accounts ( name TEXT NOT NULL, is_active BOOLEAN ); +CREATE TABLE accounts_no_not_null ( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL +); INSERT INTO accounts(id, name, is_active) VALUES (1, 'Herp Derpinson', 1); CREATE VIEW accounts_view as @@ -35,3 +39,8 @@ CREATE TABLE products ( price NUMERIC, CONSTRAINT price_greater_than_zero CHECK (price > 0) ); + +CREATE TABLE packages ( + package_id INTEGER PRIMARY KEY, + project_id INTEGER +); diff --git a/tests/sqlite/sqlite.db b/tests/sqlite/sqlite.db index e357615025..5d776ba266 100644 Binary files a/tests/sqlite/sqlite.db and b/tests/sqlite/sqlite.db differ