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

How can I store `chrono::Duration` in the sqlite 3 database with diesel? #1542

Closed
vityafx opened this Issue Feb 6, 2018 · 14 comments

Comments

Projects
None yet
3 participants
@vityafx

vityafx commented Feb 6, 2018

I wanna have a duration field in my table. However, I don't know what type to use for it, because, looking at chrono types implementation for sqlite I think it is impossible.
According to the [chrono::Duration documentation](https://docs.rs/chrono/0.4.0/chrono
/struct.Duration.html), it can be stored as BIGINTEGER in the sqlite3 table, since it is:

ISO 8601 time duration with nanosecond precision. This also allows for the negative duration; see individual methods for details.

So, my guess is to implement that via storing in sqlite3 table with BIGINTEGER type and implementing FromSql and ToSql traits for the chrono::Duration by calling the chrono::Duration::nanoseconds() static method.

@weiznich

This comment has been minimized.

Contributor

weiznich commented Feb 6, 2018

You will need to add a new type wrapper for the chrono::Duration type. Derive AsExpression and FromSqlRow for that type. After that you need to implement ToSql and FromSql for your new type in respect to BIGINTEGER.
See this testcase as an example for supporting custom rust types.

@vityafx

This comment has been minimized.

vityafx commented Feb 6, 2018

@weiznich Can't I simply implement FromSql and Tosql for the chrono::Duration as it is done for the NaiveDateTime, NaiveDate and NaiveTime structures?

@weiznich

This comment has been minimized.

Contributor

weiznich commented Feb 6, 2018

No you can't because you own neither chrono::Duration nor FromSql/ToSql

@vityafx

This comment has been minimized.

vityafx commented Feb 6, 2018

Oh, well, I was not clear enough :) I wanted this to be done in the diesel itself if it is not. I think it is not, especially for v.1.1.1, so I did my own implementation.

@vityafx

This comment has been minimized.

vityafx commented Feb 6, 2018

@weiznich Could you help me with implementing this in my fork? It seems to me it does not work even so:

models.rs:6:35
  |
6 | #[derive(Debug, Clone, Queryable, Insertable)]
  |                                   ^^^^^^^^^^ the trait `diesel::Expression` is not implemented for `chrono::Duration`
  |
  = note: required because of the requirements on the impl of `diesel::Expression` for `&chrono::Duration`
  = note: required because of the requirements on the impl of `diesel::expression::AsExpression<diesel::sql_types::BigInt>` for `&chrono::Duration`

How can I implement it? I thought this way enough:

impl FromSql<BigInt, Sqlite> for chrono::Duration {
    fn from_sql(value: Option<&<Sqlite as Backend>::RawValue>) -> deserialize::Result<Self> {
        let i64_value = <i64 as FromSql<BigInt, Sqlite>>::from_sql(value)?;
        Ok(chrono::Duration::nanoseconds(i64_value))
    }
}

impl ToSql<BigInt, Sqlite> for chrono::Duration {
    fn to_sql<W: Write>(&self, out: &mut Output<W, Sqlite>) -> serialize::Result {
        if let Some(num_nanoseconds) = self.num_nanoseconds() {
            ToSql::<BigInt, Sqlite>::to_sql(&num_nanoseconds, out)
        } else {
            Err(format!("{:?} as nanoseconds is too larg to fit in an i64", self).into())
        }
    }
}
@weiznich

This comment has been minimized.

Contributor

weiznich commented Feb 6, 2018

You need to add impl for AsExpression for &'a chrono::Duration similar to that ones in this file

(I'm not sure if chrono::Duration will be accepted as a supported type for sqlite.)

@vityafx

This comment has been minimized.

vityafx commented Feb 6, 2018

@weiznich looks like it worked, thanks.

@sgrif

This comment has been minimized.

Member

sgrif commented Feb 6, 2018

I don't think we would support duration support for SQLite.

@sgrif sgrif closed this Feb 6, 2018

@vityafx

This comment has been minimized.

vityafx commented Feb 7, 2018

@sgrif could you explain please? Why not?

@sgrif

This comment has been minimized.

Member

sgrif commented Feb 7, 2018

We don't typically support types that don't have a direct database mapping to a SQL type. Even if we did, there's no obvious way to store it, and you wouldn't be able to do anything useful with it.

@vityafx

This comment has been minimized.

vityafx commented Feb 7, 2018

@sgrif This is sad. I have implemented chrono::Duration for both postgresql and sqlite backends, all tests are passed and everything works good. the chrono::Duration type is nothing more but just i64 value with nanoseconds, so it absolutely fits into the BigInteger types of SQL which is exactly 8 bytes for both postgresql and sqlite.

@vityafx

This comment has been minimized.

vityafx commented Feb 15, 2018

@sgrif So, what is wrong with Duration exactly, since it is just a number which can be stored perfectly fine? You still think it is so bad to have an implementation for it?

@sgrif

This comment has been minimized.

Member

sgrif commented Feb 15, 2018

SQLite has no corresponding interval type to store it as. I'd expect now + Duration to work if we supported durations. It wouldn't.

@vityafx

This comment has been minimized.

vityafx commented Feb 16, 2018

@sgrif what do you think about that? https://github.com/vityafx/diesel-chrono-duration

Could you review it please? :-[

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment