(Fix) Handle nullability of SQLite rowid alias columns (#4088)

* Correctly handle rowid alias columns

* Added test for rowid aliasing

* (SQLite) Added test for nullability of rowid

Tests the nullability of explicitly referring to the rowid column (not
through an alias).

Author: Lege19

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] = <value of constant>
                     state.mem.r.insert(
                         p2,
@@ -1778,3 +1782,63 @@ 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 == [SqliteTypeInfo(DataType::Integer)] && nullable == [Some(false)]
+        } else {
+            false
+        }
+    );
+    assert!(
+        if let Ok((ty, nullable)) = explain(&mut conn, "SELECT * FROM not_an_alias") {
+            ty == [SqliteTypeInfo(DataType::Integer)] && nullable == [Some(true)]
+        } else {
+            false
+        }
+    );
+    assert!(
+        if let Ok((ty, nullable)) = explain(&mut conn, "SELECT rowid FROM an_alias") {
+            ty == [SqliteTypeInfo(DataType::Integer)] && nullable == [Some(false)]
+        } else {
+            false
+        }
+    );
+    assert!(
+        if let Ok((ty, nullable)) = explain(&mut conn, "SELECT rowid FROM not_an_alias") {
+            ty == [SqliteTypeInfo(DataType::Integer)] && nullable == [Some(false)]
+        } else {
+            false
+        }
+    );
+}

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<Option<bool>, Error> {
         unsafe {
             let index = check_col_idx!(index);
-
             // https://sqlite.org/c3ref/column_database_name.html
             //
             // ### Note
@@ -212,24 +217,25 @@ 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(
                 self.db_handle(),
                 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)
+                },
+            )
         }
     }