From 2b03ce3d31613db14dcac23644eb360f4bc28c94 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Wed, 19 May 2021 11:34:54 +0200 Subject: [PATCH 1/8] Generate SQL type definitions for unknown types This commit adds support for generating SQL type definitions for unknown types to `diesel print-schema`. The basic idea is to generate the corresponding marker type to always end up with an existing schema. Especially this does not generate any code that is required for serializing/deserializing rust values. --- CHANGELOG.md | 4 +- diesel/src/macros/mod.rs | 4 +- diesel/src/pg/types/mod.rs | 1 - diesel_cli/src/cli.rs | 7 + diesel_cli/src/config.rs | 6 + diesel_cli/src/database.rs | 4 +- .../infer_schema_internals/data_structures.rs | 3 +- .../src/infer_schema_internals/mysql.rs | 1 + diesel_cli/src/infer_schema_internals/pg.rs | 1 + .../src/infer_schema_internals/sqlite.rs | 3 +- diesel_cli/src/main.rs | 4 + diesel_cli/src/print_schema.rs | 197 +++++++++++++++++- diesel_cli/tests/print_schema.rs | 27 +++ .../diesel.toml | 3 + .../postgres/expected.rs | 31 +++ .../postgres/schema.sql | 5 + .../diesel.toml | 3 + .../postgres/expected.rs | 8 + .../postgres/schema.sql | 5 + .../postgres/expected.rs | 10 + .../diesel.toml | 2 + .../postgres/expected.rs | 15 ++ .../postgres/schema.sql | 5 + 23 files changed, 333 insertions(+), 16 deletions(-) create mode 100644 diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/diesel.toml create mode 100644 diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs create mode 100644 diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/schema.sql create mode 100644 diesel_cli/tests/print_schema/print_schema_disabling_custom_type_works/diesel.toml create mode 100644 diesel_cli/tests/print_schema/print_schema_disabling_custom_type_works/postgres/expected.rs create mode 100644 diesel_cli/tests/print_schema/print_schema_disabling_custom_type_works/postgres/schema.sql create mode 100644 diesel_cli/tests/print_schema/schema_file_contains_custom_types/diesel.toml create mode 100644 diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/expected.rs create mode 100644 diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/schema.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index 4daecf91e391..890d51314bab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,8 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/ * Added support for SQL functions without arguments for SQLite. +* 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. + ### Removed * All previously deprecated items have been removed. @@ -171,8 +173,6 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/ * The `#[table_name]` attribute for derive macros can now refer to any path and is no longer limited to identifiers from the current scope. -* Interacting with a database requires a mutable connection. - ### Fixed * Many types were incorrectly considered non-aggregate when they should not diff --git a/diesel/src/macros/mod.rs b/diesel/src/macros/mod.rs index cbc789017e63..ee61f73f44ca 100644 --- a/diesel/src/macros/mod.rs +++ b/diesel/src/macros/mod.rs @@ -616,10 +616,10 @@ macro_rules! __diesel_table_impl { },)+], ) => { $($meta)* + #[allow(unused_imports)] pub mod $table_name { - #![allow(dead_code)] - $($imports)* pub use self::columns::*; + $($imports)* /// Re-exports all of the columns of this table, as well as the /// table struct renamed to the module name. This is meant to be diff --git a/diesel/src/pg/types/mod.rs b/diesel/src/pg/types/mod.rs index 5cf705428ccb..81e843de4a15 100644 --- a/diesel/src/pg/types/mod.rs +++ b/diesel/src/pg/types/mod.rs @@ -183,7 +183,6 @@ pub mod sql_types { pub type BigSerial = crate::sql_types::BigInt; /// The `UUID` SQL type. This type can only be used with `feature = "uuid"` - /// (uuid <=0.6) or `feature = "uuidv07"` (uuid = 0.7) /// /// ### [`ToSql`] impls /// diff --git a/diesel_cli/src/cli.rs b/diesel_cli/src/cli.rs index 4ef475a9e70c..b5bcdfbbdcec 100644 --- a/diesel_cli/src/cli.rs +++ b/diesel_cli/src/cli.rs @@ -218,6 +218,13 @@ pub fn build_cli() -> App<'static, 'static> { .multiple(true) .number_of_values(1) .help("A list of types to import for every table, separated by commas."), + ) + .arg( + Arg::with_name("generate-custom-type-definitions") + .long("generate-custom-type-definitions") + .takes_value(true) + .possible_values(&["true", "false"]) + .help("Generate SQL type definitions for types not provided by diesel"), ); let config_arg = Arg::with_name("CONFIG_FILE") diff --git a/diesel_cli/src/config.rs b/diesel_cli/src/config.rs index 6ff3420f8b49..8d97386b928b 100644 --- a/diesel_cli/src/config.rs +++ b/diesel_cli/src/config.rs @@ -67,9 +67,15 @@ pub struct PrintSchema { pub patch_file: Option, #[serde(default)] pub import_types: Option>, + #[serde(default)] + pub generate_missing_sql_type_definitions: Option, } impl PrintSchema { + pub fn generate_missing_sql_type_definitions(&self) -> bool { + self.generate_missing_sql_type_definitions.unwrap_or(true) + } + pub fn schema_name(&self) -> Option<&str> { self.schema.as_deref() } diff --git a/diesel_cli/src/database.rs b/diesel_cli/src/database.rs index d75ad26e552c..d50314cabcd2 100644 --- a/diesel_cli/src/database.rs +++ b/diesel_cli/src/database.rs @@ -16,7 +16,7 @@ use std::fs::{self, File}; use std::io::Write; use std::path::Path; -enum Backend { +pub enum Backend { #[cfg(feature = "postgres")] Pg, #[cfg(feature = "sqlite")] @@ -26,7 +26,7 @@ enum Backend { } impl Backend { - fn for_url(database_url: &str) -> Self { + pub fn for_url(database_url: &str) -> Self { match database_url { _ if database_url.starts_with("postgres://") || database_url.starts_with("postgresql://") => diff --git a/diesel_cli/src/infer_schema_internals/data_structures.rs b/diesel_cli/src/infer_schema_internals/data_structures.rs index e8c693a38288..54f4d8bf1a73 100644 --- a/diesel_cli/src/infer_schema_internals/data_structures.rs +++ b/diesel_cli/src/infer_schema_internals/data_structures.rs @@ -15,9 +15,10 @@ pub struct ColumnInformation { pub nullable: bool, } -#[derive(Debug)] +#[derive(Debug, PartialEq, Clone)] pub struct ColumnType { pub rust_name: String, + pub sql_name: String, pub is_array: bool, pub is_nullable: bool, pub is_unsigned: bool, diff --git a/diesel_cli/src/infer_schema_internals/mysql.rs b/diesel_cli/src/infer_schema_internals/mysql.rs index 8e7d2685b75a..8418c8403af1 100644 --- a/diesel_cli/src/infer_schema_internals/mysql.rs +++ b/diesel_cli/src/infer_schema_internals/mysql.rs @@ -92,6 +92,7 @@ pub fn determine_column_type( let unsigned = determine_unsigned(&attr.type_name); Ok(ColumnType { + sql_name: tpe.trim().to_lowercase(), rust_name: tpe.trim().to_camel_case(), is_array: false, is_nullable: attr.nullable, diff --git a/diesel_cli/src/infer_schema_internals/pg.rs b/diesel_cli/src/infer_schema_internals/pg.rs index 0fcbc6e71659..03a2bdd62c38 100644 --- a/diesel_cli/src/infer_schema_internals/pg.rs +++ b/diesel_cli/src/infer_schema_internals/pg.rs @@ -30,6 +30,7 @@ pub fn determine_column_type( } Ok(ColumnType { + sql_name: tpe.to_lowercase(), rust_name: tpe.to_camel_case(), is_array, is_nullable: attr.nullable, diff --git a/diesel_cli/src/infer_schema_internals/sqlite.rs b/diesel_cli/src/infer_schema_internals/sqlite.rs index ae08665b413d..4e4c41b38bb7 100644 --- a/diesel_cli/src/infer_schema_internals/sqlite.rs +++ b/diesel_cli/src/infer_schema_internals/sqlite.rs @@ -172,7 +172,8 @@ pub fn determine_column_type( }; Ok(ColumnType { - rust_name: path, + rust_name: path.clone(), + sql_name: path, is_array: false, is_nullable: attr.nullable, is_unsigned: false, diff --git a/diesel_cli/src/main.rs b/diesel_cli/src/main.rs index e53dc83c1618..f426d1e52985 100644 --- a/diesel_cli/src/main.rs +++ b/diesel_cli/src/main.rs @@ -562,6 +562,10 @@ fn run_infer_schema(matches: &ArgMatches) -> Result<(), Box; @@ -64,6 +66,93 @@ pub fn run_print_schema( Ok(()) } +fn common_diesel_types(types: &mut HashSet<&str>) { + types.insert("Bool"); + types.insert("Integer"); + types.insert("SmallInt"); + types.insert("BigInt"); + types.insert("Binary"); + types.insert("Text"); + types.insert("Double"); + types.insert("Float"); + types.insert("Numeric"); + + // hidden type defs + types.insert("Float4"); + types.insert("Smallint"); + types.insert("Int2"); + types.insert("Int4"); + types.insert("Int8"); + types.insert("Bigint"); + types.insert("Float8"); + types.insert("Decimal"); + types.insert("VarChar"); + types.insert("Varchar"); + types.insert("Char"); + types.insert("Tinytext"); + types.insert("Mediumtext"); + types.insert("Longtext"); + types.insert("Tinyblob"); + types.insert("Blob"); + types.insert("Mediumblob"); + types.insert("Longblob"); + types.insert("Varbinary"); + types.insert("Bit"); +} + +#[cfg(feature = "postgres")] +fn pg_diesel_types() -> HashSet<&'static str> { + let mut types = HashSet::new(); + types.insert("Cidr"); + types.insert("Date"); + types.insert("Inet"); + types.insert("Jsonb"); + types.insert("MacAddr"); + types.insert("Money"); + types.insert("Oid"); + types.insert("Range"); + types.insert("Timestamptz"); + types.insert("Uuid"); + types.insert("Json"); + types.insert("Timestamp"); + types.insert("Record"); + types.insert("Interval"); + + // hidden type defs + types.insert("Int4range"); + types.insert("Int8range"); + types.insert("Daterange"); + types.insert("Numrange"); + types.insert("Tsrange"); + types.insert("Tstzrange"); + types.insert("SmallSerial"); + types.insert("BigSerial"); + types.insert("Serial"); + types.insert("Bytea"); + types.insert("Bpchar"); + types.insert("Macaddr"); + + common_diesel_types(&mut types); + types +} + +#[cfg(feature = "mysql")] +fn mysql_diesel_types() -> HashSet<&'static str> { + let mut types = HashSet::new(); + common_diesel_types(&mut types); + + types.insert("TinyInt"); + types.insert("Tinyint"); + types +} + +#[cfg(feature = "sqlite")] +fn sqlite_diesel_types() -> HashSet<&'static str> { + let mut types = HashSet::new(); + common_diesel_types(&mut types); + types +} + pub fn output_schema( database_url: &str, config: &config::PrintSchema, @@ -78,20 +167,54 @@ pub fn output_schema( let table_data = table_names .into_iter() .map(|t| load_table_data(database_url, t, &config.column_sorting)) - .collect::>>()?; + .collect::, Box>>()?; + + let mut out = String::new(); + writeln!(out, "{}", SCHEMA_HEADER)?; + + writeln!(out)?; + let backend = Backend::for_url(database_url); + + let custom_types = if config.generate_missing_sql_type_definitions() { + let diesel_provided_types = match backend { + #[cfg(feature = "postgres")] + Backend::Pg => pg_diesel_types(), + #[cfg(feature = "sqlite")] + Backend::Sqlite => sqlite_diesel_types(), + #[cfg(feature = "mysql")] + Backend::Mysql => mysql_diesel_types(), + }; + + let mut all_types = table_data + .iter() + .flat_map(|t| t.column_data.iter().map(|c| &c.ty)) + .filter(|t| !diesel_provided_types.contains(&t.rust_name as &str)) + .cloned() + .collect::>(); + + all_types.sort_unstable_by(|a, b| a.rust_name.cmp(&b.rust_name)); + all_types.dedup_by(|a, b| a.rust_name.eq(&b.rust_name)); + all_types + } else { + Vec::new() + }; + let definitions = TableDefinitions { tables: table_data, fk_constraints: foreign_keys, include_docs: config.with_docs, + custom_type_defs: CustomTypeList { + backend, + types: custom_types, + with_docs: config.with_docs, + }, import_types: config.import_types(), }; - let mut out = String::new(); - writeln!(out, "{}", SCHEMA_HEADER)?; - if let Some(schema_name) = config.schema_name() { write!(out, "{}", ModuleDefinition(schema_name, definitions))?; } else { + write!(out, "{}", definitions.custom_type_defs)?; write!(out, "{}", definitions)?; } @@ -105,12 +228,53 @@ pub fn output_schema( Ok(out) } +struct CustomTypeList { + backend: Backend, + types: Vec, + with_docs: bool, +} + +impl CustomTypeList { + fn contains(&self, tpe: &str) -> bool { + self.types.iter().find(|c| c.rust_name == tpe).is_some() + } +} + +impl Display for CustomTypeList { + fn fmt(&self, out: &mut Formatter<'_>) -> fmt::Result { + for t in &self.types { + match self.backend { + #[cfg(feature = "postgres")] + Backend::Pg => { + if self.with_docs { + writeln!(out, "/// The `{}` SQL type", t.rust_name)?; + writeln!(out, "///")?; + writeln!(out, "/// (Automatically generated by Diesel.)")?; + } + writeln!(out, "#[derive(diesel::SqlType)]")?; + writeln!(out, "#[postgres(type_name = \"{}\")]", t.sql_name)?; + writeln!(out, "pub struct {};", t.rust_name)?; + writeln!(out)?; + } + #[cfg(feature = "sqlite")] + Backend::Sqlite => { + unreachable!("We only generate a closed set of types for sqlite") + } + #[cfg(feature = "mysql")] + Backend::Mysql => todo!(), + } + } + Ok(()) + } +} + struct ModuleDefinition<'a>(&'a str, TableDefinitions<'a>); impl<'a> Display for ModuleDefinition<'a> { fn fmt(&self, f: &mut Formatter) -> fmt::Result { { let mut out = PadAdapter::new(f); + write!(out, "{}", self.1.custom_type_defs)?; writeln!(out, "pub mod {} {{", self.0)?; write!(out, "{}", self.1)?; } @@ -124,6 +288,7 @@ struct TableDefinitions<'a> { fk_constraints: Vec, include_docs: bool, import_types: Option<&'a [String]>, + custom_type_defs: CustomTypeList, } impl<'a> Display for TableDefinitions<'a> { @@ -142,6 +307,7 @@ impl<'a> Display for TableDefinitions<'a> { table, include_docs: self.include_docs, import_types: self.import_types, + custom_type_defs: &self.custom_type_defs } )?; } @@ -176,8 +342,9 @@ impl<'a> Display for TableDefinitions<'a> { struct TableDefinition<'a> { table: &'a TableData, - import_types: Option<&'a [String]>, include_docs: bool, + import_types: Option<&'a [String]>, + custom_type_defs: &'a CustomTypeList, } impl<'a> Display for TableDefinition<'a> { @@ -187,10 +354,26 @@ impl<'a> Display for TableDefinition<'a> { let mut out = PadAdapter::new(f); writeln!(out)?; + let mut has_written_import = false; if let Some(types) = self.import_types { for import in types { writeln!(out, "use {};", import)?; + has_written_import = true; + } + } + + for col in &self.table.column_data { + dbg!(&col.rust_name); + if dbg!(self.custom_type_defs.contains(&col.ty.rust_name)) { + if !has_written_import { + writeln!(out, "use diesel::sql_types::*;")?; + } + writeln!(out, "use super::{};", col.ty.rust_name)?; + has_written_import = true; } + } + + if has_written_import { writeln!(out)?; } diff --git a/diesel_cli/tests/print_schema.rs b/diesel_cli/tests/print_schema.rs index 2a9849446dd9..014ada318cba 100644 --- a/diesel_cli/tests/print_schema.rs +++ b/diesel_cli/tests/print_schema.rs @@ -171,6 +171,33 @@ fn schema_file_is_relative_to_project_root() { assert!(p.has_file("src/schema.rs")); } +#[test] +#[cfg(feature = "postgres")] +fn schema_file_contains_custom_types() { + test_print_schema( + "schema_file_contains_custom_types", + vec!["--generate-custom-type-definitions", "true"], + ) +} + +#[test] +#[cfg(feature = "postgres")] +fn print_schema_disabling_custom_type_works() { + test_print_schema( + "print_schema_disabling_custom_type_works", + vec!["--generate-custom-type-definitions", "false"], + ) +} + +#[test] +#[cfg(feature = "postgres")] +fn print_schema_default_is_to_generate_custom_types() { + test_print_schema( + "print_schema_default_is_to_generate_custom_types", + vec!["--with-docs"], + ) +} + #[cfg(feature = "sqlite")] const BACKEND: &str = "sqlite"; #[cfg(feature = "postgres")] diff --git a/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/diesel.toml b/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/diesel.toml new file mode 100644 index 000000000000..750e5ba85830 --- /dev/null +++ b/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/diesel.toml @@ -0,0 +1,3 @@ +[print_schema] +file = "src/schema.rs" +with_docs = true diff --git a/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs b/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs new file mode 100644 index 000000000000..bba1a2d2c13c --- /dev/null +++ b/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs @@ -0,0 +1,31 @@ +// @generated automatically by Diesel CLI. + +/// The `MyType` SQL type +/// +/// (Automatically generated by Diesel.) +#[derive(diesel::SqlType)] +#[postgres(type_name = "my_type")] +pub struct MyType; + +diesel::table! { + use diesel::sql_types::*; + use super::MyType; + + /// Representation of the `custom_types` table. + /// + /// (Automatically generated by Diesel.) + custom_types (id) { + /// The `id` column of the `custom_types` table. + /// + /// Its SQL type is `Int4`. + /// + /// (Automatically generated by Diesel.) + id -> Int4, + /// The `custom_enum` column of the `custom_types` table. + /// + /// Its SQL type is `MyType`. + /// + /// (Automatically generated by Diesel.) + custom_enum -> MyType, + } +} diff --git a/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/schema.sql b/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/schema.sql new file mode 100644 index 000000000000..16de9d2dcf63 --- /dev/null +++ b/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/schema.sql @@ -0,0 +1,5 @@ +CREATE TYPE my_type AS ENUM ('foo', 'bar'); +CREATE TABLE custom_types ( + id SERIAL PRIMARY KEY, + custom_enum my_type NOT NULL +); diff --git a/diesel_cli/tests/print_schema/print_schema_disabling_custom_type_works/diesel.toml b/diesel_cli/tests/print_schema/print_schema_disabling_custom_type_works/diesel.toml new file mode 100644 index 000000000000..a9381ebda0ec --- /dev/null +++ b/diesel_cli/tests/print_schema/print_schema_disabling_custom_type_works/diesel.toml @@ -0,0 +1,3 @@ +[print_schema] +file = "src/schema.rs" +generate_missing_sql_type_definitions = false diff --git a/diesel_cli/tests/print_schema/print_schema_disabling_custom_type_works/postgres/expected.rs b/diesel_cli/tests/print_schema/print_schema_disabling_custom_type_works/postgres/expected.rs new file mode 100644 index 000000000000..1dc573bcc0f7 --- /dev/null +++ b/diesel_cli/tests/print_schema/print_schema_disabling_custom_type_works/postgres/expected.rs @@ -0,0 +1,8 @@ +// @generated automatically by Diesel CLI. + +diesel::table! { + custom_types (id) { + id -> Int4, + custom_enum -> MyType, + } +} diff --git a/diesel_cli/tests/print_schema/print_schema_disabling_custom_type_works/postgres/schema.sql b/diesel_cli/tests/print_schema/print_schema_disabling_custom_type_works/postgres/schema.sql new file mode 100644 index 000000000000..16de9d2dcf63 --- /dev/null +++ b/diesel_cli/tests/print_schema/print_schema_disabling_custom_type_works/postgres/schema.sql @@ -0,0 +1,5 @@ +CREATE TYPE my_type AS ENUM ('foo', 'bar'); +CREATE TABLE custom_types ( + id SERIAL PRIMARY KEY, + custom_enum my_type NOT NULL +); diff --git a/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs b/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs index 1e0a66310d27..272165b8e5e8 100644 --- a/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs +++ b/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs @@ -1,6 +1,16 @@ // @generated automatically by Diesel CLI. +/// The `UserJob` SQL type +/// +/// (Automatically generated by Diesel.) +#[derive(diesel::SqlType)] +#[postgres(type_name = "user_job")] +pub struct UserJob; + diesel::table! { + use diesel::sql_types::*; + use super::UserJob; + /// Representation of the `users` table. /// /// (Automatically generated by Diesel.) diff --git a/diesel_cli/tests/print_schema/schema_file_contains_custom_types/diesel.toml b/diesel_cli/tests/print_schema/schema_file_contains_custom_types/diesel.toml new file mode 100644 index 000000000000..f57985adb185 --- /dev/null +++ b/diesel_cli/tests/print_schema/schema_file_contains_custom_types/diesel.toml @@ -0,0 +1,2 @@ +[print_schema] +file = "src/schema.rs" diff --git a/diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/expected.rs b/diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/expected.rs new file mode 100644 index 000000000000..f810db35c785 --- /dev/null +++ b/diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/expected.rs @@ -0,0 +1,15 @@ +// @generated automatically by Diesel CLI. + +#[derive(diesel::SqlType)] +#[postgres(type_name = "my_type")] +pub struct MyType; + +diesel::table! { + use diesel::sql_types::*; + use super::MyType; + + custom_types (id) { + id -> Int4, + custom_enum -> MyType, + } +} diff --git a/diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/schema.sql b/diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/schema.sql new file mode 100644 index 000000000000..16de9d2dcf63 --- /dev/null +++ b/diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/schema.sql @@ -0,0 +1,5 @@ +CREATE TYPE my_type AS ENUM ('foo', 'bar'); +CREATE TABLE custom_types ( + id SERIAL PRIMARY KEY, + custom_enum my_type NOT NULL +); From ce201886fa6e6d9cceb11d551c47aa4ad83fb3f3 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Fri, 18 Jun 2021 18:26:38 +0200 Subject: [PATCH 2/8] Address review comments --- diesel/src/macros/mod.rs | 2 +- diesel_cli/src/cli.rs | 4 +-- diesel_cli/src/main.rs | 4 +-- diesel_cli/src/print_schema.rs | 33 ++++++++++++++----- diesel_cli/tests/print_schema.rs | 11 +------ .../postgres/expected.rs | 15 ++++++--- .../postgres/expected.rs | 15 ++++++--- 7 files changed, 50 insertions(+), 34 deletions(-) diff --git a/diesel/src/macros/mod.rs b/diesel/src/macros/mod.rs index ee61f73f44ca..12fa206aecc6 100644 --- a/diesel/src/macros/mod.rs +++ b/diesel/src/macros/mod.rs @@ -616,7 +616,7 @@ macro_rules! __diesel_table_impl { },)+], ) => { $($meta)* - #[allow(unused_imports)] + #[allow(unused_imports, dead_code)] pub mod $table_name { pub use self::columns::*; $($imports)* diff --git a/diesel_cli/src/cli.rs b/diesel_cli/src/cli.rs index b5bcdfbbdcec..e4ca5d4b8ded 100644 --- a/diesel_cli/src/cli.rs +++ b/diesel_cli/src/cli.rs @@ -221,9 +221,7 @@ pub fn build_cli() -> App<'static, 'static> { ) .arg( Arg::with_name("generate-custom-type-definitions") - .long("generate-custom-type-definitions") - .takes_value(true) - .possible_values(&["true", "false"]) + .long("no-generate-missing-sql-type-definitions") .help("Generate SQL type definitions for types not provided by diesel"), ); diff --git a/diesel_cli/src/main.rs b/diesel_cli/src/main.rs index f426d1e52985..f777b4bf54aa 100644 --- a/diesel_cli/src/main.rs +++ b/diesel_cli/src/main.rs @@ -562,8 +562,8 @@ fn run_infer_schema(matches: &ArgMatches) -> Result<(), Box bool { self.types.iter().find(|c| c.rust_name == tpe).is_some() } } impl Display for CustomTypeList { - fn fmt(&self, out: &mut Formatter<'_>) -> fmt::Result { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + if self.types.is_empty() { + return Ok(()); + } for t in &self.types { match self.backend { #[cfg(feature = "postgres")] Backend::Pg => { if self.with_docs { - writeln!(out, "/// The `{}` SQL type", t.rust_name)?; + writeln!(f, "/// A module containing custom SQL type definitions")?; + writeln!(f, "///")?; + writeln!(f, "/// (Automatically generated by Diesel.)")?; + } + let mut out = PadAdapter::new(f); + writeln!(out, "pub mod sql_types {{")?; + if self.with_docs { + writeln!(out, "/// The `{}` SQL type", t.sql_name)?; writeln!(out, "///")?; writeln!(out, "/// (Automatically generated by Diesel.)")?; } @@ -255,13 +267,16 @@ impl Display for CustomTypeList { writeln!(out, "#[postgres(type_name = \"{}\")]", t.sql_name)?; writeln!(out, "pub struct {};", t.rust_name)?; writeln!(out)?; + writeln!(f, "}}")?; } #[cfg(feature = "sqlite")] Backend::Sqlite => { - unreachable!("We only generate a closed set of types for sqlite") + let _ = (&f, self.with_docs, t); } #[cfg(feature = "mysql")] - Backend::Mysql => todo!(), + Backend::Mysql => { + let _ = (&f, self.with_docs, t); + } } } Ok(()) @@ -362,16 +377,18 @@ impl<'a> Display for TableDefinition<'a> { } } + #[cfg(feature = "postgres")] for col in &self.table.column_data { - dbg!(&col.rust_name); - if dbg!(self.custom_type_defs.contains(&col.ty.rust_name)) { + if self.custom_type_defs.contains(&col.ty.rust_name) { if !has_written_import { writeln!(out, "use diesel::sql_types::*;")?; } - writeln!(out, "use super::{};", col.ty.rust_name)?; + writeln!(out, "use super::sql_types::{};", col.ty.rust_name)?; has_written_import = true; } } + #[cfg(not(feature = "postgres"))] + let _ = self.custom_type_defs; if has_written_import { writeln!(out)?; diff --git a/diesel_cli/tests/print_schema.rs b/diesel_cli/tests/print_schema.rs index 014ada318cba..bb5dc27ceae5 100644 --- a/diesel_cli/tests/print_schema.rs +++ b/diesel_cli/tests/print_schema.rs @@ -171,21 +171,12 @@ fn schema_file_is_relative_to_project_root() { assert!(p.has_file("src/schema.rs")); } -#[test] -#[cfg(feature = "postgres")] -fn schema_file_contains_custom_types() { - test_print_schema( - "schema_file_contains_custom_types", - vec!["--generate-custom-type-definitions", "true"], - ) -} - #[test] #[cfg(feature = "postgres")] fn print_schema_disabling_custom_type_works() { test_print_schema( "print_schema_disabling_custom_type_works", - vec!["--generate-custom-type-definitions", "false"], + vec!["--no-generate-missing-sql-type-definitions"], ) } diff --git a/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs b/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs index bba1a2d2c13c..2d014c18d149 100644 --- a/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs +++ b/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs @@ -1,15 +1,20 @@ // @generated automatically by Diesel CLI. -/// The `MyType` SQL type +/// A module containing custom SQL type definitions /// /// (Automatically generated by Diesel.) -#[derive(diesel::SqlType)] -#[postgres(type_name = "my_type")] -pub struct MyType; +pub mod sql_types { + /// The `my_type` SQL type + /// + /// (Automatically generated by Diesel.) + #[derive(diesel::SqlType)] + #[postgres(type_name = "my_type")] + pub struct MyType; +} diesel::table! { use diesel::sql_types::*; - use super::MyType; + use super::sql_types::MyType; /// Representation of the `custom_types` table. /// diff --git a/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs b/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs index 272165b8e5e8..58379e4d98d8 100644 --- a/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs +++ b/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs @@ -1,15 +1,20 @@ // @generated automatically by Diesel CLI. -/// The `UserJob` SQL type +/// A module containing custom SQL type definitions /// /// (Automatically generated by Diesel.) -#[derive(diesel::SqlType)] -#[postgres(type_name = "user_job")] -pub struct UserJob; +pub mod sql_types { + /// The `user_job` SQL type + /// + /// (Automatically generated by Diesel.) + #[derive(diesel::SqlType)] + #[postgres(type_name = "user_job")] + pub struct UserJob; +} diesel::table! { use diesel::sql_types::*; - use super::UserJob; + use super::sql_types::UserJob; /// Representation of the `users` table. /// From df0cd1b91c54d41cd40e7f493d67a8ba3eee0471 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Tue, 22 Jun 2021 15:26:29 +0200 Subject: [PATCH 3/8] Correctly generate custom type definitions for types in a custom schema --- diesel_cli/src/cli.rs | 2 +- .../infer_schema_internals/data_structures.rs | 18 +++++-- .../src/infer_schema_internals/inference.rs | 10 ++-- .../information_schema.rs | 49 ++++++++++++++++--- .../src/infer_schema_internals/mysql.rs | 1 + diesel_cli/src/infer_schema_internals/pg.rs | 8 +++ .../src/infer_schema_internals/sqlite.rs | 1 + diesel_cli/src/print_schema.rs | 47 +++++++++++++----- diesel_cli/tests/print_schema.rs | 9 ++++ .../diesel.toml | 4 ++ .../postgres/expected.rs | 38 ++++++++++++++ .../postgres/schema.sql | 5 ++ 12 files changed, 165 insertions(+), 27 deletions(-) create mode 100644 diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/diesel.toml create mode 100644 diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/postgres/expected.rs create mode 100644 diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/postgres/schema.sql diff --git a/diesel_cli/src/cli.rs b/diesel_cli/src/cli.rs index e4ca5d4b8ded..dd7174c7b8ea 100644 --- a/diesel_cli/src/cli.rs +++ b/diesel_cli/src/cli.rs @@ -218,7 +218,7 @@ pub fn build_cli() -> App<'static, 'static> { .multiple(true) .number_of_values(1) .help("A list of types to import for every table, separated by commas."), - ) + ) .arg( Arg::with_name("generate-custom-type-definitions") .long("no-generate-missing-sql-type-definitions") diff --git a/diesel_cli/src/infer_schema_internals/data_structures.rs b/diesel_cli/src/infer_schema_internals/data_structures.rs index 54f4d8bf1a73..735f141edb23 100644 --- a/diesel_cli/src/infer_schema_internals/data_structures.rs +++ b/diesel_cli/src/infer_schema_internals/data_structures.rs @@ -12,11 +12,13 @@ use super::table_data::TableName; pub struct ColumnInformation { pub column_name: String, pub type_name: String, + pub type_schema: Option, pub nullable: bool, } #[derive(Debug, PartialEq, Clone)] pub struct ColumnType { + pub schema: Option, pub rust_name: String, pub sql_name: String, pub is_array: bool, @@ -60,7 +62,12 @@ pub struct ColumnDefinition { } impl ColumnInformation { - pub fn new(column_name: T, type_name: U, nullable: bool) -> Self + pub fn new( + column_name: T, + type_name: U, + type_schema: Option, + nullable: bool, + ) -> Self where T: Into, U: Into, @@ -68,6 +75,7 @@ impl ColumnInformation { ColumnInformation { column_name: column_name.into(), type_name: type_name.into(), + type_schema, nullable, } } @@ -77,12 +85,12 @@ impl ColumnInformation { impl Queryable for ColumnInformation where DB: Backend + UsesInformationSchema, - (String, String, String): FromStaticSqlRow, + (String, String, Option, String): FromStaticSqlRow, { - type Row = (String, String, String); + type Row = (String, String, Option, String); fn build(row: Self::Row) -> deserialize::Result { - Ok(ColumnInformation::new(row.0, row.1, row.2 == "YES")) + Ok(ColumnInformation::new(row.0, row.1, row.2, row.3 == "YES")) } } @@ -94,7 +102,7 @@ where type Row = (i32, String, String, bool, Option, bool); fn build(row: Self::Row) -> deserialize::Result { - Ok(ColumnInformation::new(row.1, row.2, !row.3)) + Ok(ColumnInformation::new(row.1, row.2, None, !row.3)) } } diff --git a/diesel_cli/src/infer_schema_internals/inference.rs b/diesel_cli/src/infer_schema_internals/inference.rs index ccf8b2de5e44..cde7762dc48b 100644 --- a/diesel_cli/src/infer_schema_internals/inference.rs +++ b/diesel_cli/src/infer_schema_internals/inference.rs @@ -105,13 +105,17 @@ fn get_column_information( fn determine_column_type( attr: &ColumnInformation, - conn: &InferConnection, + conn: &mut InferConnection, ) -> Result> { match *conn { #[cfg(feature = "sqlite")] InferConnection::Sqlite(_) => super::sqlite::determine_column_type(attr), #[cfg(feature = "postgres")] - InferConnection::Pg(_) => super::pg::determine_column_type(attr), + InferConnection::Pg(ref mut conn) => { + use crate::infer_schema_internals::information_schema::UsesInformationSchema; + + super::pg::determine_column_type(attr, diesel::pg::Pg::default_schema(conn)?) + } #[cfg(feature = "mysql")] InferConnection::Mysql(_) => super::mysql::determine_column_type(attr), } @@ -206,7 +210,7 @@ pub fn load_table_data( let column_data = get_column_information(&mut connection, &name, column_sorting)? .into_iter() .map(|c| { - let ty = determine_column_type(&c, &connection)?; + let ty = determine_column_type(&c, &mut connection)?; let rust_name = rust_name_for_sql_name(&c.column_name); Ok(ColumnDefinition { diff --git a/diesel_cli/src/infer_schema_internals/information_schema.rs b/diesel_cli/src/infer_schema_internals/information_schema.rs index 07842db356d4..698f604eb285 100644 --- a/diesel_cli/src/infer_schema_internals/information_schema.rs +++ b/diesel_cli/src/infer_schema_internals/information_schema.rs @@ -4,7 +4,7 @@ use std::error::Error; use diesel::backend::Backend; use diesel::deserialize::{FromSql, FromSqlRow}; use diesel::dsl::*; -use diesel::expression::{is_aggregate, QueryMetadata, ValidGrouping}; +use diesel::expression::{is_aggregate, MixedAggregates, QueryMetadata, ValidGrouping}; #[cfg(feature = "mysql")] use diesel::mysql::Mysql; #[cfg(feature = "postgres")] @@ -24,7 +24,16 @@ pub trait UsesInformationSchema: Backend { + QueryId + QueryFragment; + type TypeSchema: SelectableExpression< + self::information_schema::columns::table, + SqlType = sql_types::Nullable, + > + ValidGrouping<()> + + QueryId + + QueryFragment; + fn type_column() -> Self::TypeColumn; + fn type_schema() -> Self::TypeSchema; + fn default_schema(conn: &mut C) -> QueryResult where C: Connection, @@ -34,11 +43,16 @@ pub trait UsesInformationSchema: Backend { #[cfg(feature = "postgres")] impl UsesInformationSchema for Pg { type TypeColumn = self::information_schema::columns::udt_name; + type TypeSchema = diesel::dsl::Nullable; fn type_column() -> Self::TypeColumn { self::information_schema::columns::udt_name } + fn type_schema() -> Self::TypeSchema { + self::information_schema::columns::udt_schema.nullable() + } + fn default_schema(_conn: &mut C) -> QueryResult { Ok("public".into()) } @@ -50,11 +64,16 @@ sql_function!(fn database() -> VarChar); #[cfg(feature = "mysql")] impl UsesInformationSchema for Mysql { type TypeColumn = self::information_schema::columns::column_type; + type TypeSchema = diesel::dsl::AsExprOf, sql_types::Nullable>; fn type_column() -> Self::TypeColumn { self::information_schema::columns::column_type } + fn type_schema() -> Self::TypeSchema { + None.into_sql() + } + fn default_schema(conn: &mut C) -> QueryResult where C: Connection, @@ -85,6 +104,7 @@ mod information_schema { __is_nullable -> VarChar, ordinal_position -> BigInt, udt_name -> VarChar, + udt_schema -> VarChar, column_type -> VarChar, } } @@ -135,11 +155,17 @@ where SqlTypeOf<( columns::column_name, ::TypeColumn, + ::TypeSchema, columns::__is_nullable, )>, Conn::Backend, >, + is_aggregate::No: MixedAggregates< + <::TypeSchema as ValidGrouping<()>>::IsAggregate, + Output = is_aggregate::No, + >, String: FromSql, + Option: FromSql, Conn::Backend>, Order< Filter< Filter< @@ -148,6 +174,7 @@ where ( columns::column_name, ::TypeColumn, + ::TypeSchema, columns::__is_nullable, ), >, @@ -165,6 +192,7 @@ where ( columns::column_name, ::TypeColumn, + ::TypeSchema, columns::__is_nullable, ), >, @@ -174,7 +202,12 @@ where >, columns::column_name, >: QueryFragment, - Conn::Backend: QueryMetadata<(sql_types::Text, sql_types::Text, sql_types::Text)>, + Conn::Backend: QueryMetadata<( + sql_types::Text, + sql_types::Text, + sql_types::Nullable, + sql_types::Text, + )>, { use self::information_schema::columns::dsl::*; @@ -184,8 +217,9 @@ where }; let type_column = Conn::Backend::type_column(); + let type_schema = Conn::Backend::type_schema(); let query = columns - .select((column_name, type_column, __is_nullable)) + .select((column_name, type_column, type_schema, __is_nullable)) .filter(table_name.eq(&table.sql_name)) .filter(table_schema.eq(schema_name)); match column_sorting { @@ -512,10 +546,11 @@ mod tests { let table_1 = TableName::new("table_1", "test_schema"); let table_2 = TableName::new("table_2", "test_schema"); - let id = ColumnInformation::new("id", "int4", false); - let text_col = ColumnInformation::new("text_col", "varchar", true); - let not_null = ColumnInformation::new("not_null", "text", false); - let array_col = ColumnInformation::new("array_col", "_varchar", false); + let pg_catalog = Some(String::from("pg_catalog")); + let id = ColumnInformation::new("id", "int4", pg_catalog.clone(), false); + let text_col = ColumnInformation::new("text_col", "varchar", pg_catalog.clone(), true); + let not_null = ColumnInformation::new("not_null", "text", pg_catalog.clone(), false); + let array_col = ColumnInformation::new("array_col", "_varchar", pg_catalog.clone(), false); assert_eq!( Ok(vec![id, text_col, not_null]), get_table_data(&mut connection, &table_1, &ColumnSorting::OrdinalPosition) diff --git a/diesel_cli/src/infer_schema_internals/mysql.rs b/diesel_cli/src/infer_schema_internals/mysql.rs index 8418c8403af1..5b4172ad8ccf 100644 --- a/diesel_cli/src/infer_schema_internals/mysql.rs +++ b/diesel_cli/src/infer_schema_internals/mysql.rs @@ -92,6 +92,7 @@ pub fn determine_column_type( let unsigned = determine_unsigned(&attr.type_name); Ok(ColumnType { + schema: None, sql_name: tpe.trim().to_lowercase(), rust_name: tpe.trim().to_camel_case(), is_array: false, diff --git a/diesel_cli/src/infer_schema_internals/pg.rs b/diesel_cli/src/infer_schema_internals/pg.rs index 03a2bdd62c38..cc4bf346e242 100644 --- a/diesel_cli/src/infer_schema_internals/pg.rs +++ b/diesel_cli/src/infer_schema_internals/pg.rs @@ -5,6 +5,7 @@ use std::io::{stderr, Write}; pub fn determine_column_type( attr: &ColumnInformation, + default_schema: String, ) -> Result> { let is_array = attr.type_name.starts_with('_'); let tpe = if is_array { @@ -30,6 +31,13 @@ pub fn determine_column_type( } Ok(ColumnType { + schema: attr.type_schema.as_ref().and_then(|s| { + if s == &default_schema { + None + } else { + Some(s.clone()) + } + }), sql_name: tpe.to_lowercase(), rust_name: tpe.to_camel_case(), is_array, diff --git a/diesel_cli/src/infer_schema_internals/sqlite.rs b/diesel_cli/src/infer_schema_internals/sqlite.rs index 4e4c41b38bb7..1f2009d71a1d 100644 --- a/diesel_cli/src/infer_schema_internals/sqlite.rs +++ b/diesel_cli/src/infer_schema_internals/sqlite.rs @@ -172,6 +172,7 @@ pub fn determine_column_type( }; Ok(ColumnType { + schema: None, rust_name: path.clone(), sql_name: path, is_array: false, diff --git a/diesel_cli/src/print_schema.rs b/diesel_cli/src/print_schema.rs index 554ce6444d66..ac0d39c40708 100644 --- a/diesel_cli/src/print_schema.rs +++ b/diesel_cli/src/print_schema.rs @@ -76,6 +76,9 @@ fn common_diesel_types(types: &mut HashSet<&str>) { types.insert("Double"); types.insert("Float"); types.insert("Numeric"); + types.insert("Timestamp"); + types.insert("Date"); + types.insert("Time"); // hidden type defs types.insert("Float4"); @@ -104,7 +107,6 @@ fn common_diesel_types(types: &mut HashSet<&str>) { fn pg_diesel_types() -> HashSet<&'static str> { let mut types = HashSet::new(); types.insert("Cidr"); - types.insert("Date"); types.insert("Inet"); types.insert("Jsonb"); types.insert("MacAddr"); @@ -114,7 +116,6 @@ fn pg_diesel_types() -> HashSet<&'static str> { types.insert("Timestamptz"); types.insert("Uuid"); types.insert("Json"); - types.insert("Timestamp"); types.insert("Record"); types.insert("Interval"); @@ -211,11 +212,10 @@ pub fn output_schema( import_types: config.import_types(), }; - write!(out, "{}", definitions.custom_type_defs)?; - if let Some(schema_name) = config.schema_name() { write!(out, "{}", ModuleDefinition(schema_name, definitions))?; } else { + write!(out, "{}", definitions.custom_type_defs)?; write!(out, "{}", definitions)?; } @@ -259,23 +259,48 @@ impl Display for CustomTypeList { let mut out = PadAdapter::new(f); writeln!(out, "pub mod sql_types {{")?; if self.with_docs { - writeln!(out, "/// The `{}` SQL type", t.sql_name)?; + if let Some(ref schema) = t.schema { + writeln!(out, "/// The `{}.{}` SQL type", schema, t.sql_name)?; + } else { + writeln!(out, "/// The `{}` SQL type", t.sql_name)?; + } writeln!(out, "///")?; writeln!(out, "/// (Automatically generated by Diesel.)")?; } writeln!(out, "#[derive(diesel::SqlType)]")?; - writeln!(out, "#[postgres(type_name = \"{}\")]", t.sql_name)?; + if let Some(ref schema) = t.schema { + writeln!( + out, + "#[postgres(type_name = \"{}\", type_schema = \"{}\")]", + t.sql_name, schema + )?; + } else { + writeln!(out, "#[postgres(type_name = \"{}\")]", t.sql_name)?; + } writeln!(out, "pub struct {};", t.rust_name)?; - writeln!(out)?; - writeln!(f, "}}")?; + writeln!(f, "}}\n")?; } #[cfg(feature = "sqlite")] Backend::Sqlite => { - let _ = (&f, self.with_docs, t); + let _ = (&f, self.with_docs); + eprintln!("Encountered unknown type for Sqlite: {}", t.sql_name); + unreachable!( + "Diesel only support a closed set of types for Sqlite. \ + If you ever see this error message please open an \ + issue at https://github.com/diesel-rs/diesel containing \ + a dump of your schema definition." + ) } #[cfg(feature = "mysql")] Backend::Mysql => { - let _ = (&f, self.with_docs, t); + let _ = (&f, self.with_docs); + eprintln!("Encountered unknown type for Mysql: {}", t.sql_name); + unreachable!( + "Mysql only supports a closed set of types. + If you ever see this error message please open an \ + issue at https://github.com/diesel-rs/diesel containing \ + a dump of your schema definition." + ) } } } @@ -289,8 +314,8 @@ impl<'a> Display for ModuleDefinition<'a> { fn fmt(&self, f: &mut Formatter) -> fmt::Result { { let mut out = PadAdapter::new(f); - write!(out, "{}", self.1.custom_type_defs)?; writeln!(out, "pub mod {} {{", self.0)?; + write!(out, "{}", self.1.custom_type_defs)?; write!(out, "{}", self.1)?; } writeln!(f, "}}")?; diff --git a/diesel_cli/tests/print_schema.rs b/diesel_cli/tests/print_schema.rs index bb5dc27ceae5..dc5dacf285a3 100644 --- a/diesel_cli/tests/print_schema.rs +++ b/diesel_cli/tests/print_schema.rs @@ -189,6 +189,15 @@ fn print_schema_default_is_to_generate_custom_types() { ) } +#[test] +#[cfg(feature = "postgres")] +fn print_schema_specifying_schema_name_with_custom_type() { + test_print_schema( + "print_schema_specifying_schema_name_with_custom_type", + vec!["--with-docs", "--schema", "custom_schema"], + ) +} + #[cfg(feature = "sqlite")] const BACKEND: &str = "sqlite"; #[cfg(feature = "postgres")] diff --git a/diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/diesel.toml b/diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/diesel.toml new file mode 100644 index 000000000000..c976c2ccdd93 --- /dev/null +++ b/diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/diesel.toml @@ -0,0 +1,4 @@ +[print_schema] +file = "src/schema.rs" +with_docs = true +schema = "custom_schema" diff --git a/diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/postgres/expected.rs b/diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/postgres/expected.rs new file mode 100644 index 000000000000..cbab01b9de03 --- /dev/null +++ b/diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/postgres/expected.rs @@ -0,0 +1,38 @@ +// @generated automatically by Diesel CLI. + +pub mod custom_schema { + /// A module containing custom SQL type definitions + /// + /// (Automatically generated by Diesel.) + pub mod sql_types { + /// The `custom_schema.my_enum` SQL type + /// + /// (Automatically generated by Diesel.) + #[derive(diesel::SqlType)] + #[postgres(type_name = "my_enum", type_schema = "custom_schema")] + pub struct MyEnum; + } + + diesel::table! { + use diesel::sql_types::*; + use super::sql_types::MyEnum; + + /// Representation of the `custom_schema.in_schema` table. + /// + /// (Automatically generated by Diesel.) + custom_schema.in_schema (id) { + /// The `id` column of the `custom_schema.in_schema` table. + /// + /// Its SQL type is `Int4`. + /// + /// (Automatically generated by Diesel.) + id -> Int4, + /// The `custom_type` column of the `custom_schema.in_schema` table. + /// + /// Its SQL type is `Nullable`. + /// + /// (Automatically generated by Diesel.) + custom_type -> Nullable, + } + } +} diff --git a/diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/postgres/schema.sql b/diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/postgres/schema.sql new file mode 100644 index 000000000000..21337d277288 --- /dev/null +++ b/diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/postgres/schema.sql @@ -0,0 +1,5 @@ +CREATE SCHEMA custom_schema; +CREATE TABLE in_public (id SERIAL PRIMARY KEY); +CREATE TYPE my_public_enum AS ENUM('A', 'B'); +CREATE TYPE custom_schema.my_enum AS ENUM ('A', 'B'); +CREATE TABLE custom_schema.in_schema (id SERIAL PRIMARY KEY, custom_type custom_schema.MY_ENUM); From a0b9c82b7d9c1befe9d80f94bc481319a7986cbb Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Tue, 22 Jun 2021 15:59:45 +0200 Subject: [PATCH 4/8] Clippy fix --- diesel_cli/src/print_schema.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/diesel_cli/src/print_schema.rs b/diesel_cli/src/print_schema.rs index ac0d39c40708..a71f0c81f676 100644 --- a/diesel_cli/src/print_schema.rs +++ b/diesel_cli/src/print_schema.rs @@ -238,7 +238,7 @@ struct CustomTypeList { impl CustomTypeList { #[cfg(feature = "postgres")] fn contains(&self, tpe: &str) -> bool { - self.types.iter().find(|c| c.rust_name == tpe).is_some() + self.types.iter().any(|c| c.rust_name == tpe) } } @@ -267,7 +267,7 @@ impl Display for CustomTypeList { writeln!(out, "///")?; writeln!(out, "/// (Automatically generated by Diesel.)")?; } - writeln!(out, "#[derive(diesel::SqlType)]")?; + writeln!(out, "#[derive(diesel::sql_types::SqlType)]")?; if let Some(ref schema) = t.schema { writeln!( out, From 5e7202176b9cfe31cb0bb6c0a669efefb438ead1 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Tue, 22 Jun 2021 16:25:32 +0200 Subject: [PATCH 5/8] We need to fix the import path for our custom type module as part of the `table!` macro otherwise it does not refer to the correct type --- diesel/src/macros/mod.rs | 18 +++++++++++++++++- examples/postgres/custom_types/diesel.toml | 1 - examples/postgres/custom_types/src/model.rs | 15 +++------------ examples/postgres/custom_types/src/schema.rs | 8 +++++++- 4 files changed, 27 insertions(+), 15 deletions(-) diff --git a/diesel/src/macros/mod.rs b/diesel/src/macros/mod.rs index 12fa206aecc6..cca129ce1cd9 100644 --- a/diesel/src/macros/mod.rs +++ b/diesel/src/macros/mod.rs @@ -11,6 +11,22 @@ pub(crate) mod prelude { }; } +#[macro_export] +#[doc(hidden)] +macro_rules! __diesel_fix_sql_type_import { + ($(use $($import:tt)::+;)*) => { + $( + $crate::__diesel_fix_sql_type_import!(@expand_import: $($import)::+); + )* + }; + (@expand_import: super:: $($Type:tt)+) => { + use super::super::$($Type)+; + }; + (@expand_import: $($Type:tt)+) => { + use $($Type)+; + } +} + #[macro_export] #[doc(hidden)] macro_rules! __diesel_column { @@ -797,7 +813,7 @@ macro_rules! __diesel_table_impl { /// Contains all of the columns of this table pub mod columns { use super::table; - $($imports)* + $crate::__diesel_fix_sql_type_import!($($imports)*); #[allow(non_camel_case_types, dead_code)] #[derive(Debug, Clone, Copy, $crate::query_builder::QueryId)] diff --git a/examples/postgres/custom_types/diesel.toml b/examples/postgres/custom_types/diesel.toml index 2cfe1c79deea..f57985adb185 100644 --- a/examples/postgres/custom_types/diesel.toml +++ b/examples/postgres/custom_types/diesel.toml @@ -1,3 +1,2 @@ [print_schema] file = "src/schema.rs" -import_types = ["diesel::sql_types::*", "crate::model::exports::*"] diff --git a/examples/postgres/custom_types/src/model.rs b/examples/postgres/custom_types/src/model.rs index 2889c93c32af..3648b3bc8d94 100644 --- a/examples/postgres/custom_types/src/model.rs +++ b/examples/postgres/custom_types/src/model.rs @@ -2,26 +2,17 @@ use diesel::deserialize::{self, FromSql, FromSqlRow}; use diesel::expression::AsExpression; use diesel::pg::{Pg, PgValue}; use diesel::serialize::{self, IsNull, Output, ToSql}; -use diesel::sql_types::SqlType; use std::io::Write; -pub mod exports { - pub use super::LanguageType as Language; -} - -#[derive(SqlType)] -#[postgres(type_name = "Language")] -pub struct LanguageType; - #[derive(Debug, AsExpression, FromSqlRow)] -#[sql_type = "LanguageType"] +#[sql_type = "crate::schema::sql_types::Language"] pub enum Language { En, Ru, De, } -impl ToSql for Language { +impl ToSql for Language { fn to_sql(&self, out: &mut Output) -> serialize::Result { match *self { Language::En => out.write_all(b"en")?, @@ -32,7 +23,7 @@ impl ToSql for Language { } } -impl FromSql for Language { +impl FromSql for Language { fn from_sql(bytes: PgValue) -> deserialize::Result { match bytes.as_bytes() { b"en" => Ok(Language::En), diff --git a/examples/postgres/custom_types/src/schema.rs b/examples/postgres/custom_types/src/schema.rs index e271250ec16e..0592a0aa13f1 100644 --- a/examples/postgres/custom_types/src/schema.rs +++ b/examples/postgres/custom_types/src/schema.rs @@ -1,8 +1,14 @@ // @generated automatically by Diesel CLI. +pub mod sql_types { + #[derive(diesel::sql_types::SqlType)] + #[postgres(type_name = "language")] + pub struct Language; +} + diesel::table! { use diesel::sql_types::*; - use crate::model::exports::*; + use super::sql_types::Language; translations (word_id, translation_id) { word_id -> Int4, From a36c5a82f4a2bf97ee9a48dcbb894697083475cc Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Tue, 22 Jun 2021 16:27:02 +0200 Subject: [PATCH 6/8] Fix the printschema tests --- .../postgres/expected.rs | 2 +- .../postgres/expected.rs | 2 +- .../postgres/expected.rs | 2 +- .../schema_file_contains_custom_types/diesel.toml | 2 -- .../postgres/expected.rs | 15 --------------- .../postgres/schema.sql | 5 ----- 6 files changed, 3 insertions(+), 25 deletions(-) delete mode 100644 diesel_cli/tests/print_schema/schema_file_contains_custom_types/diesel.toml delete mode 100644 diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/expected.rs delete mode 100644 diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/schema.sql diff --git a/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs b/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs index 2d014c18d149..4f1d8cb049a7 100644 --- a/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs +++ b/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs @@ -7,7 +7,7 @@ pub mod sql_types { /// The `my_type` SQL type /// /// (Automatically generated by Diesel.) - #[derive(diesel::SqlType)] + #[derive(diesel::sql_types::SqlType)] #[postgres(type_name = "my_type")] pub struct MyType; } diff --git a/diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/postgres/expected.rs b/diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/postgres/expected.rs index cbab01b9de03..e30b4ab7ab4c 100644 --- a/diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/postgres/expected.rs +++ b/diesel_cli/tests/print_schema/print_schema_specifying_schema_name_with_custom_type/postgres/expected.rs @@ -8,7 +8,7 @@ pub mod custom_schema { /// The `custom_schema.my_enum` SQL type /// /// (Automatically generated by Diesel.) - #[derive(diesel::SqlType)] + #[derive(diesel::sql_types::SqlType)] #[postgres(type_name = "my_enum", type_schema = "custom_schema")] pub struct MyEnum; } diff --git a/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs b/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs index 58379e4d98d8..32d0eccc789f 100644 --- a/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs +++ b/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs @@ -7,7 +7,7 @@ pub mod sql_types { /// The `user_job` SQL type /// /// (Automatically generated by Diesel.) - #[derive(diesel::SqlType)] + #[derive(diesel::sql_types::SqlType)] #[postgres(type_name = "user_job")] pub struct UserJob; } diff --git a/diesel_cli/tests/print_schema/schema_file_contains_custom_types/diesel.toml b/diesel_cli/tests/print_schema/schema_file_contains_custom_types/diesel.toml deleted file mode 100644 index f57985adb185..000000000000 --- a/diesel_cli/tests/print_schema/schema_file_contains_custom_types/diesel.toml +++ /dev/null @@ -1,2 +0,0 @@ -[print_schema] -file = "src/schema.rs" diff --git a/diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/expected.rs b/diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/expected.rs deleted file mode 100644 index f810db35c785..000000000000 --- a/diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/expected.rs +++ /dev/null @@ -1,15 +0,0 @@ -// @generated automatically by Diesel CLI. - -#[derive(diesel::SqlType)] -#[postgres(type_name = "my_type")] -pub struct MyType; - -diesel::table! { - use diesel::sql_types::*; - use super::MyType; - - custom_types (id) { - id -> Int4, - custom_enum -> MyType, - } -} diff --git a/diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/schema.sql b/diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/schema.sql deleted file mode 100644 index 16de9d2dcf63..000000000000 --- a/diesel_cli/tests/print_schema/schema_file_contains_custom_types/postgres/schema.sql +++ /dev/null @@ -1,5 +0,0 @@ -CREATE TYPE my_type AS ENUM ('foo', 'bar'); -CREATE TABLE custom_types ( - id SERIAL PRIMARY KEY, - custom_enum my_type NOT NULL -); From 06558e455242b6947bb2757a18a0848b557b8411 Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Tue, 22 Jun 2021 14:58:32 +0000 Subject: [PATCH 7/8] Apply suggestions from code review --- diesel_cli/src/print_schema.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/diesel_cli/src/print_schema.rs b/diesel_cli/src/print_schema.rs index a71f0c81f676..2115cac6c037 100644 --- a/diesel_cli/src/print_schema.rs +++ b/diesel_cli/src/print_schema.rs @@ -10,7 +10,7 @@ use std::error::Error; use std::fmt::{self, Display, Formatter, Write}; use std::io::Write as IoWrite; -const SCHEMA_HEADER: &str = "// @generated automatically by Diesel CLI."; +const SCHEMA_HEADER: &str = "// @generated automatically by Diesel CLI.\n"; type Regex = RegexWrapper<::regex::Regex>; @@ -173,7 +173,6 @@ pub fn output_schema( let mut out = String::new(); writeln!(out, "{}", SCHEMA_HEADER)?; - writeln!(out)?; let backend = Backend::for_url(database_url); let custom_types = if config.generate_missing_sql_type_definitions() { From ed2fc1477db617ced7372ebd63b6cc93ae76a4ef Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Wed, 23 Jun 2021 08:51:06 +0200 Subject: [PATCH 8/8] Readd missing changlog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 890d51314bab..dbcbd3946912 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -173,6 +173,8 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/ * The `#[table_name]` attribute for derive macros can now refer to any path and is no longer limited to identifiers from the current scope. +* Interacting with a database requires a mutable connection. + ### Fixed * Many types were incorrectly considered non-aggregate when they should not