SupportsReturningClause mysql

[zhiburt]

Problem Description

Following the get started guide step by step I was willing to run insert code with mysql.

        // ...
        let inserted = diesel::insert_into(entities::table)
            .values(&ext)
            .get_result::<Entity>(&conn)
            .unwrap();

But I was constantly getting a compile error.

the trait bound `diesel::mysql::Mysql: diesel::backend::SupportsReturningClause` is not satisfied
required because of the requirements on the impl of `diesel::query_builder::QueryFragment<diesel::mysql::Mysql>` for `diesel::query_builder::returning_clause::ReturningClause<schema::extension::columns::id>`
required because of the requirements on the impl of `diesel::query_builder::QueryFragment<diesel::mysql::Mysql>` for 
...

And eventually (after figuring out that execute doesn't return id 😄 ) I took a look at the doc and it seems to me currently it's impossible to run such code with mysql since its supported only by postgress right?

https://docs.diesel.rs/diesel/backend/trait.SupportsReturningClause.html

What are you trying to accomplish?

If it's true I would consider adding some information somewhere that it's not work with mysql could reduce peoples time spent on trying to resolve it. Probably in get_result doc comment.

Comment

I may be incorrect in my assumptions in which case I am sorry 😞

And thank you for the crate.

[thecooldrop]

I would like to take this issue as first contribution to Diesel.

May I ask you @weiznich to assign it to me?

[weiznich]

@thecooldrop Feel free to submit a PR for this. I should probably add that I don't think that get_result is the right place for this as get_result is available for queries using the mysql backend, just not for insert statements.

[thecooldrop]

@weiznich Your comment is really confusing me since according to my analysis, given that MySql does not implement SupportsReturningClause we should not be able to have get_results for any of the statement types with MySql backend. Here is why:

1. RunQueryDsl::get_result<U>(self, conn: &Conn ) -> QueryResult<U> is defined only for those implementors of the trait which also implement the LoadQuery<Conn,U>
2. LoadQuery<Conn,U> is implemented only for those types which implement AsQuery whose associated AsQuery::Query associated type implements the QueryFragment<DB: Backend>.
3. The structs InsertStatement, UpdateStatement and DeleteStatement have AsQuery implementations, which have ReturningClause<T::AllColumns> as Ret type. ( when used exactly as in case above, other Ret types arise if calls to returning are placed between the query build and execution of query )
4. The QueryFragment<DB: Backend> is implemented only only for those statements whose Ret type parameters are themselves QueryFragment<DB: Backend> implementors
5. Due to (4) for QueryFragment<DB: Backend> to be implemented for the ReturningClause<Expr> the associated backend has to implement the SupportsReturningClause, which is only implemented for Pg, thus it can not be applicable to any of those statement types for any other backend.

This can be further complicated and nested if type parameter of 'Ret' of given statement type is again merely implementation of QueryFragment, where recursion continues down until we reach a Ret type whose type parameter is merely an expression.

Given the argumentation above, I would agree that complete documentation does not belong into RunQueryDsl::get_result<U> method, but a simple remark saying that this method is applicable only to those backends implementing the SupportsReturningClause may be appropriate. Further a link to SupportsReturningStatement might be included to lead the readers, where further more detailed documentation can be placed.

I must add tho that contra argument to adding this would be an implication, that such comment would have to be added to all making use of RETURNING ... SQL statements, which would not be efficient. Compile error message does clearly indicate that method is not callable because the SupportsReturningClause is not implemented for a specified backend.

Given the two ways of decision above I would rather pursue the second one.

[sgrif]

Just to confirm, get_result should not be implemented for insert queries on MySQL. As the compile error points out, MySQL provides no mechanism to perform an insert and get the resulting row in a single query (which is done using a RETURNING clause on PG).

[thecooldrop]

@sgrif It seems like the get_result method should not be implemented for update or delete statements for MySQL, or by extension of my argumentation above, for any backend other than Postgres. Did I miss something?

[sgrif]

Nope, you didn't miss anything. PostgreSQL is the only driver we support by default for which we can reasonably implement this method. I'm just using language which excludes MySQL instead of language which specifies PostgreSQL since there are other SQL databases for which this could be implemented, that Diesel doesn't support out of the box, but for which a third party crate could provide a driver

[WildEgo]

Nope, you didn't miss anything. PostgreSQL is the only driver we support by default for which we can reasonably implement this method. I'm just using language which excludes MySQL instead of language which specifies PostgreSQL since there are other SQL databases for which this could be implemented, that Diesel doesn't support out of the box, but for which a third party crate could provide a driver

Is there any guide on how to make a driver for such? I'm currently using MariaDB and it does kinda work but I lose support for get_result, if there are any guides I would gladly setup something even if just so I can temporarily use it.

[weiznich]

@WildEgo Unfortunately we do not provide a guide how to implement a third party backend. I can outline some general steps you can follow to implement such a backend, but be warned that this is likely no easy task.
Basically you "only" need to provide types that implement Connection and Backend + implement support for serialization and deserialization for a bunch of types. For a mariadb backend you can likely start from the existing mysql backend.
Additionally if you are really interested on working on this have a look on #1882. Contributions there are welcome and I will try to find time to provide some mentoring for that.

[WildEgo]

@WildEgo Unfortunately we do not provide a guide how to implement a third party backend. I can outline some general steps you can follow to implement such a backend, but be warned that this is likely no easy task. Basically you "only" need to provide types that implement Connection and Backend + implement support for serialization and deserialization for a bunch of types. For a mariadb backend you can likely start from the existing mysql backend. Additionally if you are really interested on working on this have a look on #1882. Contributions there are welcome and I will try to find time to provide some mentoring for that.

If you can get me a bit of guiding light on how to test it in real time and stuff I'll gladly try to do it even tho I'm kinda just out of reading the basic guide on rust, if I can make it it's good experience and so it is if I don't

[weiznich]

@WildEgo Have a look at the CONTRIBUTING.md document. It contains quite a lot details about how diesel is setup.
If you really plan working on this I suggest to open a new discussion thread to start discussing a general strategy there and as soon as there are actual code changes open a WIP PR, to discuss the exact changes there.

[RfDzDeveloper]

Hi all,
do you planing solve problem with .get_result() related to MariaDB/MySql? It's not a big thing, but it will be really helpful.

diesel::insert_into(posts::table) .values(&new_post) .get_result::<Post>(connection) .expect("Error saving new post");

[weiznich]

@RfDzDeveloper As already written above: For the Mysql backend that's impossible to solve due to restrictions of the SQL implementation. For MariaDB we would need a new backend to support this. We would accept contributions there, but we as maintainer team do not plan to implement that in the near future. Repeating these kind of questions does not chaneg the answers.

[weiznich]

@alexwatever Please do not ping people to ask your questions. Otherwise: Feel free to open a PR against the websites to change this guide. I personally feel that it is already pretty clear that the getting started guide uses postgresql as database system.

[alexwatever]

@alexwatever Please do not ping people to ask your questions. Otherwise: Feel free to open a PR against the websites to change this guide. I personally feel that it is already pretty clear that the getting started guide uses postgresql as database system.

@weiznich Fair enough, I apologise. I've removed my comment.