diff --git a/sqlx-macros/src/query/output.rs b/sqlx-macros/src/query/output.rs index e93b3867..659e747b 100644 --- a/sqlx-macros/src/query/output.rs +++ b/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(describe: &Describe) -> 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) { - Some(quote! { Option<#type_> }) - } else { + if column.not_null.unwrap_or(false) { 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::()?; Ok(ColumnOverride::NonNull) + } else if lookahead.peek(Token![?]) { + input.parse::()?; + + Ok(ColumnOverride::Nullable) } else { Err(lookahead.error()) } diff --git a/src/macros.rs b/src/macros.rs index 34c8e7a0..09320646 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -152,7 +152,53 @@ /// // For Postgres this would have been inferred to be Option 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 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 diff --git a/tests/mysql/macros.rs b/tests/mysql/macros.rs index 135b7056..bd259793 100644 --- a/tests/mysql/macros.rs +++ b/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::().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); diff --git a/tests/postgres/macros.rs b/tests/postgres/macros.rs index 4d9c90e2..c19d76a8 100644 --- a/tests/postgres/macros.rs +++ b/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::().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); diff --git a/tests/sqlite/macros.rs b/tests/sqlite/macros.rs index 338b1609..f0b42485 100644 --- a/tests/sqlite/macros.rs +++ b/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::().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::().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::().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::().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?; diff --git a/tests/sqlite/setup.sql b/tests/sqlite/setup.sql index 18cc8426..f0306ab9 100644 --- a/tests/sqlite/setup.sql +++ b/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); diff --git a/tests/sqlite/sqlite.db b/tests/sqlite/sqlite.db index 1daa80ee..9b925366 100644 Binary files a/tests/sqlite/sqlite.db and b/tests/sqlite/sqlite.db differ