sqlx icon indicating copy to clipboard operation
sqlx copied to clipboard

Implement `FromRow` for `Option<NonZero*>`?

Open GnomedDev opened this issue 3 years ago • 2 comments

I've got some columns which are never 0, so it would be useful to implement FromRow for Option<NonZero*>, where a zero value returns None. This allows for niche optimizations and other nice things for Option<NonZero*>

GnomedDev avatar Jun 23 '22 11:06 GnomedDev

For one, it wouldn't be FromRow as we're talking about scalar values here, so it would be implementing Type, Decode and maybe Encode.

Secondly, I don't think we would want just Option<NonZero*> as that begs the question of why isn't NonZero* also supported? But we can't have specialized behavior for both because of the blanket impls for Option<T> which decode NULL as None.

There's also the question of if we do support just Option<NonZero*>, what do we encode None as when binding? NULL or 0? I think it depends entirely on the application.

In general, weird bugs can happen when programs treat NULL and 0 as equivalent, and I'd like to avoid handing out footguns like that.

If we did support this, I would think we should implement Type, Encode and Decode for NonZero* and return error if decoding the value 0. It is a logic error, after all. Then Option<NonZero*> returns None for NULL, behaving as normal.

You'd then choose an appropriate type for your situation:

  • If the column may never be NULL or 0, use NonZero*.
    • This would allow the containing type to utilize niche-filling optimizations, e.g. Option<Foo> being the same size as Foo where Foo contains a NonZero*.
  • If the column can be NULL but not 0, use Option<NonZero*>
    • This should be the same size as the equivalent integer type but will not enable niche-filling for the containing type as the niche is already being used by that Option.
    • For sanity, the column should also have a CHECK(foo IS DISTINCT FROM 0) constraint.
  • If the column can be NULL or 0, just use Option<{i16, i32, i64}>.
    • Or, use NULLIF(<column>, 0) in your query with Option<NonZero*> as the decoding type so the choice of using NULL instead of 0 is explicit.

Preferring to send NULL over the wire instead of 0 actually saves bandwidth for MSSQL, MySQL and Postgres as NULL values are encoded out-of-line.

abonander avatar Jun 23 '22 22:06 abonander

Sure, second option sounds best for me, I could probably get some use out of it, just requires NonZero* to implement the relevant traits

GnomedDev avatar Jun 23 '22 23:06 GnomedDev

Is there any plan about implementing this? I could make a PR for it if nobody else wants to do it, since I personally also want that feature.

AlphaKeks avatar Feb 19 '24 23:02 AlphaKeks