feat(macros): support nullable column override

sqlx-macros/src/query/output.rs

@@ -30,6 +30,7 @@ struct ColumnDecl {
 
 enum ColumnOverride {
     NonNull,
+    Nullable,
     Wildcard,
     Exact(Type),
 }
@@ -53,14 +54,19 @@ pub fn columns_to_rust<DB: DatabaseExt>(describe: &Describe<DB>) -> crate::Resul
             let type_ = match decl.r#override {
                 Some(ColumnOverride::Exact(ty)) => Some(ty.to_token_stream()),
                 Some(ColumnOverride::Wildcard) => None,
+                // these three could be combined but I prefer the clarity here
                 Some(ColumnOverride::NonNull) => Some(get_column_type(i, column)),
+                Some(ColumnOverride::Nullable) => {
+                    let type_ = get_column_type(i, column);
+                    Some(quote! { Option<#type_> })
+                }
                 None => {
                     let type_ = get_column_type(i, column);
 
-                    if !column.not_null.unwrap_or(false) {
+                    if column.not_null.unwrap_or(false) {
-                        Some(quote! { Option<#type_> })
-                    } else {
                         Some(type_)
+                    } else {
+                        Some(quote! { Option<#type_> })
                     }
                 }
             };
@@ -165,7 +171,7 @@ impl ColumnDecl {
         // find the end of the identifier because we want to use our own logic to parse it
         // if we tried to feed this into `syn::parse_str()` we might get an un-great error
         // for some kinds of invalid identifiers
-        let (ident, remainder) = if let Some(i) = col_name.find(&[':', '!'][..]) {
+        let (ident, remainder) = if let Some(i) = col_name.find(&[':', '!', '?'][..]) {
             let (ident, remainder) = col_name.split_at(i);
 
             (parse_ident(ident)?, remainder)
@@ -202,6 +208,10 @@ impl Parse for ColumnOverride {
             input.parse::<Token![!]>()?;
 
             Ok(ColumnOverride::NonNull)
+        } else if lookahead.peek(Token![?]) {
+            input.parse::<Token![?]>()?;
+
+            Ok(ColumnOverride::Nullable)
         } else {
             Err(lookahead.error())
         }

src/macros.rs

@@ -152,7 +152,53 @@
 /// // For Postgres this would have been inferred to be Option<i32> instead
 /// assert_eq!(record.id, 1i32);
 /// # }
+///
 /// ```
+/// * selecting a column `foo as "foo?"` (Postgres / SQLite) or `` foo as `foo?` `` overrides
+/// inferred nullability and forces the column to be treated as nullable; this is provided mainly
+/// for symmetry with `!`, but also because nullability inference currently has some holes and false
+/// negatives that may not be completely fixable without doing our own complex analysis on the given
+/// query.
+///
+/// ```rust,ignore
+/// # async fn main() {
+/// # let mut conn = panic!();
+/// // Postgres:
+/// // Note that this query wouldn't work in SQLite as we still don't know the exact type of `id`
+/// let record = sqlx::query!(r#"select 1 as "id?""#) // MySQL: use "select 1 as `id?`" instead
+///     .fetch_one(&mut conn)
+///     .await?;
+///
+/// // For Postgres this would have been inferred to be Option<i32> anyway
+/// // but this is just a basic example
+/// assert_eq!(record.id, Some(1i32));
+/// # }
+/// ```
+///
+/// One current such hole is exposed by left-joins involving `NOT NULL` columns in Postgres and
+/// SQLite; as we only know nullability for a given column based on the `NOT NULL` constraint
+/// of its original column in a table, if that column is then brought in via a `LEFT JOIN`
+/// we have no good way to know and so continue assuming it may not be null which may result
+/// in some `UnexpectedNull` errors at runtime.
+///
+/// Using `?` as an override we can fix this for columns we know to be nullable in practice:
+///
+/// ```rust,ignore
+/// # async fn main() {
+/// # let mut conn = panic!();
+/// // Ironically this is the exact column we look at to determine nullability in Postgres
+/// let record = sqlx::query!(
+///     r#"select attnotnull as "attnotnull?" from (values (1)) ids left join pg_attribute on false"#
+/// )
+/// .fetch_one(&mut conn)
+/// .await?;
+///
+/// // For Postgres this would have been inferred to be `bool` and we would have gotten an error
+/// assert_eq!(record.attnotnull, None);
+/// # }
+/// ```
+///
+/// See [launchbadge/sqlx#367](https://github.com/launchbadge/sqlx/issues/367) for more details on this issue.
 ///
 /// * selecting a column `foo as "foo: T"` (Postgres / SQLite) or `` foo as `foo: T` `` (MySQL)
 /// overrides the inferred type which is useful when selecting user-defined custom types

tests/mysql/macros.rs

@@ -106,6 +106,20 @@ async fn test_column_override_not_null() -> anyhow::Result<()> {
     Ok(())
 }
 
+#[sqlx_macros::test]
+async fn test_column_override_nullable() -> anyhow::Result<()> {
+    let mut conn = new::<MySql>().await?;
+
+    // MySQL by default tells us `id` is not-null
+    let record = sqlx::query!("select * from (select 1 as `id?`) records")
+        .fetch_one(&mut conn)
+        .await?;
+
+    assert_eq!(record.id, Some(1));
+
+    Ok(())
+}
+
 #[derive(PartialEq, Eq, Debug, sqlx::Type)]
 #[sqlx(transparent)]
 struct MyInt4(i32);

tests/postgres/macros.rs

@@ -268,6 +268,23 @@ async fn test_column_override_not_null() -> anyhow::Result<()> {
     Ok(())
 }
 
+#[sqlx_macros::test]
+async fn test_column_override_nullable() -> anyhow::Result<()> {
+    let mut conn = new::<Postgres>().await?;
+
+    // workaround for https://github.com/launchbadge/sqlx/issues/367
+    // declare a `NOT NULL` column from a left-joined table to be nullable
+    let record = sqlx::query!(
+        r#"select text as "text?" from (values (1)) foo(id) left join tweet on false"#
+    )
+    .fetch_one(&mut conn)
+    .await?;
+
+    assert_eq!(record.text, None);
+
+    Ok(())
+}
+
 #[derive(PartialEq, Eq, Debug, sqlx::Type)]
 #[sqlx(transparent)]
 struct MyInt4(i32);

tests/sqlite/macros.rs

@@ -76,11 +76,24 @@ async fn macro_select_from_view() -> anyhow::Result<()> {
 async fn test_column_override_not_null() -> anyhow::Result<()> {
     let mut conn = new::<Sqlite>().await?;
 
-    let record = sqlx::query!(r#"select is_active as "is_active!" from accounts"#)
+    let record = sqlx::query!(r#"select owner_id as `owner_id!` from tweet"#)
         .fetch_one(&mut conn)
         .await?;
 
-    assert_eq!(record.is_active, true);
+    assert_eq!(record.owner_id, 1);
+
+    Ok(())
+}
+
+#[sqlx_macros::test]
+async fn test_column_override_nullable() -> anyhow::Result<()> {
+    let mut conn = new::<Sqlite>().await?;
+
+    let record = sqlx::query!(r#"select text as `text?` from tweet"#)
+        .fetch_one(&mut conn)
+        .await?;
+
+    assert_eq!(record.text.as_deref(), Some("#sqlx is pretty cool!"));
 
     Ok(())
 }
@@ -97,7 +110,7 @@ async fn test_column_override_wildcard() -> anyhow::Result<()> {
 
     let mut conn = new::<Sqlite>().await?;
 
-    let record = sqlx::query_as!(Record, r#"select id as "id: _" from accounts"#)
+    let record = sqlx::query_as!(Record, r#"select id as "id: _" from tweet"#)
         .fetch_one(&mut conn)
         .await?;
 
@@ -117,7 +130,7 @@ async fn test_column_override_wildcard() -> anyhow::Result<()> {
 async fn test_column_override_exact() -> anyhow::Result<()> {
     let mut conn = new::<Sqlite>().await?;
 
-    let record = sqlx::query!(r#"select id as "id: MyInt" from accounts"#)
+    let record = sqlx::query!(r#"select id as "id: MyInt" from tweet"#)
         .fetch_one(&mut conn)
         .await?;
 

tests/sqlite/setup.sql

@@ -6,3 +6,5 @@ CREATE TABLE tweet
     is_sent  BOOLEAN NOT NULL DEFAULT TRUE,
     owner_id BIGINT
 );
+
+insert into tweet(id, text, owner_id) values (1, '#sqlx is pretty cool!', 1);