Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#[sqlx(json)] does not support nullable values #2849

Open
Obstbeeren opened this issue Nov 1, 2023 · 10 comments · May be fixed by #3677
Open

#[sqlx(json)] does not support nullable values #2849

Obstbeeren opened this issue Nov 1, 2023 · 10 comments · May be fixed by #3677
Labels

Comments

@Obstbeeren
Copy link

Bug Description

When trying to use #[sqlx(json)] on a nullable column, an error is thrown

Minimal Reproduction

create table test
(
    id   serial primary key,
    data json
);

insert into test
values (1, NULL),
       (2, '{
         "data": 1
       }'::json);
#[derive(Debug, sqlx::FromRow)]
struct Test {
    id: i32,
    #[sqlx(json)]
    data: Option<Data>,
    // this works without problems
    // data: Option<Json<Data>>
}

#[derive(Debug, serde::Deserialize)]
struct Data {
    data: i32,
}

#[tokio::main]
async fn main() {
    let database = sqlx::postgres::PgPoolOptions::new()
        .connect("postgresql://johannes:1@localhost:5432/plyoox")
        .await
        .expect("Database connection failed");

    // works
    let x = sqlx::query_as::<_, Test>("SELECT id, data FROM test WHERE id = 2")
        .fetch_one(&database)
        .await
        .expect("Error");

    println!("{:?}", x);

    // fails
    let x = sqlx::query_as::<_, Test>("SELECT id, data FROM test WHERE id = 1")
        .fetch_one(&database)
        .await
        .expect("Error");

    println!("{:?}", x);
}

Info

  • SQLx version: 0.7.2
  • SQLx features enabled: "postgres", "runtime-tokio", "tls-native-tls", "json"
  • Database server and version: Postgres 16.0
  • Operating system: Ubuntu
  • rustc --version: 1.73.0
@Obstbeeren Obstbeeren added the bug label Nov 1, 2023
@abonander
Copy link
Collaborator

What is the error?

@Obstbeeren
Copy link
Author

UnexpectedNullError:
ColumnDecode { index: "\"data\"", source: UnexpectedNullError }

@abonander
Copy link
Collaborator

abonander commented Nov 3, 2023

Okay, that's why I asked because on first blush I thought you were getting a compiler error. That's why it's important to specify exactly what error you're getting.

So the problem is that #[sqlx(json)] transforms the field to Json<Option<Data>>, which means it can handle the JSON string 'NULL' but not the SQL value NULL. That does seem rather surprising on review but the transformation is quite naive.

Now, it's quite possible that someone might actually want it to work that way, so unfortunately we can't just change it to output Option<Json<Data>> now without it being a breaking change.

However, there might be another option.

I think it would be relatively easy to specialize Json<Option<Data>> to accept either 'NULL' or NULL . If it gets a null column error, it could just attempt to decode from a literal 'NULL' instead, That would even let it work with types that borrow from the deserializer still.

I'm not sure if that would be too surprising or not. Do you think anyone would actually ever need to distinguish between 'NULL' and NULL?

@Obstbeeren
Copy link
Author

I'm sorry for the confusion, I won't forget it next time :D

I think it would be relatively easy to specialize Json<Option<Data>> to accept either 'NULL' or NULL . If it gets a null column error, it could just attempt to decode from a literal 'NULL' instead, That would even let it work with types that borrow from the deserializer still.

I'm not sure if that would be too surprising or not. Do you think anyone would actually ever need to distinguish between 'NULL' and NULL?

I cannot think of any situation where someone would need to distinguish between the these, so I really like this solution.

Another possbile solution would be something like #[sqlx(json(nullable))] that converts Option<Data> to Option<Json<Data>> , but I'm not to sure about that.

@abonander
Copy link
Collaborator

Yeah, #[sqlx(json(nullable))] was the other option I was thinking.

@abonander
Copy link
Collaborator

Thinking about this more, I'm wary of quietly changing the behavior of Json in a minor release, even if it's hard to imagine why someone would care. Though #[sqlx(json(nullable)]] feels less elegant, it would at least be backwards-compatible.

@IgnisDa
Copy link

IgnisDa commented Nov 22, 2023

@abonander is there any update on this? I am getting the same error.

@abonander
Copy link
Collaborator

We'd always welcome a pull request.

@frederikhors
Copy link

I was also impacted by this today! I don't know what to do now. Why is an Optional field saved in the DB with the text "null"?

This is really strange.

Anyway thanks all of you for your hard work! ♥

@y0zong
Copy link

y0zong commented Aug 23, 2024

for now I done this in inelegant way and hope there will be official way for this

  1. remove #[sqlx(json)]
  2. remove singer? in SQL(same result but no need to keep it)
  3. use json_build_object return json value from SELECT
  4. implementation FromRow deserialize json value for spec field(singer: Option)
        CASE WHEN songs.singer_id IS NOT NULL THEN
            json_build_object('id', a3.id, 'name', a3.name)
        ELSE
            NULL
        END AS singer
#[derive(Debug, PartialEq, Deserialize, Serialize, sqlx::FromRow)]
pub struct Artist {
    pub id: Uuid,
    pub name: String,
    pub created_at: DateTime<Utc>,
}

#[derive(Debug, PartialEq, Deserialize, Serialize)]
pub struct Song {
    pub id: Uuid,
    pub name: String,
    pub singer: Option<Artist>,
    pub updated_at: DateTime<Utc>,
}

impl FromRow<'_, PgRow> for Song {
    fn from_row(row: &PgRow) -> sqlx::Result<Self> {
        Ok(Self {
            id: row.get("id"),
            name: row.get("name"),
            // here we check if null
            singer: match row.try_get::<JsonValue, &str>("singer") {
                Ok(singer) => Some(Artist::deserialize::<JsonValue>(singer).unwrap()),
                Err(_) => None,
            },
            updated_at: row.get("updated_at"),
        })
    }
}

@seanaye seanaye linked a pull request Jan 10, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants