/diesel

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

Provide a `FromSql` impl for `*const str` and `*const [u8]` #1365

Closed
sgrif opened this Issue Dec 9, 2017 · 0 comments

Comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
  1.1
1 participant
@sgrif
@sgrif
Member

sgrif commented Dec 9, 2017

There are various impls that compose on the FromSql impls for String and Vec<u8>, but do not need an owned string/vec. We cannot safely provide an impl for &str, or &[u8]. The only safe impl we could provide would be one where the return value lasts as long as the argument to from_sql. This would only ever be useful for composed implementations, but providing that safe impl would require restructuring FromSql to have a lifetime attached, which I really don't want to do.

Though we can't represent the lifetime of this reference with the current structure of FromSql, we can absolutely provide an impl for raw pointers, documenting how long they last.

@sgrif sgrif added this to the 1.1 milestone Dec 9, 2017

sgrif added a commit that referenced this issue Jan 7, 2018

@sgrif
Provide `FromSql` impls for `*const str` and `*const [u8]` 
There are several impls which could be composed on impls for `&str` and
`&[u8]` if they were available. Unfortunately, providing those impls is
impossible in Diesel 1.0. We would need a lifetime on `FromSql`,
changing the signature to this:

```rust
trait FromSql<'a, ST, DB> {
    fn from_sql(bytes: Option<&'a DB::RawValue>) -> ...
}
```

While we can't provide impls for `&str` and `&[u8]`, we can provide
impls for `*const` versions of both. The docs about how long the pointer
is valid for *do* get rendered, and since dereferencing this pointers
requires unsafe code, we can reasonably assume that anyone who uses
these impls has looked at the docs for the assumed invaraints.

I've updated several of our built-in impls that just do string parsing
to use these impls, to demonstrate why they're useful. Keep in mind that
outside of Diesel it is impossible to reference `SqliteValue`. While the
impls I've updated are backend specific, it is also possible to use
these impls to write backend agnostic implementations.

Fixes #1365.
862ffde

@sgrif sgrif referenced this issue Jan 7, 2018

Merged

Provide `FromSql` impls for `*const str` and `*const [u8]` #1458

sgrif added a commit that referenced this issue Jan 13, 2018

@sgrif
Provide `FromSql` impls for `*const str` and `*const [u8]` 
There are several impls which could be composed on impls for `&str` and
`&[u8]` if they were available. Unfortunately, providing those impls is
impossible in Diesel 1.0. We would need a lifetime on `FromSql`,
changing the signature to this:

```rust
trait FromSql<'a, ST, DB> {
    fn from_sql(bytes: Option<&'a DB::RawValue>) -> ...
}
```

While we can't provide impls for `&str` and `&[u8]`, we can provide
impls for `*const` versions of both. The docs about how long the pointer
is valid for *do* get rendered, and since dereferencing this pointers
requires unsafe code, we can reasonably assume that anyone who uses
these impls has looked at the docs for the assumed invaraints.

I've updated several of our built-in impls that just do string parsing
to use these impls, to demonstrate why they're useful. Keep in mind that
outside of Diesel it is impossible to reference `SqliteValue`. While the
impls I've updated are backend specific, it is also possible to use
these impls to write backend agnostic implementations.

Fixes #1365.
ec7a80d

sgrif added a commit that referenced this issue Jan 13, 2018

@sgrif
Provide `FromSql` impls for `*const str` and `*const [u8]` 
There are several impls which could be composed on impls for `&str` and
`&[u8]` if they were available. Unfortunately, providing those impls is
impossible in Diesel 1.0. We would need a lifetime on `FromSql`,
changing the signature to this:

```rust
trait FromSql<'a, ST, DB> {
    fn from_sql(bytes: Option<&'a DB::RawValue>) -> ...
}
```

While we can't provide impls for `&str` and `&[u8]`, we can provide
impls for `*const` versions of both. The docs about how long the pointer
is valid for *do* get rendered, and since dereferencing this pointers
requires unsafe code, we can reasonably assume that anyone who uses
these impls has looked at the docs for the assumed invaraints.

I've updated several of our built-in impls that just do string parsing
to use these impls, to demonstrate why they're useful. Keep in mind that
outside of Diesel it is impossible to reference `SqliteValue`. While the
impls I've updated are backend specific, it is also possible to use
these impls to write backend agnostic implementations.

Fixes #1365.
2e7e206

sgrif added a commit that referenced this issue Jan 13, 2018

