From cba76b5a6d856d8b678fd8d2b452f92433618c7d Mon Sep 17 00:00:00 2001 From: Kevin GRONDIN Date: Mon, 19 Feb 2024 13:21:09 +0100 Subject: [PATCH] Use BigInt for integer primary key in sqlite for diesel-cli --- .github/workflows/ci.yml | 2 +- CHANGELOG.md | 41 ++-- diesel_cli/Cargo.toml | 2 +- diesel_cli/src/cli.rs | 22 ++ diesel_cli/src/config.rs | 2 + .../src/infer_schema_internals/inference.rs | 27 ++- .../src/infer_schema_internals/sqlite.rs | 190 +++++++++++++++++- diesel_cli/src/main.rs | 4 + diesel_cli/src/migrations/diff_schema.rs | 86 ++++++-- diesel_cli/src/migrations/mod.rs | 5 +- diesel_cli/src/print_schema.rs | 2 +- .../mysql/down.sql/expected.snap | 7 + .../mysql/initial_schema.sql | 1 + .../mysql/schema_out.rs/expected.snap | 12 ++ .../mysql/up.sql/expected.snap | 9 + .../postgres/down.sql/expected.snap | 8 + .../postgres/initial_schema.sql | 1 + .../postgres/schema_out.rs/expected.snap | 13 ++ .../postgres/up.sql/expected.snap | 10 + .../schema.rs | 6 + .../sqlite/down.sql/expected.snap | 7 + .../sqlite/initial_schema.sql | 1 + .../sqlite/schema_out.rs/expected.snap | 12 ++ .../sqlite/up.sql/expected.snap | 10 + diesel_cli/tests/migration_generate.rs | 8 + diesel_cli/tests/print_schema.rs | 9 + .../diesel.toml | 4 + .../sqlite/expected.snap | 30 +++ .../sqlite/schema.sql | 3 + 29 files changed, 482 insertions(+), 52 deletions(-) create mode 100644 diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/down.sql/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/initial_schema.sql create mode 100644 diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/schema_out.rs/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/up.sql/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/down.sql/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/initial_schema.sql create mode 100644 diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/schema_out.rs/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/up.sql/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/schema.rs create mode 100644 diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/down.sql/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/initial_schema.sql create mode 100644 diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/schema_out.rs/expected.snap create mode 100644 diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/up.sql/expected.snap create mode 100644 diesel_cli/tests/print_schema/print_schema_sqlite_rowid_column/diesel.toml create mode 100644 diesel_cli/tests/print_schema/print_schema_sqlite_rowid_column/sqlite/expected.snap create mode 100644 diesel_cli/tests/print_schema/print_schema_sqlite_rowid_column/sqlite/schema.sql diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d1b50011926a..5677f3d12c92 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -260,7 +260,7 @@ jobs: env: BACKEND: ${{ matrix.backend }} run: | - (cd examples/${{ matrix.backend }} && rustup run ${{ matrix.rust }} bash test_all) + (cd examples/${{ matrix.backend }} && rustup run ${{ matrix.rust }} bash test_all) - name: Test migrations-internals shell: bash diff --git a/CHANGELOG.md b/CHANGELOG.md index 8bdfef85ed44..fff8d3863205 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,9 +3,9 @@ All user visible changes to this project will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org/), as described for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/text/1105-api-evolution.md) -For any named minimal supported Rust version we guarantee that it is possible to build Diesel with the -default features enabled using some set of dependencies. Those set of dependencies is not necessarily -an up to date version of the specific dependency. We check this by using the unstable `-Z minimal-version` cargo flag. +For any named minimal supported Rust version we guarantee that it is possible to build Diesel with the +default features enabled using some set of dependencies. Those set of dependencies is not necessarily +an up to date version of the specific dependency. We check this by using the unstable `-Z minimal-version` cargo flag. Increasing the minimal supported Rust version will always be coupled at least with a minor release. ## Unreleased @@ -19,6 +19,7 @@ Increasing the minimal supported Rust version will always be coupled at least wi * Support for connection instrumentation. This allows to inspect any query run by your application * Logging in diesel-cli * Support for libsqlite3-sys 0.28 +* Add `sqlite-integer-primary-key-is-bigint` configuration option, usable with SQLite 3.37 or above, allowing to use `BigInt` for `INTEGER PRIMARY KEY` columns in SQLite for tables without the `WITHOUT ROWID` attribute ([SQLite doc](https://www.sqlite.org/lang_createtable.html#rowid)). ### Changed @@ -26,7 +27,7 @@ Increasing the minimal supported Rust version will always be coupled at least wi ## [2.1.0] 2023-05-26 -### Changed +### Changed * The minimal officially supported rustc version is now 1.65.0 @@ -48,7 +49,7 @@ Increasing the minimal supported Rust version will always be coupled at least wi ## [2.0.4] 2023-04-18 -## Fixed +## Fixed * Workaround the missing name resolution in rust-analyzer. This should fix type inference for some diesel queries. (It remains broken for queries containing `.filter()`/`.inner_join()`/`.left_join()`. These require fixes in rust-analyzer itself) * Fixed a bug that could lead to inserting null values instead of empty values for custom sqlite types @@ -59,9 +60,9 @@ Increasing the minimal supported Rust version will always be coupled at least wi * Support for `libsqlite3-sys` 0.26 -## [diesel_derives 2.0.2] 2023-03-13 +## [diesel_derives 2.0.2] 2023-03-13 -## Fixed +## Fixed * Fixing the fallout of a breaking change from `quote` by not using their internal API @@ -130,7 +131,7 @@ Increasing the minimal supported Rust version will always be coupled at least wi in such a way to support constructing a dynamic value depending on this type. * Added a `without-deprecated` feature that unconditionally disables deprecated items. - Use this feature flag to verify that none of your dependencies is setting + Use this feature flag to verify that none of your dependencies is setting the `with-deprecated` flag internally. * Added support for PostgreSQL's `SIMILAR TO` and `NOT SIMILAR TO`. @@ -154,7 +155,7 @@ Increasing the minimal supported Rust version will always be coupled at least wi * Diesel CLI will now generate SQL type definitions for SQL types that are not supported by diesel out of the box. It's possible to disable this behavior via the `generate_missing_sql_type_definitions` config option. -* Added an option to `#[derive(Insertable)]` that let you insert `NULL` values instead of `DEFAULT` values for `Option` +* Added an option to `#[derive(Insertable)]` that let you insert `NULL` values instead of `DEFAULT` values for `Option` * Added support for all the derive attributes being inside `#[diesel(...)]` @@ -259,12 +260,12 @@ Increasing the minimal supported Rust version will always be coupled at least wi card implementations for types implementing `Queryable` or `QueryableByName` so non generic code does not require any change. For generic code you likely need to replace a trait bound on `Queryable` with a trait bound on `FromSqlRow` - and a bound to `QueryableByName` with `FromSqlRow`. + and a bound to `QueryableByName` with `FromSqlRow`. * CLI flags of `only-tables` and `except-tables` are now interpreted as regular expressions. Similarly, `only_tables` and `except_tables` in `diesel.toml` are treated as regular expressions. -* Now you can sort column fields by name with the `column-sorting` option. +* Now you can sort column fields by name with the `column-sorting` option. It can be set to either `ordinal_position` (default) or `name`. This ensures stable sorting even if columns are removed and re-added. @@ -277,25 +278,25 @@ Increasing the minimal supported Rust version will always be coupled at least wi * `TypeMetadata::MetadataLookup` is now `?Sized`. -* Multiple implementations of `Connection` are now possible +* Multiple implementations of `Connection` are now possible because of the new `PgMetadataLookup` trait. * For the `Pg` backend, `TypeMetadata::MetadataLookup` has changed to `dyn PgMetadataLookup`. -* Diesel's migration framework was rewritten from the ground. Existing migrations continue to +* Diesel's migration framework was rewritten from the ground. Existing migrations continue to be compatible with the rewrite, but code calling into `diesel_migrations` requires an update. See the [migration guide](2-0-migration) for details. * `eq_any()` now emits a `= ANY()` expression for the postgresql backend instead of `IN()` * `ne_all()` now emits a `!= ALL()` expression for the postgresql backend instead of `NOT IN()` -* The sqlite backend now uses a single batch insert statement if there are now default values present +* The sqlite backend now uses a single batch insert statement if there are now default values present in the values clause * The MySQL connection is using the CLIENT_FOUND_ROWS from now on. This means that updating rows without changing any values will return the number of matched rows (like most other SQL servers do), as opposed to the number of changed rows. -* The definition of `ToSql::to_sql` and `QueryFragment::walk_ast` has changed to allow serializing values without +* The definition of `ToSql::to_sql` and `QueryFragment::walk_ast` has changed to allow serializing values without copying the value itself. This is useful for database backends like sqlite where you can directly share a buffer - with the database. Beside of the changed signature, existing impls of this trait should remain unchanged in almost + with the database. Beside of the changed signature, existing impls of this trait should remain unchanged in almost all cases. * The `PIPES_AS_CONCAT` sql_mode is no longer set @@ -342,14 +343,14 @@ queries or set `PIPES_AS_CONCAT` manually. * We've refactored our type level representation of nullable values. This allowed us to fix multiple long standing bugs regarding the correct handling of nullable values in some corner cases (#104, #2274) - + * Parenthesis are now inserted around all infix operations provided by diesel's `ExpressionMethods` traits * Queries containing a `distinct on` clause check now on compile time that a compatible order clause was set. * Implementations of custom SQLite SQL functions now check for panics -* `diesel print-schema` now generates `Array>` rather than `Array` for Postgres Array types. Existence of +* `diesel print-schema` now generates `Array>` rather than `Array` for Postgres Array types. Existence of `NULL` values in database arrays would previously result in deserialization errors. Non-nullable arrays are now opt in (by schema patching). @@ -363,8 +364,8 @@ queries or set `PIPES_AS_CONCAT` manually. * `diesel::pg::upsert` has been deprecated to support upsert queries on more than one backend. Please use `diesel::upsert` instead. - -* `diesel::dsl::any` and `diesel::dsl::all` are now deprecated in + +* `diesel::dsl::any` and `diesel::dsl::all` are now deprecated in favour of `ExpressionMethods::eq_any()` and `ExpressionMethods::ne_all()` diff --git a/diesel_cli/Cargo.toml b/diesel_cli/Cargo.toml index a9d2ba62f3f4..05354ffabc87 100644 --- a/diesel_cli/Cargo.toml +++ b/diesel_cli/Cargo.toml @@ -36,7 +36,7 @@ pq-sys = { version = ">=0.4, <0.6.0", optional = true } diffy = "0.3.0" regex = "1.0.6" serde_regex = "1.1" -diesel_table_macro_syntax = {version = "0.1", path = "../diesel_table_macro_syntax"} +diesel_table_macro_syntax = { version = "0.1", path = "../diesel_table_macro_syntax" } syn = { version = "2", features = ["visit"] } tracing = "0.1" tracing-subscriber = { version = "0.3.10", features = ["env-filter"] } diff --git a/diesel_cli/src/cli.rs b/diesel_cli/src/cli.rs index 25bf28de6028..c35fd86568c6 100644 --- a/diesel_cli/src/cli.rs +++ b/diesel_cli/src/cli.rs @@ -148,6 +148,18 @@ pub fn build_cli() -> Command { .num_args(0..=1) .require_equals(true), ) + .arg( + Arg::new("sqlite-integer-primary-key-is-bigint") + .long("sqlite-integer-primary-key-is-bigint") + .requires("SCHEMA_RS") + .action(ArgAction::SetTrue) + .help( + "For SQLite 3.37 and above, detect `INTEGER PRIMARY KEY` columns as `BigInt`, \ + when the table isn't declared with `WITHOUT ROWID`.\n\ + See https://www.sqlite.org/lang_createtable.html#rowid for more information.\n\ + Only used with the `--diff-schema` argument." + ), + ) .arg( Arg::new("table-name") .index(2) @@ -289,6 +301,16 @@ pub fn build_cli() -> Command { .action(clap::ArgAction::Append) .number_of_values(1) .help("A list of derives to implement for every automatically generated SqlType in the schema, separated by commas."), + ) + .arg( + Arg::new("sqlite-integer-primary-key-is-bigint") + .long("sqlite-integer-primary-key-is-bigint") + .action(ArgAction::SetTrue) + .help( + "For SQLite 3.37 and above, detect `INTEGER PRIMARY KEY` columns as `BigInt`, \ + when the table isn't declared with `WITHOUT ROWID`.\n\ + See https://www.sqlite.org/lang_createtable.html#rowid for more information." + ), ); let config_arg = Arg::new("CONFIG_FILE") diff --git a/diesel_cli/src/config.rs b/diesel_cli/src/config.rs index 7a886aef3d4d..889b62317a6d 100644 --- a/diesel_cli/src/config.rs +++ b/diesel_cli/src/config.rs @@ -90,6 +90,8 @@ pub struct PrintSchema { pub generate_missing_sql_type_definitions: Option, #[serde(default)] pub custom_type_derives: Option>, + #[serde(default)] + pub sqlite_integer_primary_key_is_bigint: Option, } impl PrintSchema { diff --git a/diesel_cli/src/infer_schema_internals/inference.rs b/diesel_cli/src/infer_schema_internals/inference.rs index cf7650e0573c..2e95ebbb5c1f 100644 --- a/diesel_cli/src/infer_schema_internals/inference.rs +++ b/diesel_cli/src/infer_schema_internals/inference.rs @@ -3,7 +3,7 @@ use diesel::result::Error::NotFound; use super::data_structures::*; use super::table_data::*; -use crate::config::Filtering; +use crate::config::{Filtering, PrintSchema}; use crate::database::InferConnection; use crate::print_schema::{ColumnSorting, DocConfig}; @@ -185,10 +185,15 @@ fn get_column_information( fn determine_column_type( attr: &ColumnInformation, conn: &mut InferConnection, + #[allow(unused_variables)] table: &TableName, + #[allow(unused_variables)] primary_keys: &[String], + #[allow(unused_variables)] config: &PrintSchema, ) -> Result { match *conn { #[cfg(feature = "sqlite")] - InferConnection::Sqlite(_) => super::sqlite::determine_column_type(attr), + InferConnection::Sqlite(ref mut conn) => { + super::sqlite::determine_column_type(conn, attr, table, primary_keys, config) + } #[cfg(feature = "postgres")] InferConnection::Pg(ref mut conn) => { use crate::infer_schema_internals::information_schema::DefaultSchema; @@ -259,11 +264,10 @@ pub fn load_foreign_key_constraints( pub fn load_table_data( connection: &mut InferConnection, name: TableName, - column_sorting: &ColumnSorting, - with_docs: DocConfig, + config: &PrintSchema, ) -> Result { // No point in loading table comments if they are not going to be displayed - let table_comment = match with_docs { + let table_comment = match config.with_docs { DocConfig::NoDocComments => None, DocConfig::OnlyDatabaseComments | DocConfig::DatabaseCommentsFallbackToAutoGeneratedDocComment => { @@ -272,15 +276,11 @@ pub fn load_table_data( }; let primary_key = get_primary_keys(connection, &name)?; - let primary_key = primary_key - .iter() - .map(|k| rust_name_for_sql_name(k)) - .collect(); - let column_data = get_column_information(connection, &name, column_sorting)? + let column_data = get_column_information(connection, &name, &config.column_sorting)? .into_iter() .map(|c| { - let ty = determine_column_type(&c, connection)?; + let ty = determine_column_type(&c, connection, &name, &primary_key, config)?; let ColumnInformation { column_name, @@ -298,6 +298,11 @@ pub fn load_table_data( }) .collect::>()?; + let primary_key = primary_key + .iter() + .map(|k| rust_name_for_sql_name(k)) + .collect::>(); + Ok(TableData { name, primary_key, diff --git a/diesel_cli/src/infer_schema_internals/sqlite.rs b/diesel_cli/src/infer_schema_internals/sqlite.rs index 272a462c10e2..dd2f9276ff7b 100644 --- a/diesel_cli/src/infer_schema_internals/sqlite.rs +++ b/diesel_cli/src/infer_schema_internals/sqlite.rs @@ -1,3 +1,5 @@ +use std::fmt; + use diesel::deserialize::{self, Queryable}; use diesel::dsl::sql; use diesel::row::NamedRow; @@ -7,6 +9,7 @@ use diesel::*; use super::data_structures::*; use super::table_data::TableName; +use crate::config::PrintSchema; use crate::print_schema::ColumnSorting; table! { @@ -100,6 +103,12 @@ impl SqliteVersion { } } +impl fmt::Display for SqliteVersion { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}.{}.{}", self.major, self.minor, self.patch) + } +} + fn get_sqlite_version(conn: &mut SqliteConnection) -> QueryResult { let query = "SELECT sqlite_version()"; let result = sql::(query).load::(conn)?; @@ -204,6 +213,55 @@ impl QueryableByName for PrimaryKeyInformation { } } +struct WithoutRowIdInformation { + name: String, + without_row_id: bool, +} + +impl QueryableByName for WithoutRowIdInformation { + fn build<'a>(row: &impl NamedRow<'a, Sqlite>) -> deserialize::Result { + Ok(Self { + name: NamedRow::get::(row, "name")?, + without_row_id: NamedRow::get::(row, "wr")?, + }) + } +} + +pub fn column_is_row_id( + conn: &mut SqliteConnection, + table: &TableName, + primary_keys: &[String], + column_name: &str, + type_name: &str, +) -> Result { + let sqlite_version = get_sqlite_version(conn)?; + if sqlite_version < SqliteVersion::new(3, 37, 0) { + return Err(crate::errors::Error::UnsupportedFeature(format!( + "Parameter `sqlite_integer_primary_key_is_bigint` needs SQLite 3.37 or above. \ + Your current SQLite version is {sqlite_version}." + ))); + } + + if type_name != "integer" { + return Ok(false); + } + + if !matches!(primary_keys, [pk] if pk == column_name) { + return Ok(false); + } + + let table_list_query = format!("PRAGMA TABLE_LIST('{}')", &table.sql_name); + let table_list_results = sql_query(table_list_query).load::(conn)?; + + let res = table_list_results + .iter() + .find(|wr_info| wr_info.name == table.sql_name) + .map(|wr_info| !wr_info.without_row_id) + .unwrap_or_default(); + + Ok(res) +} + #[derive(Queryable)] struct ForeignKeyListRow { _id: i32, @@ -260,8 +318,14 @@ pub fn get_primary_keys( Ok(collected) } -#[tracing::instrument] -pub fn determine_column_type(attr: &ColumnInformation) -> Result { +#[tracing::instrument(skip(conn))] +pub fn determine_column_type( + conn: &mut SqliteConnection, + attr: &ColumnInformation, + table: &TableName, + primary_keys: &[String], + config: &PrintSchema, +) -> Result { let mut type_name = attr.type_name.to_lowercase(); if type_name == "generated always" { type_name.clear(); @@ -274,7 +338,17 @@ pub fn determine_column_type(attr: &ColumnInformation) -> Result>(); + + let on_column_types = column_infos + .iter() + .map(|column_info| { + ( + column_info.column_name.as_str(), + determine_column_type( + &mut conn, + column_info, + &table, + &primary_keys, + &PrintSchema { + sqlite_integer_primary_key_is_bigint: Some(true), + ..Default::default() + }, + ) + .unwrap() + .sql_name, + ) + }) + .collect::>(); + + assert_eq!( + (table_name, off_column_types), + (table_name, expected_off_types) + ); + + assert_eq!( + (table_name, on_column_types), + (table_name, expected_on_types) + ); + } +} diff --git a/diesel_cli/src/main.rs b/diesel_cli/src/main.rs index a0ae06a971a3..f8cc868c43bd 100644 --- a/diesel_cli/src/main.rs +++ b/diesel_cli/src/main.rs @@ -296,6 +296,10 @@ fn run_infer_schema(matches: &ArgMatches) -> Result<(), crate::errors::Error> { config.custom_type_derives = Some(derives); } + if matches.get_flag("sqlite-integer-primary-key-is-bigint") { + config.sqlite_integer_primary_key_is_bigint = Some(true); + } + run_print_schema(&mut conn, &config, &mut stdout())?; Ok(()) } diff --git a/diesel_cli/src/migrations/diff_schema.rs b/diesel_cli/src/migrations/diff_schema.rs index 08c05f15d52d..d55d4e610979 100644 --- a/diesel_cli/src/migrations/diff_schema.rs +++ b/diesel_cli/src/migrations/diff_schema.rs @@ -9,13 +9,13 @@ use std::io::Read; use std::path::Path; use syn::visit::Visit; -use crate::config::Config; +use crate::config::{Config, PrintSchema}; use crate::database::InferConnection; use crate::infer_schema_internals::{ filter_table_names, load_table_names, ColumnDefinition, ColumnType, ForeignKeyConstraint, TableData, TableName, }; -use crate::print_schema::DocConfig; +use crate::print_schema::{ColumnSorting, DocConfig}; fn compatible_type_list() -> HashMap<&'static str, Vec<&'static str>> { let mut map = HashMap::new(); @@ -32,7 +32,7 @@ pub fn generate_sql_based_on_diff_schema( matches: &ArgMatches, schema_file_path: &Path, ) -> Result<(String, String), crate::errors::Error> { - let config = config.set_filter(matches)?; + let mut config = config.set_filter(matches)?; let project_root = crate::find_project_root()?; @@ -88,6 +88,21 @@ pub fn generate_sql_based_on_diff_schema( expected_schema_map.insert(t.table_name.to_string(), t); } + config.print_schema.with_docs = DocConfig::NoDocComments; + config.print_schema.column_sorting = ColumnSorting::OrdinalPosition; + + // Parameter `sqlite_integer_primary_key_is_bigint` is only used for a SQLite connection + match conn { + #[cfg(feature = "postgres")] + InferConnection::Pg(_) => config.print_schema.sqlite_integer_primary_key_is_bigint = None, + #[cfg(feature = "sqlite")] + InferConnection::Sqlite(_) => (), + #[cfg(feature = "mysql")] + InferConnection::Mysql(_) => { + config.print_schema.sqlite_integer_primary_key_is_bigint = None; + } + } + let mut schema_diff = Vec::new(); for table in tables_from_database { @@ -95,8 +110,7 @@ pub fn generate_sql_based_on_diff_schema( let columns = crate::infer_schema_internals::load_table_data( &mut conn, table.clone(), - &crate::print_schema::ColumnSorting::OrdinalPosition, - DocConfig::NoDocComments, + &config.print_schema, )?; if let Some(t) = expected_schema_map.remove(&table.sql_name.to_lowercase()) { tracing::info!(table = ?t.sql_name, "Table exists in schema.rs"); @@ -208,19 +222,19 @@ pub fn generate_sql_based_on_diff_schema( #[cfg(feature = "postgres")] InferConnection::Pg(_) => { let mut qb = diesel::pg::PgQueryBuilder::default(); - diff.generate_up_sql(&mut qb)?; + diff.generate_up_sql(&mut qb, &config.print_schema)?; qb.finish() } #[cfg(feature = "sqlite")] InferConnection::Sqlite(_) => { let mut qb = diesel::sqlite::SqliteQueryBuilder::default(); - diff.generate_up_sql(&mut qb)?; + diff.generate_up_sql(&mut qb, &config.print_schema)?; qb.finish() } #[cfg(feature = "mysql")] InferConnection::Mysql(_) => { let mut qb = diesel::mysql::MysqlQueryBuilder::default(); - diff.generate_up_sql(&mut qb)?; + diff.generate_up_sql(&mut qb, &config.print_schema)?; qb.finish() } }; @@ -229,19 +243,19 @@ pub fn generate_sql_based_on_diff_schema( #[cfg(feature = "postgres")] InferConnection::Pg(_) => { let mut qb = diesel::pg::PgQueryBuilder::default(); - diff.generate_down_sql(&mut qb)?; + diff.generate_down_sql(&mut qb, &config.print_schema)?; qb.finish() } #[cfg(feature = "sqlite")] InferConnection::Sqlite(_) => { let mut qb = diesel::sqlite::SqliteQueryBuilder::default(); - diff.generate_down_sql(&mut qb)?; + diff.generate_down_sql(&mut qb, &config.print_schema)?; qb.finish() } #[cfg(feature = "mysql")] InferConnection::Mysql(_) => { let mut qb = diesel::mysql::MysqlQueryBuilder::default(); - diff.generate_down_sql(&mut qb)?; + diff.generate_down_sql(&mut qb, &config.print_schema)?; qb.finish() } }; @@ -311,6 +325,7 @@ impl SchemaDiff { fn generate_up_sql( &self, query_builder: &mut impl QueryBuilder, + config: &PrintSchema, ) -> Result<(), crate::errors::Error> where DB: Backend, @@ -352,12 +367,18 @@ impl SchemaDiff { ) }) .collect::>(); + + let sqlite_integer_primary_key_is_bigint = config + .sqlite_integer_primary_key_is_bigint + .unwrap_or_default(); + generate_create_table( query_builder, table, &column_data, &primary_keys, &foreign_keys, + sqlite_integer_primary_key_is_bigint, )?; } SchemaDiff::ChangeTable { @@ -390,7 +411,11 @@ impl SchemaDiff { Ok(()) } - fn generate_down_sql(&self, query_builder: &mut impl QueryBuilder) -> QueryResult<()> + fn generate_down_sql( + &self, + query_builder: &mut impl QueryBuilder, + config: &PrintSchema, + ) -> QueryResult<()> where DB: Backend, { @@ -411,12 +436,17 @@ impl SchemaDiff { }) .collect::>(); + let sqlite_integer_primary_key_is_bigint = config + .sqlite_integer_primary_key_is_bigint + .unwrap_or_default(); + generate_create_table( query_builder, &table.sql_name.to_lowercase(), &columns.column_data, &columns.primary_key, &fk, + sqlite_integer_primary_key_is_bigint, )?; } SchemaDiff::CreateTable { to_create, .. } => { @@ -428,6 +458,9 @@ impl SchemaDiff { removed_columns, changed_columns, } => { + // We don't need to check the `sqlite_integer_primary_key_is_bigint` parameter here + // since `ÀLTER TABLE` queries cannot modify primary key columns in SQLite. + // See https://www.sqlite.org/lang_altertable.html#alter_table_add_column for more information. for c in added_columns .iter() .chain(changed_columns.iter().map(|(_, b)| b)) @@ -498,6 +531,7 @@ fn generate_create_table( column_data: &[ColumnDefinition], primary_keys: &[String], foreign_keys: &[(String, String, String)], + sqlite_integer_primary_key_is_bigint: bool, ) -> QueryResult<()> where DB: Backend, @@ -514,11 +548,35 @@ where query_builder.push_sql(",\n"); } query_builder.push_sql("\t"); + + let is_only_primary_key = + primary_keys.contains(&column.rust_name) && primary_keys.len() == 1; + query_builder.push_identifier(&column.sql_name)?; - generate_column_type_name(query_builder, &column.ty); - if primary_keys.contains(&column.rust_name) && primary_keys.len() == 1 { + + // When the `sqlite_integer_primary_key_is_bigint` config parameter is used, + // if a column is the only primary key and its type is `BigInt`, + // we consider it equivalent to the `rowid` column in order to be compatible + // with the `print-schema` command using the same config parameter. + // See https://www.sqlite.org/lang_createtable.html#rowid for more information. + if sqlite_integer_primary_key_is_bigint + && is_only_primary_key + && column.ty.sql_name.eq_ignore_ascii_case("BigInt") + { + let ty = ColumnType { + rust_name: "Integer".into(), + sql_name: "Integer".into(), + ..column.ty.clone() + }; + generate_column_type_name(query_builder, &ty); + } else { + generate_column_type_name(query_builder, &column.ty); + } + + if is_only_primary_key { query_builder.push_sql(" PRIMARY KEY"); } + if let Some((table, _, pk)) = foreign_keys.iter().find(|(_, k, _)| k == &column.rust_name) { foreign_key_list.push((column, table, pk)); } diff --git a/diesel_cli/src/migrations/mod.rs b/diesel_cli/src/migrations/mod.rs index f4785795594b..0d529e354086 100644 --- a/diesel_cli/src/migrations/mod.rs +++ b/diesel_cli/src/migrations/mod.rs @@ -91,12 +91,15 @@ pub(super) fn run_migration_command(matches: &ArgMatches) -> Result<(), crate::e let (up_sql, down_sql) = if let Some(diff_schema) = args.get_one::("SCHEMA_RS") { - let config = Config::read(matches)?; + let mut config = Config::read(matches)?; let diff_schema = if diff_schema == "NOT_SET" { config.print_schema.file.clone() } else { Some(PathBuf::from(diff_schema)) }; + if args.get_flag("sqlite-integer-primary-key-is-bigint") { + config.print_schema.sqlite_integer_primary_key_is_bigint = Some(true); + } if let Some(diff_schema) = diff_schema { self::diff_schema::generate_sql_based_on_diff_schema( config, diff --git a/diesel_cli/src/print_schema.rs b/diesel_cli/src/print_schema.rs index 5217192dd345..76e6573e5187 100644 --- a/diesel_cli/src/print_schema.rs +++ b/diesel_cli/src/print_schema.rs @@ -157,7 +157,7 @@ pub fn output_schema( remove_unsafe_foreign_keys_for_codegen(connection, &foreign_keys, &table_names); let table_data = table_names .into_iter() - .map(|t| load_table_data(connection, t, &config.column_sorting, config.with_docs)) + .map(|t| load_table_data(connection, t, config)) .collect::, crate::errors::Error>>()?; let mut out = String::new(); diff --git a/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/down.sql/expected.snap new file mode 100644 index 000000000000..6bf6dc40cefc --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/down.sql/expected.snap @@ -0,0 +1,7 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_add_table" +--- +-- This file should undo anything in `up.sql` +DROP TABLE IF EXISTS `users`; + diff --git a/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/initial_schema.sql b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/initial_schema.sql new file mode 100644 index 000000000000..8b137891791f --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/initial_schema.sql @@ -0,0 +1 @@ + diff --git a/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/schema_out.rs/expected.snap new file mode 100644 index 000000000000..a7c2e4fa010d --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/schema_out.rs/expected.snap @@ -0,0 +1,12 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_add_table" +--- +// @generated automatically by Diesel CLI. + +diesel::table! { + users (id) { + id -> Bigint, + name -> Text, + } +} diff --git a/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/up.sql/expected.snap new file mode 100644 index 000000000000..f384ae4bfca8 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/mysql/up.sql/expected.snap @@ -0,0 +1,9 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_add_table" +--- +-- Your SQL goes here +CREATE TABLE `users`( + `id` BIGINT NOT NULL PRIMARY KEY, + `name` TEXT NOT NULL +); diff --git a/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/down.sql/expected.snap new file mode 100644 index 000000000000..432f142620b2 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/down.sql/expected.snap @@ -0,0 +1,8 @@ +--- +source: diesel_cli/tests/migration_generate.rs +assertion_line: 300 +description: "Test: diff_add_table" +--- +-- This file should undo anything in `up.sql` +DROP TABLE IF EXISTS "users"; + diff --git a/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/initial_schema.sql b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/initial_schema.sql new file mode 100644 index 000000000000..8b137891791f --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/initial_schema.sql @@ -0,0 +1 @@ + diff --git a/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/schema_out.rs/expected.snap new file mode 100644 index 000000000000..cb7df36b6aec --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/schema_out.rs/expected.snap @@ -0,0 +1,13 @@ +--- +source: diesel_cli/tests/migration_generate.rs +assertion_line: 323 +description: "Test: diff_add_table" +--- +// @generated automatically by Diesel CLI. + +diesel::table! { + users (id) { + id -> Int8, + name -> Text, + } +} diff --git a/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/up.sql/expected.snap new file mode 100644 index 000000000000..561ed4c34775 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/postgres/up.sql/expected.snap @@ -0,0 +1,10 @@ +--- +source: diesel_cli/tests/migration_generate.rs +assertion_line: 295 +description: "Test: diff_add_table" +--- +-- Your SQL goes here +CREATE TABLE "users"( + "id" BIGINT NOT NULL PRIMARY KEY, + "name" TEXT NOT NULL +); diff --git a/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/schema.rs b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/schema.rs new file mode 100644 index 000000000000..bcb1d8b16565 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/schema.rs @@ -0,0 +1,6 @@ +table! { + users { + id -> BigInt, + name -> Text, + } +} diff --git a/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/down.sql/expected.snap new file mode 100644 index 000000000000..6bf6dc40cefc --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/down.sql/expected.snap @@ -0,0 +1,7 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_add_table" +--- +-- This file should undo anything in `up.sql` +DROP TABLE IF EXISTS `users`; + diff --git a/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/initial_schema.sql b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/initial_schema.sql new file mode 100644 index 000000000000..8b137891791f --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/initial_schema.sql @@ -0,0 +1 @@ + diff --git a/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/schema_out.rs/expected.snap new file mode 100644 index 000000000000..0f8d10baa63e --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/schema_out.rs/expected.snap @@ -0,0 +1,12 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_add_table" +--- +// @generated automatically by Diesel CLI. + +diesel::table! { + users (id) { + id -> BigInt, + name -> Text, + } +} diff --git a/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/up.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/up.sql/expected.snap new file mode 100644 index 000000000000..a53abfe52623 --- /dev/null +++ b/diesel_cli/tests/generate_migrations/diff_add_table_sqlite_rowid_column/sqlite/up.sql/expected.snap @@ -0,0 +1,10 @@ +--- +source: diesel_cli/tests/migration_generate.rs +description: "Test: diff_add_table" +--- +-- Your SQL goes here +CREATE TABLE `users`( + `id` INTEGER NOT NULL PRIMARY KEY, + `name` TEXT NOT NULL +); + diff --git a/diesel_cli/tests/migration_generate.rs b/diesel_cli/tests/migration_generate.rs index 1c6deafa663b..fd2169b24615 100644 --- a/diesel_cli/tests/migration_generate.rs +++ b/diesel_cli/tests/migration_generate.rs @@ -220,6 +220,14 @@ fn migration_generate_from_diff_add_table() { test_generate_migration("diff_add_table", Vec::new()); } +#[test] +fn migration_generate_from_diff_add_table_sqlite_rowid_column() { + test_generate_migration( + "diff_add_table_sqlite_rowid_column", + vec!["--sqlite-integer-primary-key-is-bigint"], + ); +} + #[test] fn migration_generate_from_diff_drop_alter_table_add_column() { test_generate_migration("diff_alter_table_add_column", Vec::new()); diff --git a/diesel_cli/tests/print_schema.rs b/diesel_cli/tests/print_schema.rs index dd298713b81a..e867a2e09edf 100644 --- a/diesel_cli/tests/print_schema.rs +++ b/diesel_cli/tests/print_schema.rs @@ -250,6 +250,15 @@ fn print_schema_generated_columns_with_generated_always() { test_print_schema("print_schema_generated_columns_generated_always", vec![]) } +#[test] +#[cfg(feature = "sqlite")] +fn print_schema_sqlite_rowid_column() { + test_print_schema( + "print_schema_sqlite_rowid_column", + vec!["--sqlite-integer-primary-key-is-bigint"], + ) +} + #[test] #[cfg(feature = "postgres")] fn print_schema_multiple_annotations() { diff --git a/diesel_cli/tests/print_schema/print_schema_sqlite_rowid_column/diesel.toml b/diesel_cli/tests/print_schema/print_schema_sqlite_rowid_column/diesel.toml new file mode 100644 index 000000000000..e914c1be0c32 --- /dev/null +++ b/diesel_cli/tests/print_schema/print_schema_sqlite_rowid_column/diesel.toml @@ -0,0 +1,4 @@ +[print_schema] +file = "src/schema.rs" +with_docs = false +sqlite_integer_primary_key_is_bigint = true diff --git a/diesel_cli/tests/print_schema/print_schema_sqlite_rowid_column/sqlite/expected.snap b/diesel_cli/tests/print_schema/print_schema_sqlite_rowid_column/sqlite/expected.snap new file mode 100644 index 000000000000..543c6a8a50df --- /dev/null +++ b/diesel_cli/tests/print_schema/print_schema_sqlite_rowid_column/sqlite/expected.snap @@ -0,0 +1,30 @@ +--- +source: diesel_cli/tests/print_schema.rs +description: "Test: print_schema_sqlite_rowid_column" +--- +// @generated automatically by Diesel CLI. + +diesel::table! { + users1 (id) { + id -> BigInt, + } +} + +diesel::table! { + users2 (id) { + id -> Integer, + } +} + +diesel::table! { + users3 (rowid) { + rowid -> BigInt, + name -> Text, + } +} + +diesel::allow_tables_to_appear_in_same_query!( + users1, + users2, + users3, +); diff --git a/diesel_cli/tests/print_schema/print_schema_sqlite_rowid_column/sqlite/schema.sql b/diesel_cli/tests/print_schema/print_schema_sqlite_rowid_column/sqlite/schema.sql new file mode 100644 index 000000000000..21c107a7a110 --- /dev/null +++ b/diesel_cli/tests/print_schema/print_schema_sqlite_rowid_column/sqlite/schema.sql @@ -0,0 +1,3 @@ +CREATE TABLE users1 (id INTEGER NOT NULL PRIMARY KEY); +CREATE TABLE users2 (id INTEGER NOT NULL PRIMARY KEY) WITHOUT ROWID; +CREATE TABLE users3 (name TEXT NOT NULL);