Question: Should nullability be checked with custom type overrides?

[phated]

I just had a runtime error surface in my application when moving to custom type overrides.

I have a nullable custom type that needs a override:

#[derive(Debug, Copy, Clone, Eq, PartialEq, sqlx::Type)]
#[sqlx(rename = "FACTION", rename_all = "uppercase")]
pub enum Faction {
    Autobot,
    Decepticon,
    Mercenary,
}

pub struct Card {
    pub faction: Option<Faction>
}

let card = sqlx::query_as!(
    Card,
    r#"
SELECT
    faction AS "faction: Option<Faction>"
FROM cards
WHERE id = $1;
"#,
    id
)
.fetch_one(&self.pool)
.await?;

When doing the migration, I accidentally used AS "faction: Faction" and the code compiled, but I ended up with a runtime error.

So, I'm wondering if that should have been caught by the compiler and informed me that I needed Option as part of my override.

[abonander]

I'm torn about this because while I do agree that this is a footgun, it's much easier to teach this form as "this is exactly the type that will be emitted".

We'd need a heuristic regarding when the user specifies Option<T> so we don't wrap it again to make Option<Option<T>> but that seems like it would be easy to get a false negative (e.g. if the user is using a type alias).

Speaking of which, part of the reason we're providing type overrides is because nullability inference has edge cases that we may never be able to handle perfectly in Postgres or SQLite. I'm loathe to introduce yet more edge cases in what is intended to be a partial solution.

It'd be nice if we could just make this a silencible compiler warning but we have no way to do that on stable.

[abonander]

If we had const_if/const_panic we could make this a compiler error which would be robust through typedefs.

[mehcode]

I actually think we should do this:

• AS "faction: Faction" - emits Option<Faction> or Faction depending on inferred nullability
• AS "faction: Faction!" - emits Faction always
• AS "faction: Faction?" - emits Option<Faction> always

This is an extension of what we already have:

• AS "faction" - emits the DB type ( if possible ) with inferred nullability
• AS "faction?" - emits Option<DBType>
• AS "faction!" - emits DBType

---

After I wrote the above I realize that "faction: Faction!" might be hard to parse as you want to just hand the right side over as a type path. Perhaps instead it could be "faction!: Faction" and "faction?: Faction".

[phated]

@mehcode Yeah, I think that is what I'd expect and I like your edit suggestion because it matches the stuff that's already there.

[mehcode]

Resolution was "yes" and implemented by @raftario, thank you :)