SQLite: `NOT NULL` not checked at incremental builds

[fbruetting]

If I put NOT NULL in every entry of my SQLite up.sql, compilation works fine.

After the first compilation, I can leave NOT NULL out and it still compiles fine, but as soon as cargo has to do a full compilation (I deleted the target folder and the database), I get strange error messages like shown below. So if NOT NULL is necessary, it needs to be checked in incremental compilation, too. Or should Diesel work in conjunction with SQLite without the need for specifying NOT NULL on every attribute?

And this has something to to with both the binarys and the database file, since I always have to delete the database and the target folder before full compilation, for the bug to appear. However, if I correct the up.sql, I just have to delete the database, and incremental compilation then works.

Tested with 0.11 and 0.12 on Rust stable.

error[E0277]: the trait bound `std::string::String: diesel::types::FromSqlRow<diesel::types::Nullable<diesel::types::Text>, diesel::sqlite::Sqlite>` is not satisfied
   --> src/db/mod.rs:168:13
    |
168 | 									  .first (&connection)
    | 									   ^^^^^ the trait `diesel::types::FromSqlRow<diesel::types::Nullable<diesel::types::Text>, diesel::sqlite::Sqlite>` is not implemented for `std::string::String`
    |
    = help: the following implementations were found:
              <std::string::String as diesel::types::FromSqlRow<diesel::types::Text, DB>>
              <std::string::String as diesel::types::FromSqlRow<diesel::types::Date, DB>>
              <std::string::String as diesel::types::FromSqlRow<diesel::types::Time, DB>>
              <std::string::String as diesel::types::FromSqlRow<diesel::types::Timestamp, DB>>
    = note: required because of the requirements on the impl of `diesel::types::FromSqlRow<(diesel::types::Integer, diesel::types::Integer, diesel::types::Text, diesel::types::Nullable<diesel::types::Text>), diesel::sqlite::Sqlite>` for `(i32, i32, std::string::String, std::string::String)`
    = note: required because of the requirements on the impl of `diesel::Queryable<(diesel::types::Integer, diesel::types::Integer, diesel::types::Text, diesel::types::Nullable<diesel::types::Text>), diesel::sqlite::Sqlite>` for `db::models::Session`

error: aborting due to previous error

[sgrif]

I'm surprised that this causes issues with incremental compilation specifically. One unfortunate property of infer_schema! is that Rust doesn't know that database changes should trigger a rebuild. Usually that's not a problem, since some code should have changed to trigger a rebuild. I would have expected that incremental compilation was more or less never caching infer_schema! since it's using a procedural macro.

Can you provide a full set of steps that I can follow to reproduce this issue, where following those steps with incremental compilation turned on causes things to incorrectly compile, but following them with incremental compilation disabled does not? There's not enough information to reproduce/identify the issue here.

[fbruetting]

Diesel.zip
I extracted a minimal example out of my program for you.

• For showing the error, just run diesel migration run --database-url data/test.db and cargo build.
• For successful compilation, fill in the missing NOT NULL in the migration up.sql file, delete the database and the target directory, create the database again and run cargo build.
  • After a successful compilation, you can delete the NOT NULL statement again in up.sql, and it still will compile fine.
  • Here you can delete the target directory, and the full compilation runs fine.
  • Now you delete the database and create it again (without the NOT NULL statement), and incremental compilation still runs fine.
  • If you THEN delete the target directory, you'll have a full compilation, and the error appears!

There should be a better error message, if there's an error in the up.sql. Assuming something that should be, is always a source of problems – please handle this more strictly!

Other question: Why doesn't Diesel work without these NOT NULL statements?

// PS 1: Why is a completely empty database file created at compilation, if I don't create it by running the diesel command?
// PS 2: How can I put the migrations directory into my db directory?

[sgrif]

Modifying a migration does not change your database. It is expected that you always create a new migration or roll back the migration before modifying it. Everything else you've mentioned falls into what I mentioned above. Cargo doesn't know that your database changed if you had no code changes. It has nothing to do with incremental compilation.

Why doesn't Diesel work without these NOT NULL statements?

Are you trying to load data into a field that isn't an Option? That would be a mismatch, as the type could contain null. I can't really answer that generally, as Diesel doesn't "require NOT NULL everywhere", but we do enforce type safety.

Why is a completely empty database file created at compilation, if I don't create it by running the diesel command?

SQLite creates the database file if it doesn't exist when you establish a connection. This is regardless of Diesel.

How can I put the migrations directory into my db directory?

Run diesel --help to see the list of options. The option you need is --migration-directory, though it doesn't work everywhere. There's an open issue for it. You may be interested in embed_migrations! as well.

I'm going to close this as I don't see any actionable issue here