@sgrif
Provide `FromSql` impls for `*const str` and `*const [u8]` 
There are several impls which could be composed on impls for `&str` and
`&[u8]` if they were available. Unfortunately, providing those impls is
impossible in Diesel 1.0. We would need a lifetime on `FromSql`,
changing the signature to this:

```rust
trait FromSql<'a, ST, DB> {
    fn from_sql(bytes: Option<&'a DB::RawValue>) -> ...
}
```

While we can't provide impls for `&str` and `&[u8]`, we can provide
impls for `*const` versions of both. The docs about how long the pointer
is valid for *do* get rendered, and since dereferencing this pointers
requires unsafe code, we can reasonably assume that anyone who uses
these impls has looked at the docs for the assumed invaraints.

I've updated several of our built-in impls that just do string parsing
to use these impls, to demonstrate why they're useful. Keep in mind that
outside of Diesel it is impossible to reference `SqliteValue`. While the
impls I've updated are backend specific, it is also possible to use
these impls to write backend agnostic implementations.

Fixes #1365.
1ba7985

sgrif added a commit that referenced this issue Jan 15, 2018

@sgrif
Provide `FromSql` impls for `*const str` and `*const [u8]` 
There are several impls which could be composed on impls for `&str` and
`&[u8]` if they were available. Unfortunately, providing those impls is
impossible in Diesel 1.0. We would need a lifetime on `FromSql`,
changing the signature to this:

```rust
trait FromSql<'a, ST, DB> {
    fn from_sql(bytes: Option<&'a DB::RawValue>) -> ...
}
```

While we can't provide impls for `&str` and `&[u8]`, we can provide
impls for `*const` versions of both. The docs about how long the pointer
is valid for *do* get rendered, and since dereferencing this pointers
requires unsafe code, we can reasonably assume that anyone who uses
these impls has looked at the docs for the assumed invaraints.

I've updated several of our built-in impls that just do string parsing
to use these impls, to demonstrate why they're useful. Keep in mind that
outside of Diesel it is impossible to reference `SqliteValue`. While the
impls I've updated are backend specific, it is also possible to use
these impls to write backend agnostic implementations.

Fixes #1365.
4204435

sgrif added a commit that referenced this issue Jan 15, 2018

@sgrif
Provide `FromSql` impls for `*const str` and `*const [u8]` 
There are several impls which could be composed on impls for `&str` and
`&[u8]` if they were available. Unfortunately, providing those impls is
impossible in Diesel 1.0. We would need a lifetime on `FromSql`,
changing the signature to this:

```rust
trait FromSql<'a, ST, DB> {
    fn from_sql(bytes: Option<&'a DB::RawValue>) -> ...
}
```

While we can't provide impls for `&str` and `&[u8]`, we can provide
impls for `*const` versions of both. The docs about how long the pointer
is valid for *do* get rendered, and since dereferencing this pointers
requires unsafe code, we can reasonably assume that anyone who uses
these impls has looked at the docs for the assumed invaraints.

I've updated several of our built-in impls that just do string parsing
to use these impls, to demonstrate why they're useful. Keep in mind that
outside of Diesel it is impossible to reference `SqliteValue`. While the
impls I've updated are backend specific, it is also possible to use
these impls to write backend agnostic implementations.

Fixes #1365.
48ce4ba

sgrif added a commit that referenced this issue Jan 15, 2018

@sgrif
Provide `FromSql` impls for `*const str` and `*const [u8]` 
There are several impls which could be composed on impls for `&str` and
`&[u8]` if they were available. Unfortunately, providing those impls is
impossible in Diesel 1.0. We would need a lifetime on `FromSql`,
changing the signature to this:

```rust
trait FromSql<'a, ST, DB> {
    fn from_sql(bytes: Option<&'a DB::RawValue>) -> ...
}
```

While we can't provide impls for `&str` and `&[u8]`, we can provide
impls for `*const` versions of both. The docs about how long the pointer
is valid for *do* get rendered, and since dereferencing this pointers
requires unsafe code, we can reasonably assume that anyone who uses
these impls has looked at the docs for the assumed invaraints.

I've updated several of our built-in impls that just do string parsing
to use these impls, to demonstrate why they're useful. Keep in mind that
outside of Diesel it is impossible to reference `SqliteValue`. While the
impls I've updated are backend specific, it is also possible to use
these impls to write backend agnostic implementations.

Fixes #1365.
Loading status checks…
c56a162

@sgrif sgrif closed this in #1458 Jan 15, 2018

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