Examine our compiler errors, and see if we can improve them

[sgrif]

At times, our compiler errors just flat out suck. While in this issue I will list several cases that I've specifically noticed are frequently problematic or have personally encountered, it should not be treated as exhaustive. If you've ever received a non-helpful compiler message from this lib, please comment with details. I do not think we will be able to fix all of them, but we should try to improve this issue before 1.0.

Truly "fixing" this will probably require significantly more control over the "trait not implemented" error, both on the trait itself (e.g. we can give a much better message for any case where SelectableExpression isn't implemented), and also at the function level (e.g. when find doesn't work, we can probably be more specific, even though none of the traits failing are specific to find).

As we attempt to make sure we create good error messages, we should update our compile-fail suite to be significantly more specific about the error messages we generate. While that will make those tests much more brittle, I'd rather have to update them frequently (and thus ensure that the new error message is still reasonable), than accidentally start generating a nonsense error for a common case.

Problem Cases

Recommending Expression instead of AsExpression

A consistent source of confusion is rustc's recommendation to implement Expression, NonAggregate, and QueryFragment, when the missing piece is actually AsExpression. Ultimately this is due to rust-lang/rust#28894. If we can't fix that issue in the language, we should probably just remove that blanket impl and brute force it instead (I'm honestly surprised the coherence rules haven't forced this already, I think this is the only blanket impl that's survived).

Some types are too long to ever appear in error messages

Another case that we should look for is any case where SelectStatement or FilterQuerySource end up in the final error message. It doesn't matter what trait is missing, the error message SelectStatement<(type1, type2, type3, 10moretypes), (col1, col2, col3, 10morecolumns), InnerJoinQuerySource<left_table, right_table>, NoWhereClause, NoOrderClause, NoLimitClause, NoOffsetClause> does not implement AsQuery will always be too noisy to be helpful. That specific error is actually one of the simpler cases, and even with that exact case, every type would have the fully qualified path, and be repeated 3 times (saying it looked at SelectStatement, &SelectStatement, and &mut SelectStatement). It's not uncommon for that kind of error message to take up the entire terminal window.

[jgillich]

Yes, please. Here is one that drives me crazy:

src/models/artist.rs:17:61: 17:66 error: the trait `diesel::types::FromSqlRow<diesel::types::Integer, diesel::pg::backend::Pg>` is not implemented for the type `collections::string::String` [E0277]

src/models/artist.rs:17         match artists::table.filter(artists::name.eq(name)).first(conn) {

                                                                                    ^~~~~

src/models/artist.rs:17:61: 17:66 help: run `rustc --explain E0277` to see a detailed explanation

src/models/artist.rs:17:61: 17:66 help: the following implementations were found:

src/models/artist.rs:17:61: 17:66 help:   <collections::string::String as diesel::types::FromSqlRow<diesel::types::VarChar, DB>>

src/models/artist.rs:17:61: 17:66 help:   <collections::string::String as diesel::types::FromSqlRow<diesel::types::Text, DB>>

Model:

#[derive(Queryable)]
pub struct Artist {
    pub name: String,
    pub id: i32
}

I looked at the tests and the code looks alright. But I'm also just getting started with Diesel, so maybe I got something fundamentally wrong..

[sgrif]

Switch the order of the columns on your model. It's trying to load the ID
column into name and vice versa. This is a separate issue I'd like to
improve.

On Fri, Mar 18, 2016, 6:25 PM Jakob Gillich [email protected]
wrote:

Yes, please. Here is one that drives me crazy:

src/models/artist.rs:17:61: 17:66 error: the trait diesel::types::FromSqlRow<diesel::types::Integer, diesel::pg::backend::Pg> is not implemented for the type collections::string::String [E0277]

src/models/artist.rs:17 match artists::table.filter(artists::name.eq(name)).first(conn) {

                                                                                ^~~~~

src/models/artist.rs:17:61: 17:66 help: run rustc --explain E0277 to see a detailed explanation

src/models/artist.rs:17:61: 17:66 help: the following implementations were found:

src/models/artist.rs:17:61: 17:66 help: <collections::string::String as diesel::types::FromSqlRow<diesel::types::VarChar, DB>>

src/models/artist.rs:17:61: 17:66 help: <collections::string::String as diesel::types::FromSqlRow<diesel::types::Text, DB>>

Model:

#[derive(Queryable)]
pub struct Artist {
pub name: String,
pub id: i32
}

I looked at the tests and the code looks alright. But I'm also just
getting started with Diesel, so maybe I got something fundamentally wrong..

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#151 (comment)

[jgillich]

Oh wow, the order matters? I had no idea haha, thanks!

[sgrif]

For now, yes. We use indexed access instead of named access for performance
reasons. I want to remove the ordered requirement without impacting that.

On Fri, Mar 18, 2016, 6:30 PM Jakob Gillich [email protected]
wrote:

Oh wow, the order matters? I had no idea haha, thanks!

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#151 (comment)

[TheNeikos]

Order Matters

I believe this is something that could be adressed (for now) in the guide as I just stumbled onto this as well, and it cost me around more than hour trying to find out what is going on.

[ggrochow]

This is one you helped me with in gitter.im

error[E0277]: the trait bound `(): diesel::Queryable<(diesel::types::Integer, diesel::types::Integer, diesel::types::Text, diesel::types::Text, diesel::types::Timestamp, diesel::types::Timestamp, diesel::types::Text, diesel::types::Text), diesel::pg::Pg>` is not satisfied
  --> src/models/file.rs:66:24
   |
66 |         let res = self.save_changes(&conn)?;
   |                        ^^^^^^^^^^^^ the trait `diesel::Queryable<(diesel::types::Integer, diesel::types::Integer, diesel::types::Text, diesel::types::Text, diesel::types::Timestamp, diesel::types::Timestamp, diesel::types::Text, diesel::types::Text), diesel::pg::Pg>` is not implemented for `()`

adding an explicit type annotation to res solved this issue

let res: Self = self.save_changes(&conn)?;

[flybayer]

I'm stuck with the below error and can't figure it out. The postgres table was created with Rails which manages the schema.

Error

(reformatted a bit for readability)

error[E0277]: the trait bound
`diesel::types::Timestamp: diesel::types::FromSqlRow<diesel::types::Timestamp, _>` is not satisfied
  --> lora/src/lib.rs:45:31
   |
45 |     let results = end_devices.load::<EndDevice>(&connection).expect("Error loading devices");
   |                               ^^^^ the trait `diesel::types::FromSqlRow<diesel::types::Timestamp, _>` is not implemented for `diesel::types::Timestamp`
   |
   = note: required because of the requirements on the impl of
   `diesel::types::FromSqlRow<(
       diesel::types::BigInt,
       diesel::types::Nullable<diesel::types::Text>,
       diesel::types::Nullable<diesel::types::Text>,
       diesel::types::Timestamp, 
       diesel::types::Timestamp), _>` 
   for 
   `(i64, 
     std::option::Option<std::string::String>, 
     std::option::Option<std::string::String>, 
     diesel::types::Timestamp, 
     diesel::types::Timestamp)`

   = note: required because of the requirements on the impl of `diesel::Queryable<(diesel::types::BigInt, diesel::types::Nullable<diesel::types::Text>, diesel::types::Nullable<diesel::types::Text>, diesel::types::Timestamp, diesel::types::Timestamp), _>` for `models::EndDevice`

Model

#[derive(Queryable)]
pub struct EndDevice {
    pub id: i64,
    pub eui: Option<String>,
    pub app_key: Option<String>,
    pub created_at: types::Timestamp,
    pub updated_at: types::Timestamp,
}

My DB Table

lens_development=# SELECT * FROM end_devices WHERE id=87;
 id |           eui           |             app_key              |         created_at         |         updated_at
----+-------------------------+----------------------------------+----------------------------+----------------------------
 87 | 00-80-00-00-00-00-01-01 | 4F7E61E31F2E9AF39DC0C52CE542909F | 2017-05-04 21:58:56.668025 | 2017-05-04 21:58:56.668025

[Eijebong]

You shouldn't use diesel::types::Timestamp
Add the chrono feature instead and use chrono types.

[flybayer]

Thank you @Eijebong, I got it working!

For reference, this is working:

use chrono::prelude::*;

#[derive(Queryable, Debug)]
pub struct EndDevice {
    pub id: i64,
    pub eui: Option<String>,
    pub app_key: Option<String>,
    pub created_at: NaiveDateTime,
    pub updated_at: NaiveDateTime,
}

[killercup]

Just saw rust-lang/rust#43000 and remembered this issue.

I think we talked about it, but I'm not sure what the result was: Did we ever try something like the following?

#![cfg_attr(feature="unstable", feature(on_unimplemented))]

#[cfg_attr(feature="unstable", rustc_on_unimplemented("To query … you can find more info in our docs at http://diesel.rs/")]
pub trait FromSql // …

[sgrif]

Yes. It doesn't give us the diagnostics we'd need in most cases, and rust-lang/rust#28894 and rust-lang/rust#33094 generally cause our error message to never show up anyway

[sgrif]

So I discussed this with some folks at the all hands this year, and rustc_on_unimplemented has gotten a lot more functionality since the last time we looked at it. It still won't solve all our problematic cases, but I think it may be enough to help us in a few of them. I'm going to try to look into this further to give the compiler folks more feedback on where it could help, and where we still need better hooks.

This is something I'd like to make a major focus in 2019, hopefully looking into using a lint to help cases that we can't fix with other compiler hooks. A lot has changed since the last activity on this issue, so I'd like to renew the call for "please give us your bad error messages". If you can provide a small, single file script to reproduce the problem (e.g. something we'd be able to add as a compile test to Diesel), that would be much appreciated.

[pickfire]

#[derive(Queryable, Debug)]
pub struct User {
    id: i32,
    name: String,
}
 
#[derive(Insertable, Debug)]
pub struct NewUser {
    name: String,
}

I was getting a weird error when missing table_name, I thought it will show some good errors but somehow I forgot to insert it. Then when I run it, I got confused with the error message, at the end it took me some time to realize it's related to missing table_name.

error[E0433]: failed to resolve: use of undeclared crate or module `new_users`
 --> src/models.rs:8:12
  |
8 | pub struct NewUser {
  |            ^^^^^^^ use of undeclared crate or module `new_users`

Even with adding #[table_name = "users"]

error[E0433]: failed to resolve: use of undeclared crate or module `users`
 --> src/models.rs:8:16
  |
8 | #[table_name = "users"]
  |                ^^^^^^^ use of undeclared crate or module `users`

But this could be fixed with use super::schema::users;, this one is faster for me since I accidentally saw this on the guide when checking but I wish the errors could be as good as the rust compiler errors.

Any idea if this could be improved?

In the meantime I got another weird error.

error[E0308]: mismatched types
  --> src/bin/init.rs:8:9
   |
8  |       let users = vec![
   |  _________^^^^^___-
   | |         |
   | |         expected struct `Vec`, found struct `hello::schema::users::table`
9  | |         NewUser { name: String::from("John") },
10 | |         NewUser { name: String::from("Jane") },
11 | |     ];
   | |_____- this expression has type `Vec<hello::models::NewUser>`
   |
   = note: expected struct `Vec<hello::models::NewUser>`
              found struct `hello::schema::users::table`

This was because I mixed users variable and users::table below, weird namespace crash, I thought it should be type namespace and won't happen.

[weiznich]

@pickfire

Even with adding #[table_name = "users"]

error[E0433]: failed to resolve: use of undeclared crate or module `users` 
--> src/models.rs:8:16
  |
8 | #[table_name = "users"]
  |                ^^^^^^^ use of undeclared crate or module `users`

But this could be fixed with use super::schema::users;, this one is faster for me since I accidentally saw this on the guide when checking but I wish the errors could be as good as the rust compiler errors.

Generating a better error message for this case is unfortunately not possible as rust does not provide any API for proc macros to check which items are in scope and which are not in scope.

I was getting a weird error when missing table_name, I thought it will show some good errors but somehow I forgot to insert it. Then when I run it, I got confused with the error message, at the end it took me some time to realize it's related to missing table_name.

error[E0433]: failed to resolve: use of undeclared crate or module `new_users`
 --> src/models.rs:8:12
 |
8 | pub struct NewUser {
 |            ^^^^^^^ use of undeclared crate or module `new_users`

If there is no #[table_name = "…"] attribute all of our derives default to using the plural (just append a s) snake case name of the current struct as table_name. As removing this behavior is a rather large change I'm not able to see how to improve that behavior here.

In the meantime I got another weird error.

error[E0308]: mismatched types
 --> src/bin/init.rs:8:9
  |
8  |       let users = vec![
  |  _________^^^^^___-
  | |         |
  | |         expected struct `Vec`, found struct `hello::schema::users::table`
9  | |         NewUser { name: String::from("John") },
10 | |         NewUser { name: String::from("Jane") },
11 | |     ];
  | |_____- this expression has type `Vec<hello::models::NewUser>`
  |
  = note: expected struct `Vec<hello::models::NewUser>`
             found struct `hello::schema::users::table`

This was because I mixed users variable and users::table below, weird namespace crash, I thought it should be type namespace and won't happen.

You likely had an use schema::users::dsl::* somewhere which reexports schema::users::table as schema::users::dsl::users. schema::users::table is a zero sized struct, which means it's both a value and a type, so something with that name exists in both namespaces. Again this is nothing we can fix on diesels side, but I think the rustc error message could be improved there giving a hint that there may be a name collision there. Could you open a issue in the rustc repo about that?

[pickfire]

Thanks for the explanation, I didn't know zero sized struct is both a value and a type.

While trying to make a minimal reproduction, I also find that you can't use #[table_name = "schema::users"], it will show = help: message: "schema::users" is not a valid identifier, I guess that is another rustc limitation on paths.