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

Compilation error on get_result #227

Closed
jimmycuadra opened this Issue Feb 26, 2016 · 10 comments

Comments

Projects
None yet
2 participants
@jimmycuadra
Contributor

jimmycuadra commented Feb 26, 2016

I'm getting a compilation error on nightly Rust (rustc 1.8.0-nightly (0ef8d4260 2016-02-24) / cargo 0.9.0-nightly (c91bf61 2016-02-24)) from calling get_result on an insert statement. Here is the error:

$ cargo build
   Compiling ruma v0.1.0 (file:///Users/jimmy/Code/ruma)
src/access_token.rs:46:10: 46:20 error: no method named `get_result` found for type `diesel::query_builder::insert_statement::InsertStatement<schema::access_tokens::table, access_token::NewAccessToken>` in the current scope
src/access_token.rs:46         .get_result(connection)
                                ^~~~~~~~~~
src/access_token.rs:46:10: 46:20 note: the method `get_result` exists but the following trait bounds were not satisfied: `diesel::query_builder::insert_statement::InsertQuery<(schema::access_tokens::columns::id, schema::access_tokens::columns::user_id, schema::access_tokens::columns::value, schema::access_tokens::columns::created_at), diesel::query_builder::insert_statement::InsertStatement<schema::access_tokens::table, access_token::NewAccessToken>> : diesel::query_builder::QueryFragment<_>`, `&diesel::query_builder::insert_statement::InsertStatement<schema::access_tokens::table, access_token::NewAccessToken> : diesel::query_builder::AsQuery`, `_ : diesel::query_builder::QueryFragment<_>`, `&diesel::query_builder::insert_statement::InsertStatement<schema::access_tokens::table, access_token::NewAccessToken> : diesel::query_builder::Query`, `&diesel::query_builder::insert_statement::InsertStatement<schema::access_tokens::table, access_token::NewAccessToken> : diesel::query_builder::Query`, `diesel::query_builder::insert_statement::InsertStatement<schema::access_tokens::table, access_token::NewAccessToken> : diesel::query_builder::Query`, `diesel::query_builder::insert_statement::InsertStatement<schema::access_tokens::table, access_token::NewAccessToken> : diesel::query_builder::Query`, `&mut diesel::query_builder::insert_statement::InsertStatement<schema::access_tokens::table, access_token::NewAccessToken> : diesel::query_builder::AsQuery`, `_ : diesel::query_builder::QueryFragment<_>`, `&mut diesel::query_builder::insert_statement::InsertStatement<schema::access_tokens::table, access_token::NewAccessToken> : diesel::query_builder::Query`, `&mut diesel::query_builder::insert_statement::InsertStatement<schema::access_tokens::table, access_token::NewAccessToken> : diesel::query_builder::Query`, `&mut diesel::query_builder::insert_statement::InsertStatement<schema::access_tokens::table, access_token::NewAccessToken> : diesel::query_builder::Query`
error: aborting due to previous error
Could not compile `ruma`.

To learn more, run the command again with --verbose.

I tried to create a reduced test case but I can't get it to fail in other scenarios. So here is the exact code that generated the above error: https://github.com/ruma/ruma/tree/diesel-error

You'll need a local copy of Diesel checked out at HEAD in a sibling directory (see .cargo/config).

I know there's a lot of code that's probably not related to wade through in Ruma, so here are what I think are the relevant details:

  • The database tables are defined using table! in src/schema.rs.
  • The R2D2 connection pool is created in src/server.rs.
  • The code path that invokes all of this is in src/api/r0/registration.rs.
  • The helper for pulling a connection out of the pool is in src/db.rs.
  • The two Diesel models are in src/user.rs and src/access_token.rs.

Let me know what other info I can provide to help narrow this down!

@sgrif

This comment has been minimized.

Member

sgrif commented Mar 5, 2016

Sorry for taking so long to reply here. I've looked through your code, and nothing looks immediately wrong to me. Is it possible to replicate this issue with less code? Sorry for the troubles that you've been having. Improving our error messages for cases like this is definitely on the radar.

@jimmycuadra

This comment has been minimized.

Contributor

jimmycuadra commented Mar 6, 2016

I'll see if I can narrow it down some more and get a simpler reproduction. Thanks for taking a look!

@sgrif

This comment has been minimized.

Member

sgrif commented Mar 13, 2016

@jimmycuadra Were you able to pare this down at all? I tried to build your example, but there's conflicts with quasi (presumably due to serde versions not being updated for the latest nightly or something). If you can just pull out the diesel code into something I can use to reproduce, that'd be helpful. But I can't build your entire app.

@jimmycuadra

This comment has been minimized.

Contributor

jimmycuadra commented Mar 13, 2016

Sorry, no, I haven't checked it again since first opening the issue. I've been knee deep in other projects. You can let it sit for now and I will leave a comment as soon as I update the branch and verify it's still a problem and/or figure out a smaller reproducible test case.

@sgrif

This comment has been minimized.

Member

sgrif commented Mar 13, 2016

Thanks for the update. Let me know if there's anything else I can do to help.

@jimmycuadra

This comment has been minimized.

Contributor

jimmycuadra commented Mar 31, 2016

I got a chance to look at this again and was able to reproduce the error without so much set up and unrelated code. Take a look here: https://github.com/jimmycuadra/diesel_error

@sgrif

This comment has been minimized.

Member

sgrif commented Mar 31, 2016

Thanks, I'll try to fix this today.

On Thu, Mar 31, 2016, 1:40 AM Jimmy Cuadra notifications@github.com wrote:

I got a chance to look at this again and was able to reproduce the error
without so much set up and unrelated code. Take a look here:
https://github.com/jimmycuadra/diesel_error


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#227 (comment)

@sgrif

This comment has been minimized.

Member

sgrif commented Mar 31, 2016

I should have spotted this originally. The issue is that insert(new_access_token) should be insert(&new_access_token). If you make that change, the code behaves as expected. (It still doesn't compile because FromSql<diesel::types::BigInt> isn't implemented for num::bigint::BigInt, but that field should be an i64 anyway because a big integer in PG is just 64 bits, not infinite).

That said, there's no reason you would be expected to figure out the problem in your code from the error message given. We either need to improve the error message here, or just have this case work. I've opened a new issue for that in #249.

Thanks for the report, sorry I didn't spot the error initially.

@sgrif sgrif closed this Mar 31, 2016

@jimmycuadra

This comment has been minimized.

Contributor

jimmycuadra commented Mar 31, 2016

Ah ha! Glad it was simple as that! Thanks so much.

@sgrif

This comment has been minimized.

Member

sgrif commented Mar 31, 2016

No problem. The result ended up being a big win for our error messages.

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