# Proposed change: Use an opaque struct instead of \`\[u8\]\` for \`Mysql::RawValue\` and \`Pg::RawValue\`

**URL:** <https://discourse.diesel.rs/t/proposed-change-use-an-opaque-struct-instead-of-u8-for-mysql-rawvalue-and-pg-rawvalue/15>\
**Category:** Diesel 2.0\
**Created:** [June 7, 2018, 9:20pm UTC](https://discourse.diesel.rs/t/proposed-change-use-an-opaque-struct-instead-of-u8-for-mysql-rawvalue-and-pg-rawvalue/15 "2018-06-07T21:20:58Z")\
**Posts on this page:** 3\
**Page:** 1

<div class="post-metadata">

**Author:** ![sean](https://yyz2.discourse-cdn.com/flex036/user_avatar/discourse.diesel.rs/sean/32/2_2.png) [@sean](https://discourse.diesel.rs/u/sean)\
**Post date:** [June 7, 2018, 9:20pm UTC](https://discourse.diesel.rs/t/proposed-change-use-an-opaque-struct-instead-of-u8-for-mysql-rawvalue-and-pg-rawvalue/15/1 "2018-06-07T21:20:58Z")

</div>

# Description of the change

- `<Mysql as Backend>::RawValue` has been changed from `[u8]` to `MysqlValue`. You can access the underlying bytes by calling `.bytes()` or `.as_ref()`.
- `<Pg as Backend>::RawValue` has been changed from `[u8]` to `PgValue`. You can access the underlying bytes by calling `.bytes()` or `.as_ref()`.

# Why is this needed?

The first part of this is future proofing. For both PG and MySQL, we’ve seen cases where it would be useful to include a type identifier along with the bytes. This is more pressing for MySQL, but for both cases this is to ensure we can provide additional information to `FromSql` implementations in the future.

For MySQL, this will allow us to decouple from libmysqlclient. Currently we rely on libmysqlclient handling type coercion for us more than we intended to. A big reason for this is that MySQL has different semantics for many operations than other backends (e.g. 32 bit integer addition returns a 64 bit integer).

This results in the behavior of `sql_query` differing from using the normal query builder, as we don’t find out the expected type until after libmysqlclient has finished doing its thing. Instead we should make deserializing numbers on MySQL more permissive, and work with an enum for every possible numeric representation.

For PG, the motivation is less directly needed. However, it would be nice to provide the OID in the future both for strict type checking, and to allow some sort of handling of dynamic values.

# Why can’t this have a deprecation cycle?

Since this affects the concrete value of an associated type, the only way we could deprecate this is by introducing a `Mysql2` backend and `Pg2` backend, deprecating the originals. This does the same thing is releasing Diesel 2.0, but it would break all code using mysql or PG, instead of just the code affected by this change.

# Expected user impact

Code which is affected by this will take one of these three forms:

```auto
impl FromSql<Mysql> for MyType {
    fn from_sql(bytes: Option<&[u8]>) -> deserialize::Result<Self> {
        // ...
    }
}

impl FromSql<Pg> for MyType {
    fn from_sql(bytes: Option<&[u8]>) -> deserialize::Result<Self> {
        // ...
    }
}

impl<DB: Backend<RawValue = [u8]> FromSql<DB> for MyType {
    fn from_sql(bytes: Option<&[u8]>) -> deserialize::Result<Self> {
        // ...
    }
}

```

For MySQL, the expected impact is low. Since that backend has a fairly defined set of types with no options for extension, there is little reason to write a custom `FromSql` implementation. There may be some users doing this for enums, but support for those can be done without relying on implementation details already.

For PG the expected impact is moderate. Applications are likely only affected if they use PG enums ([crates.io](http://crates.io) is impacted in one place). Any crate which adds support for new PG types will be affected, such as hstore or postgis. `diesel_full_text_search` would not be impacted.

It is unlikely there is any code using `DB: Backend<RawValue = [u8]>` outside of Diesel itself.

# Migration strategy

On MySQL, the argument type will need to be changed to `Option<&MysqlValue>`. The function body should not be affected, since any `FromSql` implementation will likely be defined in terms of existing implementations.

On PG, the argument type will need to be changed to `Option<&PgValue>`. Any usage of the bytes in the function body that actually expects a `&[u8]` (e.g. doing something other than passing it to `FromSql::from_sql` for another type) will need to either call `.bytes` or `.as_ref`.

Any implementations for `DB: Backend<RawValue = [u8]>` will need to be changed to `DB: Backend, Backend::RawValue: AsRef<u8>`. The function body will need to use `.as_ref` in all places the bytes were being used.

In all cases, code can support both Diesel 1.0 and 2.0 by using `.as_ref`, since `[u8]: AsRef<[u8]>`.

---

<div class="post-metadata">

**Author:** ![sean](https://yyz2.discourse-cdn.com/flex036/user_avatar/discourse.diesel.rs/sean/32/2_2.png) [@sean](https://discourse.diesel.rs/u/sean)\
**Post date:** [June 27, 2018, 8:14pm UTC](https://discourse.diesel.rs/t/proposed-change-use-an-opaque-struct-instead-of-u8-for-mysql-rawvalue-and-pg-rawvalue/15/2 "2018-06-27T20:14:06Z")

</div>

It actually looks like we won’t be able to change implementations for `Backend<RawValue = [u8]>` to `Backend::RawValue: AsRef<[u8]>`. For some reason it ends up conflicting with SQLite

---

<div class="post-metadata">

**Author:** ![Thomspoon](https://avatars.discourse-cdn.com/v4/letter/t/13edae/32.png) [@Thomspoon](https://discourse.diesel.rs/u/Thomspoon)\
**Post date:** [September 4, 2018, 12:35am UTC](https://discourse.diesel.rs/t/proposed-change-use-an-opaque-struct-instead-of-u8-for-mysql-rawvalue-and-pg-rawvalue/15/3 "2018-09-04T00:35:57Z")

</div>

I’ve started migrating to this opaque struct for my own uses, and I found everything works perfectly until we get to PgResult. In PgResult::get we have this code:

```auto
    pub fn get(&self, row_idx: usize, col_idx: usize) -> Option<&PgValue> {
        if self.is_null(row_idx, col_idx) {
            None
        } else {
            let row_idx = row_idx as libc::c_int;
            let col_idx = col_idx as libc::c_int;
            unsafe {
                let value_ptr =
                    PQgetvalue(self.internal_result.as_ptr(), row_idx, col_idx) as *const u8;
                let num_bytes = PQgetlength(self.internal_result.as_ptr(), row_idx, col_idx);
                Some(&PgValue {
                    data: slice::from_raw_parts(value_ptr, num_bytes as usize).to_vec(),
                    oid: 1, //TODO
                })
            }
        }
    }

```

however, the PgValue obviously doesn’t live long enough. Unless there is a better solution, would a good alternative be to add the PgValue to the PgResult as an intermediate value in the struct?

```auto
pub struct PgResult {
    internal_result: RawResult,
    intermediate_value: PgValue,
}

```

Which would change as the result is read?

Otherwise I guess we could construct a new PgValue at the Row impl and give it Copy/Clone so it gets copied upstream?
