From c51f11f029553d31ff11d01c5a40ead2914115d5 Mon Sep 17 00:00:00 2001 From: Omid Rad Date: Sun, 14 May 2023 12:36:19 +0200 Subject: [PATCH 1/9] Multi-column distinct_on for Pg --- CHANGELOG.md | 2 + diesel/src/pg/query_builder/distinct_on.rs | 7 --- diesel/src/query_dsl/distinct_dsl.rs | 3 +- diesel/src/query_dsl/group_by_dsl.rs | 2 +- diesel/src/query_source/aliasing/dsl_impls.rs | 1 - diesel_tests/tests/distinct.rs | 56 +++++++++++++++++++ 6 files changed, 60 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 112c6b68f319..c2f00587d9db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ Increasing the minimal supported Rust version will always be coupled at least wi on the difference between your database and the provided `schema.rs` file * Add a `ON CONFLICT (...) DO UPDATE ... [WHERE ...]` conditional clause support for PostgreSQL. * Add support for MySQL's `ON DUPLICATE KEY DO UPDATE` syntax through the existing upsert functions. +* Add ability to define multiple columns in a single `distinct_on` for PostgreSQL, + like: `.distinct_on((column_a, column_b))`. * Support for `libsqlite3-sys` 0.26 ## [diesel_derives 2.0.2] 2023-03-13 diff --git a/diesel/src/pg/query_builder/distinct_on.rs b/diesel/src/pg/query_builder/distinct_on.rs index 06bf8ffe744a..7a53ab541e4b 100644 --- a/diesel/src/pg/query_builder/distinct_on.rs +++ b/diesel/src/pg/query_builder/distinct_on.rs @@ -7,7 +7,6 @@ use crate::query_builder::{ use crate::query_dsl::methods::DistinctOnDsl; use crate::query_dsl::order_dsl::ValidOrderingForDistinct; use crate::result::QueryResult; -use crate::sql_types::SingleValue; use crate::{Expression, QuerySource}; /// Represents `DISTINCT ON (...)` @@ -20,19 +19,16 @@ impl ValidOrderingForDistinct> for OrderClause<(T,)> {} impl ValidOrderingForDistinct> for OrderClause where T: Expression, - T::SqlType: SingleValue, { } impl ValidOrderingForDistinct> for OrderClause> where T: Expression, - T::SqlType: SingleValue, { } impl ValidOrderingForDistinct> for OrderClause> where T: Expression, - T::SqlType: SingleValue, { } @@ -40,14 +36,12 @@ impl ValidOrderingForDistinct> for OrderClause<(crate::helper_types::Desc,)> where T: Expression, - T::SqlType: SingleValue, { } impl ValidOrderingForDistinct> for OrderClause<(crate::helper_types::Asc,)> where T: Expression, - T::SqlType: SingleValue, { } @@ -68,7 +62,6 @@ impl DistinctOnDsl where F: QuerySource, Selection: SelectableExpression, - Selection::SqlType: SingleValue, Self: SelectQuery, O: ValidOrderingForDistinct>, SelectStatement, S, DistinctOnClause, W, O, LOf, G, H>: diff --git a/diesel/src/query_dsl/distinct_dsl.rs b/diesel/src/query_dsl/distinct_dsl.rs index 3b0b90a112c3..9b8bc5981be2 100644 --- a/diesel/src/query_dsl/distinct_dsl.rs +++ b/diesel/src/query_dsl/distinct_dsl.rs @@ -31,7 +31,7 @@ where { type Output = dsl::Distinct>>; - fn distinct(self) -> dsl::Distinct>> { + fn distinct(self) -> Self::Output { self.as_query().distinct() } } @@ -56,7 +56,6 @@ pub trait DistinctOnDsl { impl DistinctOnDsl for T where Selection: SelectableExpression, - Selection::SqlType: crate::sql_types::SingleValue, T: Table + AsQuery>>, SelectStatement>: DistinctOnDsl, T::DefaultSelection: Expression + ValidGrouping<()>, diff --git a/diesel/src/query_dsl/group_by_dsl.rs b/diesel/src/query_dsl/group_by_dsl.rs index 7a712b5af533..d2bada7e005c 100644 --- a/diesel/src/query_dsl/group_by_dsl.rs +++ b/diesel/src/query_dsl/group_by_dsl.rs @@ -18,7 +18,7 @@ pub trait GroupByDsl { type Output; /// See the trait documentation. - fn group_by(self, expr: Expr) -> dsl::GroupBy; + fn group_by(self, expr: Expr) -> Self::Output; } impl GroupByDsl for T diff --git a/diesel/src/query_source/aliasing/dsl_impls.rs b/diesel/src/query_source/aliasing/dsl_impls.rs index 80da92ecc9b3..1b8c4d93062b 100644 --- a/diesel/src/query_source/aliasing/dsl_impls.rs +++ b/diesel/src/query_source/aliasing/dsl_impls.rs @@ -161,7 +161,6 @@ impl DistinctOnDsl for Alias where S: AliasSource, Selection: SelectableExpression, - Selection::SqlType: crate::sql_types::SingleValue, Self: QuerySource + AsQuery>>, SelectStatement>: DistinctOnDsl, ::DefaultSelection: diff --git a/diesel_tests/tests/distinct.rs b/diesel_tests/tests/distinct.rs index 04cf585491f9..84ba2e72b08f 100644 --- a/diesel_tests/tests/distinct.rs +++ b/diesel_tests/tests/distinct.rs @@ -137,3 +137,59 @@ fn distinct_on_select_order_by_two_columns() { assert_eq!(expected_data, data); } + +#[cfg(feature = "postgres")] +#[test] +fn distinct_on_two_columns() { + use crate::schema::users::dsl::*; + + let connection = &mut connection(); + diesel::sql_query( + "INSERT INTO users (name, hair_color) VALUES ('Sean', 'black'), ('Sean', 'aqua'), ('Tess', 'bronze'), ('Tess', 'champagne')", + ).execute(connection) + .unwrap(); + + let source = users + .select((name, hair_color)) + .order((name, hair_color.desc())) + .distinct_on((id, name)); + let expected_data = vec![ + NewUser::new("Sean", Some("black")), + NewUser::new("Tess", Some("champagne")), + ]; + let data: Vec<_> = source.load(connection).unwrap(); + + assert_eq!(expected_data, data); + + let source = users + .select((name, hair_color)) + .order((name.asc(), hair_color.desc())) + .distinct_on((id, name)); + let data: Vec<_> = source.load(connection).unwrap(); + + assert_eq!(expected_data, data); + + let source = users + .select((name, hair_color)) + .order((name.desc(), hair_color.desc())) + .distinct_on((id, name)); + let expected_data = vec![ + NewUser::new("Tess", Some("champagne")), + NewUser::new("Sean", Some("black")), + ]; + let data: Vec<_> = source.load(connection).unwrap(); + + assert_eq!(expected_data, data); + + let source = users + .select((name, hair_color)) + .order((name.desc(), hair_color)) + .distinct_on((id, name)); + let expected_data = vec![ + NewUser::new("Tess", Some("bronze")), + NewUser::new("Sean", Some("aqua")), + ]; + let data: Vec<_> = source.load(connection).unwrap(); + + assert_eq!(expected_data, data); +} From b7901540268f6c2bf455864f7c7d76baaffedb7e Mon Sep 17 00:00:00 2001 From: Omid Rad Date: Mon, 15 May 2023 14:11:21 +0200 Subject: [PATCH 2/9] a bit of code style and typos --- .github/ISSUE_TEMPLATE/bug_report.md | 4 +-- CHANGELOG.md | 4 +-- diesel/src/backend.rs | 26 +++++++++---------- diesel/src/connection/mod.rs | 10 +++---- diesel/src/connection/statement_cache.rs | 2 +- diesel/src/deserialize.rs | 4 +-- diesel/src/expression/array_comparison.rs | 14 +++++----- diesel/src/expression/exists.rs | 2 +- diesel/src/expression/mod.rs | 2 +- diesel/src/lib.rs | 4 +-- diesel/src/macros/mod.rs | 6 ++--- diesel/src/migration/mod.rs | 2 +- diesel/src/mysql/connection/bind.rs | 26 +++++++++---------- diesel/src/mysql/connection/stmt/iterator.rs | 2 +- diesel/src/mysql/connection/stmt/mod.rs | 2 +- .../query_builder/query_fragment_impls.rs | 2 +- diesel/src/mysql/types/primitives.rs | 12 ++++----- diesel/src/mysql/value.rs | 14 +++++----- diesel/src/pg/backend.rs | 16 ++++++------ diesel/src/pg/connection/mod.rs | 4 +-- diesel/src/pg/query_builder/distinct_on.rs | 16 ++++-------- diesel/src/pg/query_builder/on_constraint.rs | 2 +- .../pg/query_builder/query_fragment_impls.rs | 10 +++---- diesel/src/pg/types/array.rs | 2 +- diesel/src/query_builder/from_clause.rs | 2 +- .../insert_with_default_for_sqlite.rs | 4 +-- diesel/src/query_builder/mod.rs | 2 +- .../query_builder/select_statement/boxed.rs | 2 +- .../src/query_builder/select_statement/mod.rs | 2 +- diesel/src/query_dsl/mod.rs | 10 +++---- diesel/src/query_source/mod.rs | 8 +++--- diesel/src/r2d2.rs | 10 +++---- diesel/src/row.rs | 2 +- diesel/src/serialize.rs | 6 ++--- .../src/sqlite/connection/bind_collector.rs | 2 +- diesel/src/sqlite/connection/stmt.rs | 6 ++--- diesel/src/type_impls/option.rs | 4 +-- diesel_cli/src/migrations/diff_schema.rs | 15 +++++------ .../tests/fail/alias_and_group_by.rs | 2 +- .../tests/fail/derive/aliases.rs | 2 +- ...tinct_on_requires_matching_order_clause.rs | 4 +-- ...e_cannot_access_memory_of_dropped_bind.rs} | 2 +- ...nnot_access_memory_of_dropped_bind.stderr} | 2 +- ...pdate_where_not_supported_on_sqlite.stderr | 2 +- .../tests/fail/mysql_on_conflict_tests.stderr | 6 ++--- .../fail/select_requires_valid_grouping.rs | 2 +- diesel_derives/src/diesel_public_if.rs | 2 +- diesel_derives/src/multiconnection.rs | 4 +-- .../examples/querying_multiple_types.rs | 2 +- diesel_dynamic_schema/src/table.rs | 2 +- .../migrations_macros/src/embed_migrations.rs | 4 +-- .../migrations_macros/src/lib.rs | 2 +- diesel_migrations/src/embedded_migrations.rs | 6 ++--- diesel_migrations/src/errors.rs | 2 +- diesel_tests/tests/insert_from_select.rs | 2 +- guide_drafts/migration_guide.md | 2 +- 56 files changed, 152 insertions(+), 161 deletions(-) rename diesel_compile_tests/tests/fail/{ensure_sqlite_cannot_access_memory_of_droped_bind.rs => ensure_sqlite_cannot_access_memory_of_dropped_bind.rs} (95%) rename diesel_compile_tests/tests/fail/{ensure_sqlite_cannot_access_memory_of_droped_bind.stderr => ensure_sqlite_cannot_access_memory_of_dropped_bind.stderr} (85%) diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 6b3845425c7f..f87326fbb541 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -45,13 +45,13 @@ If you want to report a bug, we added some points below which help us track down Please include as much of your codebase as needed to reproduce the error. If the relevant files are large, please consider linking to a public repository or a [Gist](https://gist.github.com/). This includes normally the following parts: * The exact code where your hit the problem -* Relevant parts your schema, so any `table!` macro calls required for the +* Relevant parts your schema, so any `table!` macro calls required for * Any other type definitions involved in the code, which produces your problem --> ## Checklist -- [ ] I have already looked over the [issue tracker](https://github.com/diesel-rs/diesel/issues) and the [disussion forum](https://github.com/diesel-rs/diesel/discussions) for similar possible closed issues. +- [ ] I have already looked over the [issue tracker](https://github.com/diesel-rs/diesel/issues) and the [discussion forum](https://github.com/diesel-rs/diesel/discussions) for similar possible closed issues. diff --git a/CHANGELOG.md b/CHANGELOG.md index c2f00587d9db..374a7fc631af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -336,7 +336,7 @@ queries or set `PIPES_AS_CONCAT` manually. ### Fixed -* Fixed a incompatibly between `diesel` and `diesel_migrations` when building both crates with cargos new `resolver = "2"` enabled. This change ensures compatibility with the upcomming 2021 rust edition. +* Fixed a incompatibly between `diesel` and `diesel_migrations` when building both crates with cargos new `resolver = "2"` enabled. This change ensures compatibility with the upcoming 2021 rust edition. ## [1.4.7] - 2021-06-08 @@ -370,7 +370,7 @@ queries or set `PIPES_AS_CONCAT` manually. ### Fixed * Update several dependencies -* Fixed a bug with printing embeded migrations +* Fixed a bug with printing embedded migrations ## [1.4.3] - 2019-10-11 diff --git a/diesel/src/backend.rs b/diesel/src/backend.rs index 628b67b16407..e71853741d7c 100644 --- a/diesel/src/backend.rs +++ b/diesel/src/backend.rs @@ -35,23 +35,23 @@ pub(crate) use self::private::{DieselReserveSpecialization, TrustedBackend}; /// connection is implemented. /// For example, the `Pg` backend does not assume that `libpq` is being used. /// Implementations of this trait can and should care about details of the wire -/// protocol used to communicated with the database. +/// protocol used to communicate with the database. /// /// Implementing support for a new backend is a complex topic and depends on the -/// details how the newly implemented backend may communicate with diesel. As of this +/// details how the newly implemented backend may communicate with diesel. As of this, /// we cannot provide concrete examples here and only present a general outline of /// the required steps. Existing backend implementations provide a good starting point -/// to see how certain things are solve for other backend implementations. +/// to see how certain things are solved for other backend implementations. /// /// Types implementing `Backend` should generally be zero sized structs. /// -/// To implement the `Backend` trait you need to: +/// To implement the `Backend` trait, you need to: /// /// * Specify how a query should be build from string parts by providing a [`QueryBuilder`] /// matching your backend /// * Specify the bind value format used by your database connection library by providing /// a [`BindCollector`](crate::query_builder::bind_collector::BindCollector) matching your backend -/// * Specify how values are receive from the database by providing a corresponding raw value +/// * Specify how values are received from the database by providing a corresponding raw value /// definition /// * Control sql dialect specific parts of diesels query dsl implementation by providing a /// matching [`SqlDialect`] implementation @@ -127,7 +127,7 @@ pub type BindCollector<'a, DB> = ::BindCollector<'a>; /// /// Accessing anything from this trait is considered to be part of the /// public API. Implementing this trait is not considered to be part of -/// diesels public API, as future versions of diesel may add additional +/// diesel's public API, as future versions of diesel may add additional /// associated constants here. /// /// Each associated type is used to configure the behaviour @@ -141,9 +141,9 @@ pub type BindCollector<'a, DB> = ::BindCollector<'a>; doc = "See the [`sql_dialect`] module for options provided by diesel out of the box." )] pub trait SqlDialect: self::private::TrustedBackend { - /// Configures how this backends supports `RETURNING` clauses + /// Configures how this backend supports `RETURNING` clauses /// - /// This allows backends to opt in `RETURNING` clause support and to + /// This allows backends to opt in `RETURNING` clause support and to /// provide a custom [`QueryFragment`](crate::query_builder::QueryFragment) #[cfg_attr( feature = "i-implement-a-third-party-backend-and-opt-into-breaking-changes", @@ -365,7 +365,7 @@ pub(crate) mod sql_dialect { pub mod returning_clause { /// A marker trait indicating if a `RETURNING` clause is supported or not /// - /// If you use custom type to specify specialized support for `RETURNING` clauses + /// If you use a custom type to specify specialized support for `RETURNING` clauses /// implementing this trait opts in supporting `RETURNING` clause syntax pub trait SupportsReturningClause {} @@ -403,7 +403,7 @@ pub(crate) mod sql_dialect { pub struct IsoSqlDefaultKeyword; /// Indicates that a backend does not support `DEFAULT` value - /// expressions0for `INSERT INTO` statements + /// expressions for `INSERT INTO` statements #[derive(Debug, Copy, Clone)] pub struct DoesNotSupportDefaultKeyword; @@ -423,7 +423,7 @@ pub(crate) mod sql_dialect { /// Indicates that this backend does not support batch /// insert statements. /// In this case diesel will emulate batch insert support - /// by inserting each row on it's own + /// by inserting each row on its own #[derive(Debug, Copy, Clone)] pub struct DoesNotSupportBatchInsert; @@ -529,7 +529,7 @@ pub(crate) mod private { /// suggests adding this a bound to a type implementing `Backend` /// consider adding the following bound instead /// `YourQueryType: QueryFragment` (the concrete bound - /// is likely mentioned by rustc as part of a `note: …` + /// is likely mentioned by rustc as part of a `note: …`) /// /// For any user implementing a custom backend: You likely want to implement /// this trait for your custom backend type to opt in the existing [`QueryFragment`] impls in diesel. @@ -544,7 +544,7 @@ pub(crate) mod private { )] pub trait DieselReserveSpecialization {} - /// This trait just indicates that noone implements + /// This trait just indicates that none implements /// [`SqlDialect`](super::SqlDialect) without enabling the /// `i-implement-a-third-party-backend-and-opt-into-breaking-changes` /// feature flag. diff --git a/diesel/src/connection/mod.rs b/diesel/src/connection/mod.rs index 8996941bdc30..281b8cd3c6c6 100644 --- a/diesel/src/connection/mod.rs +++ b/diesel/src/connection/mod.rs @@ -96,7 +96,7 @@ pub type LoadRowIter<'conn, 'query, C, DB, B = DefaultLoadingMode> = /// /// Implementing a new connection based on an existing backend can enable the usage of /// other methods to connect to the database. One example here would be to replace -/// the offical diesel provided connection implementations with an implementation +/// the official diesel provided connection implementations with an implementation /// based on a pure rust connection crate. /// /// **It's important to use prepared statements to implement the following methods:** @@ -131,7 +131,7 @@ pub type LoadRowIter<'conn, 'query, C, DB, B = DefaultLoadingMode> = /// * A field type that describes a database field value. /// This type needs to implement [`Field`](crate::row::Field) /// * A connection type that wraps the connection + -/// the nessesary state managment. +/// the necessary state management. /// * Maybe a [`TransactionManager`] implementation matching /// the interface provided by the database connection crate. /// Otherwise the implementation used by the corresponding @@ -163,7 +163,7 @@ pub type LoadRowIter<'conn, 'query, C, DB, B = DefaultLoadingMode> = /// As implementations differ significantly between the supported backends /// we cannot give a one for all description here. Generally it's likely a /// good idea to follow the implementation of the corresponding connection -/// in diesel at a heigh level to gain some idea how to implement your +/// in diesel at a high level to gain some idea how to implement your /// custom implementation. /// /// ## Implement support for an unsupported database system @@ -309,7 +309,7 @@ where }; Self::TransactionManager::begin_transaction(self)?; // set the test transaction flag - // to pervent that this connection gets droped in connection pools + // to prevent that this connection gets dropped in connection pools // Tests commonly set the poolsize to 1 and use `begin_test_transaction` // to prevent modifications to the schema Self::TransactionManager::transaction_manager_status_mut(self).set_test_transaction_flag(); @@ -386,7 +386,7 @@ where /// The specific part of a [`Connection`] which actually loads data from the database /// /// This is a separate trait to allow connection implementations to specify -/// different loading modes via the generic paramater. +/// different loading modes via the generic parameter. pub trait LoadConnection: Connection { /// The cursor type returned by [`LoadConnection::load`] /// diff --git a/diesel/src/connection/statement_cache.rs b/diesel/src/connection/statement_cache.rs index 6ca0c8c1bc33..4487bd6b086a 100644 --- a/diesel/src/connection/statement_cache.rs +++ b/diesel/src/connection/statement_cache.rs @@ -117,7 +117,7 @@ pub struct StatementCache { /// /// This information can be used by the connection implementation /// to signal this fact to the database while actually -/// perparing the statement +/// preparing the statement #[derive(Debug, Clone, Copy)] #[cfg_attr( doc_cfg, diff --git a/diesel/src/deserialize.rs b/diesel/src/deserialize.rs index 6240fa566106..0838bb5ddf02 100644 --- a/diesel/src/deserialize.rs +++ b/diesel/src/deserialize.rs @@ -488,7 +488,7 @@ where } } -/// A helper trait to deserialize a statically sized row into an tuple +/// A helper trait to deserialize a statically sized row into a tuple /// /// **If you see an error message mentioning this trait you are likely trying to /// map the result of a query to a struct with mismatching field types. Recheck @@ -526,7 +526,7 @@ where T::Row: FromStaticSqlRow, { // This inline(always) attribute is here as benchmarks have shown - // a up to 5% reduction in instruction count of having it here + // up to 5% reduction in instruction count of having it here // // A plain inline attribute does not show similar improvements #[inline(always)] diff --git a/diesel/src/expression/array_comparison.rs b/diesel/src/expression/array_comparison.rs index 19a40bacfc35..3c5002fc7386 100644 --- a/diesel/src/expression/array_comparison.rs +++ b/diesel/src/expression/array_comparison.rs @@ -1,5 +1,5 @@ //! This module contains the query dsl node definitions -//! for array comparision operations like `IN` and `NOT IN` +//! for array comparison operations like `IN` and `NOT IN` use crate::backend::sql_dialect; use crate::backend::Backend; @@ -25,7 +25,7 @@ use std::marker::PhantomData; /// Third party backend can customize the [`QueryFragment`] /// implementation of this query dsl node via /// [`SqlDialect::ArrayComparison`]. A customized implementation -/// is expected to provide the same sematics as a ANSI SQL +/// is expected to provide the same semantics as an ANSI SQL /// `IN` expression. /// /// The postgres backend provided a specialized implementation @@ -45,7 +45,7 @@ pub struct In { /// Third party backend can customize the [`QueryFragment`] /// implementation of this query dsl node via /// [`SqlDialect::ArrayComparison`]. A customized implementation -/// is expected to provide the same sematics as a ANSI SQL +/// is expected to provide the same semantics as an ANSI SQL /// `NOT IN` expression.0 /// /// The postgres backend provided a specialized implementation @@ -154,13 +154,13 @@ impl_selectable_expression!(NotIn); /// This trait describes how a type is transformed to the /// `IN (values)` value expression /// -/// Diesel provided several implemenations here: +/// Diesel provided several implementations here: /// /// - An implementation for any [`Iterator`] over values /// that implement [`AsExpression`] for the corresponding /// sql type ST. The corresponding values clause will contain /// bind statements for each individual value. -/// - An implementation for select statements, that return +/// - An implementation for select statements, that returns /// a single field. The corresponding values clause will contain /// the sub query. /// @@ -196,7 +196,7 @@ where } /// A helper trait to check if the values clause of -/// a [`In`] or [`NotIn`] query dsl node is empty or not +/// an [`In`] or [`NotIn`] query dsl node is empty or not pub trait MaybeEmpty { /// Returns `true` if self represents an empty collection /// Otherwise `false` is returned. @@ -243,7 +243,7 @@ where } } -/// Query dsl node for a `IN (values)` clause containing +/// Query dsl node for an `IN (values)` clause containing /// a variable number of bind values. /// /// Third party backend can customize the [`QueryFragment`] diff --git a/diesel/src/expression/exists.rs b/diesel/src/expression/exists.rs index 6019533cf68e..82779bfbc049 100644 --- a/diesel/src/expression/exists.rs +++ b/diesel/src/expression/exists.rs @@ -43,7 +43,7 @@ pub fn exists(query: T) -> exists { /// Third party backend can customize the [`QueryFragment`] /// implementation of this query dsl node via /// [`SqlDialect::ExistsSyntax`]. A customized implementation -/// is expected to provide the same sematics as a ANSI SQL +/// is expected to provide the same semantics as an ANSI SQL /// `EXIST (subselect)` expression. #[derive(Clone, Copy, QueryId, Debug)] #[non_exhaustive] diff --git a/diesel/src/expression/mod.rs b/diesel/src/expression/mod.rs index e3ab7c71149b..3f7d17ce70f3 100644 --- a/diesel/src/expression/mod.rs +++ b/diesel/src/expression/mod.rs @@ -755,7 +755,7 @@ pub mod is_aggregate { } #[cfg(feature = "unstable")] -// this needs to be a seperate module for the reasons given in +// this needs to be a separate module for the reasons given in // https://github.com/rust-lang/rust/issues/65860 mod unstable; diff --git a/diesel/src/lib.rs b/diesel/src/lib.rs index f4c047eaef64..af6cde080a9e 100644 --- a/diesel/src/lib.rs +++ b/diesel/src/lib.rs @@ -155,7 +155,7 @@ //! //! - `sqlite`: This feature enables the diesel sqlite backend. Enabling this feature requires per default //! a compatible copy of `libsqlite3` for your target architecture. Alternatively, you can add `libsqlite3-sys` -//! with the `bundled` feature as a dependecy to your crate so SQLite will be bundled: +//! with the `bundled` feature as a dependency to your crate so SQLite will be bundled: //! ```toml //! [dependencies] //! libsqlite3-sys = { version = "0.25.2", features = ["bundled"] } @@ -204,7 +204,7 @@ //! - `numeric`: This feature flag enables support for (de)serializing numeric values from the database using types //! provided by `bigdecimal` //! - `r2d2`: This feature flag enables support for the `r2d2` connection pool implementation. -//! - `extras`: This feature enables the feature flaged support for any third party crate. This implies the +//! - `extras`: This feature enables the feature flagged support for any third party crate. This implies the //! following feature flags: `serde_json`, `chrono`, `uuid`, `network-address`, `numeric`, `r2d2` //! - `with-deprecated`: This feature enables items marked as `#[deprecated]`. It is enabled by default. //! disabling this feature explicitly opts out diesels stability guarantee. diff --git a/diesel/src/macros/mod.rs b/diesel/src/macros/mod.rs index cd51b033ad46..c1c3ad274c43 100644 --- a/diesel/src/macros/mod.rs +++ b/diesel/src/macros/mod.rs @@ -68,7 +68,7 @@ pub use diesel_derives::table_proc as table; /// /// * `child_table` is the Table with the Foreign key. /// -/// So given the Table decaration from [Associations docs](crate::associations) +/// So given the Table declaration from [Associations docs](crate::associations) /// /// * The parent table would be `User` /// * The child table would be `Post` @@ -249,7 +249,7 @@ macro_rules! __diesel_internal_backend_specific_allow_tables_to_appear_in_same_q #[macro_export] #[cfg(not(feature = "postgres_backend"))] macro_rules! __diesel_internal_backend_specific_allow_tables_to_appear_in_same_query { - ($left:ident, $rigth:ident) => {}; + ($left:ident, $right:ident) => {}; } #[doc(hidden)] @@ -633,7 +633,7 @@ mod tests { } } - // allow using table::collumn + // allow using table::column allow_columns_to_appear_in_same_group_by_clause!(a::b, b::a, a::d,); // allow using full paths diff --git a/diesel/src/migration/mod.rs b/diesel/src/migration/mod.rs index 68eeffb6d8ba..d40632b4e3ff 100644 --- a/diesel/src/migration/mod.rs +++ b/diesel/src/migration/mod.rs @@ -136,7 +136,7 @@ pub trait MigrationMetadata { /// to receive a number of migrations from. pub trait MigrationSource { /// Get a list of migrations associated with this - /// migration soucre. + /// migration source. fn migrations(&self) -> Result>>>; } diff --git a/diesel/src/mysql/connection/bind.rs b/diesel/src/mysql/connection/bind.rs index 1cd5a37758b3..d2153b17e789 100644 --- a/diesel/src/mysql/connection/bind.rs +++ b/diesel/src/mysql/connection/bind.rs @@ -147,7 +147,7 @@ bitflags::bitflags! { impl From for Flags { fn from(flags: u32) -> Self { Flags::from_bits(flags).expect( - "We encountered a unknown type flag while parsing \ + "We encountered an unknown type flag while parsing \ Mysql's type information. If you see this error message \ please open an issue at diesels github page.", ) @@ -174,9 +174,9 @@ impl Clone for BindData { let slice = unsafe { // We know that this points to a slice and the pointer is not null at this // location - // The length pointer is valid as long as noone missuses `bind_for_truncated_data` - // as this is the only location that updates the length vield before the corresponding data are - // written. At the time of writting this comment the `BindData::bind_for_truncated_data` + // The length pointer is valid as long as none missuses `bind_for_truncated_data` + // as this is the only location that updates the length field before the corresponding data are + // written. At the time of writing this comment, the `BindData::bind_for_truncated_data` // function is only called by `Binds::populate_dynamic_buffers` which ensures the corresponding // invariant. std::slice::from_raw_parts(ptr.as_ptr(), self.length as usize) @@ -411,9 +411,9 @@ impl BindData { let slice = unsafe { // We know that this points to a slice and the pointer is not null at this // location - // The length pointer is valid as long as noone missuses `bind_for_truncated_data` - // as this is the only location that updates the length vield before the corresponding data are - // written. At the time of writting this comment the `BindData::bind_for_truncated_data` + // The length pointer is valid as long as none missuses `bind_for_truncated_data` + // as this is the only location that updates the length field before the corresponding data are + // written. At the time of writing this comment, the `BindData::bind_for_truncated_data` // function is only called by `Binds::populate_dynamic_buffers` which ensures the corresponding // invariant. std::slice::from_raw_parts(data.as_ptr(), self.length as usize) @@ -433,9 +433,9 @@ impl BindData { self.length = actual_bytes_in_buffer as libc::c_ulong; } - // This function is marked as unsafe as it returns a owned value + // This function is marked as unsafe as it returns an owned value // containing a pointer with a lifetime coupled to self. - // Callers need to ensure that the returend value cannot outlive `self` + // Callers need to ensure that the returned value cannot outlive `self` unsafe fn mysql_bind(&mut self) -> ffi::MYSQL_BIND { use std::ptr::addr_of_mut; @@ -679,7 +679,7 @@ impl From<(ffi::enum_field_types, Flags)> for MysqlType { enum_field_types::MYSQL_TYPE_YEAR => unreachable!( "The year type should have set the unsigned flag. If you ever \ see this error message, something has gone very wrong. Please \ - open an issue at the diesel githup repo in this case" + open an issue at the diesel github repo in this case" ), // Null value enum_field_types::MYSQL_TYPE_NULL => unreachable!( @@ -706,9 +706,9 @@ impl From<(ffi::enum_field_types, Flags)> for MysqlType { | enum_field_types::MYSQL_TYPE_TIME2 | enum_field_types::MYSQL_TYPE_DATETIME2 | enum_field_types::MYSQL_TYPE_TIMESTAMP2 => unreachable!( - "The mysql documentation states that this types are \ - only used on server side, so if you see this error \ - something has gone wrong. Please open a issue at \ + "The mysql documentation states that these types are \ + only used on the server side, so if you see this error \ + something has gone wrong. Please open an issue at \ the diesel github repo." ), } diff --git a/diesel/src/mysql/connection/stmt/iterator.rs b/diesel/src/mysql/connection/stmt/iterator.rs index d64cdb38d617..dad029c97223 100644 --- a/diesel/src/mysql/connection/stmt/iterator.rs +++ b/diesel/src/mysql/connection/stmt/iterator.rs @@ -63,7 +63,7 @@ impl<'a> Iterator for StatementIterator<'a> { } else { // The shared bind buffer is in use by someone else, // this means we copy out the values and replace the used reference - // by the copied values. After this we can advance the statment + // by the copied values. After this we can advance the statement // another step let mut last_row = { let mut last_row = match self.last_row.try_borrow_mut() { diff --git a/diesel/src/mysql/connection/stmt/mod.rs b/diesel/src/mysql/connection/stmt/mod.rs index 6bc5be540fe0..3e22feeebfda 100644 --- a/diesel/src/mysql/connection/stmt/mod.rs +++ b/diesel/src/mysql/connection/stmt/mod.rs @@ -160,7 +160,7 @@ impl<'a> StatementUse<'a> { } /// This function should be called after `execute` only - /// otherwise it's not guranteed to return a valid result + /// otherwise it's not guaranteed to return a valid result pub(in crate::mysql::connection) unsafe fn result_size(&mut self) -> QueryResult { let size = ffi::mysql_stmt_num_rows(self.inner.stmt.as_ptr()); usize::try_from(size).map_err(|e| Error::DeserializationError(Box::new(e))) diff --git a/diesel/src/mysql/query_builder/query_fragment_impls.rs b/diesel/src/mysql/query_builder/query_fragment_impls.rs index ac72033ea4bd..0b4276aa8213 100644 --- a/diesel/src/mysql/query_builder/query_fragment_impls.rs +++ b/diesel/src/mysql/query_builder/query_fragment_impls.rs @@ -135,7 +135,7 @@ where } /// A marker type signaling that the given `ON CONFLICT` clause -/// uses mysql's `ON DUPLICATE KEY` syntaxt that triggers on +/// uses mysql's `ON DUPLICATE KEY` syntax that triggers on /// all unique constraints /// /// See [`InsertStatement::on_conflict`](crate::query_builder::InsertStatement::on_conflict) diff --git a/diesel/src/mysql/types/primitives.rs b/diesel/src/mysql/types/primitives.rs index 8377eece079b..765688f6efd8 100644 --- a/diesel/src/mysql/types/primitives.rs +++ b/diesel/src/mysql/types/primitives.rs @@ -60,17 +60,17 @@ impl FromSql for i16 { }), NumericRepresentation::Big(x) => x.try_into().map_err(|_| { Box::new(DeserializationError( - "Numeric overflow/underflow occured".into(), + "Numeric overflow/underflow occurred".into(), )) as _ }), NumericRepresentation::Float(x) => f32_to_i64(x)?.try_into().map_err(|_| { Box::new(DeserializationError( - "Numeric overflow/underflow occured".into(), + "Numeric overflow/underflow occurred".into(), )) as _ }), NumericRepresentation::Double(x) => f64_to_i64(x)?.try_into().map_err(|_| { Box::new(DeserializationError( - "Numeric overflow/underflow occured".into(), + "Numeric overflow/underflow occurred".into(), )) as _ }), NumericRepresentation::Decimal(bytes) => decimal_to_integer(bytes), @@ -87,20 +87,20 @@ impl FromSql for i32 { NumericRepresentation::Medium(x) => Ok(x), NumericRepresentation::Big(x) => x.try_into().map_err(|_| { Box::new(DeserializationError( - "Numeric overflow/underflow occured".into(), + "Numeric overflow/underflow occurred".into(), )) as _ }), NumericRepresentation::Float(x) => f32_to_i64(x).and_then(|i| { i.try_into().map_err(|_| { Box::new(DeserializationError( - "Numeric overflow/underflow occured".into(), + "Numeric overflow/underflow occurred".into(), )) as _ }) }), NumericRepresentation::Double(x) => f64_to_i64(x).and_then(|i| { i.try_into().map_err(|_| { Box::new(DeserializationError( - "Numeric overflow/underflow occured".into(), + "Numeric overflow/underflow occurred".into(), )) as _ }) }), diff --git a/diesel/src/mysql/value.rs b/diesel/src/mysql/value.rs index 2769f28282cb..98550bf8bae7 100644 --- a/diesel/src/mysql/value.rs +++ b/diesel/src/mysql/value.rs @@ -96,18 +96,18 @@ impl<'a> MysqlValue<'a> { #[derive(Debug, Clone, Copy)] #[non_exhaustive] pub enum NumericRepresentation<'a> { - /// Correponds to `MYSQL_TYPE_TINY` + /// Corresponds to `MYSQL_TYPE_TINY` Tiny(i8), - /// Correponds to `MYSQL_TYPE_SHORT` + /// Corresponds to `MYSQL_TYPE_SHORT` Small(i16), - /// Correponds to `MYSQL_TYPE_INT24` and `MYSQL_TYPE_LONG` + /// Corresponds to `MYSQL_TYPE_INT24` and `MYSQL_TYPE_LONG` Medium(i32), - /// Correponds to `MYSQL_TYPE_LONGLONG` + /// Corresponds to `MYSQL_TYPE_LONGLONG` Big(i64), - /// Correponds to `MYSQL_TYPE_FLOAT` + /// Corresponds to `MYSQL_TYPE_FLOAT` Float(f32), - /// Correponds to `MYSQL_TYPE_DOUBLE` + /// Corresponds to `MYSQL_TYPE_DOUBLE` Double(f64), - /// Correponds to `MYSQL_TYPE_DECIMAL` and `MYSQL_TYPE_NEWDECIMAL` + /// Corresponds to `MYSQL_TYPE_DECIMAL` and `MYSQL_TYPE_NEWDECIMAL` Decimal(&'a [u8]), } diff --git a/diesel/src/pg/backend.rs b/diesel/src/pg/backend.rs index 63e02b997ace..6791b03eef70 100644 --- a/diesel/src/pg/backend.rs +++ b/diesel/src/pg/backend.rs @@ -85,7 +85,7 @@ impl PgTypeMetadata { })) } - /// Create a new instance of this type based on dynamically lookup informations + /// Create a new instance of this type based on dynamically lookup information /// /// This function is useful for third party crates that may implement a custom /// postgres connection type and want to bring their own lookup mechanism. @@ -126,7 +126,7 @@ impl TypeMetadata for Pg { impl SqlDialect for Pg { type ReturningClause = sql_dialect::returning_clause::PgLikeReturningClause; - type OnConflictClause = PgOnConflictClaues; + type OnConflictClause = PgOnConflictClause; type InsertWithDefaultKeyword = sql_dialect::default_keyword_for_insert::IsoSqlDefaultKeyword; type BatchInsertSupport = sql_dialect::batch_insert_support::PostgresLikeBatchInsertSupport; @@ -138,18 +138,18 @@ impl SqlDialect for Pg { type SelectStatementSyntax = sql_dialect::select_statement_syntax::AnsiSqlSelectStatement; type ExistsSyntax = sql_dialect::exists_syntax::AnsiSqlExistsSyntax; - type ArrayComparison = PgStyleArrayComparision; + type ArrayComparison = PgStyleArrayComparison; } impl DieselReserveSpecialization for Pg {} impl TrustedBackend for Pg {} #[derive(Debug, Copy, Clone)] -pub struct PgOnConflictClaues; +pub struct PgOnConflictClause; -impl sql_dialect::on_conflict_clause::SupportsOnConflictClause for PgOnConflictClaues {} -impl sql_dialect::on_conflict_clause::SupportsOnConflictClauseWhere for PgOnConflictClaues {} -impl sql_dialect::on_conflict_clause::PgLikeOnConflictClause for PgOnConflictClaues {} +impl sql_dialect::on_conflict_clause::SupportsOnConflictClause for PgOnConflictClause {} +impl sql_dialect::on_conflict_clause::SupportsOnConflictClauseWhere for PgOnConflictClause {} +impl sql_dialect::on_conflict_clause::PgLikeOnConflictClause for PgOnConflictClause {} #[derive(Debug, Copy, Clone)] -pub struct PgStyleArrayComparision; +pub struct PgStyleArrayComparison; diff --git a/diesel/src/pg/connection/mod.rs b/diesel/src/pg/connection/mod.rs index 3e5533080d18..5e0e766f0d34 100644 --- a/diesel/src/pg/connection/mod.rs +++ b/diesel/src/pg/connection/mod.rs @@ -78,7 +78,7 @@ use std::os::raw as libc; /// ## `PgRowByRowLoadingMode` /// /// By using this mode `PgConnection` defaults to loading each row of the result set -/// sepreatly. This might be desired for huge result sets. +/// separately. This might be desired for huge result sets. /// /// This loading mode **prevents** creating more than one iterator at once using /// the same connection. The following code is **not** allowed: @@ -119,7 +119,7 @@ pub struct PgConnection { connection_and_transaction_manager: ConnectionAndTransactionManager, } -// according to libpq documentation a connection can be transfered to other threads +// according to libpq documentation a connection can be transferred to other threads #[allow(unsafe_code)] unsafe impl Send for PgConnection {} diff --git a/diesel/src/pg/query_builder/distinct_on.rs b/diesel/src/pg/query_builder/distinct_on.rs index 7a53ab541e4b..a8e8bf6af33d 100644 --- a/diesel/src/pg/query_builder/distinct_on.rs +++ b/diesel/src/pg/query_builder/distinct_on.rs @@ -16,19 +16,13 @@ pub struct DistinctOnClause(pub(crate) T); impl ValidOrderingForDistinct> for NoOrderClause {} impl ValidOrderingForDistinct> for OrderClause<(T,)> {} -impl ValidOrderingForDistinct> for OrderClause -where - T: Expression, -{ -} -impl ValidOrderingForDistinct> for OrderClause> -where - T: Expression, +impl ValidOrderingForDistinct> for OrderClause where T: Expression {} +impl ValidOrderingForDistinct> for OrderClause> where + T: Expression { } -impl ValidOrderingForDistinct> for OrderClause> -where - T: Expression, +impl ValidOrderingForDistinct> for OrderClause> where + T: Expression { } diff --git a/diesel/src/pg/query_builder/on_constraint.rs b/diesel/src/pg/query_builder/on_constraint.rs index e23fbe51ec7d..db4da30288ad 100644 --- a/diesel/src/pg/query_builder/on_constraint.rs +++ b/diesel/src/pg/query_builder/on_constraint.rs @@ -59,7 +59,7 @@ impl<'a> QueryId for OnConstraint<'a> { const HAS_STATIC_QUERY_ID: bool = false; } -impl<'a> QueryFragment +impl<'a> QueryFragment for ConflictTarget> { fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, Pg>) -> QueryResult<()> { diff --git a/diesel/src/pg/query_builder/query_fragment_impls.rs b/diesel/src/pg/query_builder/query_fragment_impls.rs index 6696d5d60f55..e8bc3cd74db9 100644 --- a/diesel/src/pg/query_builder/query_fragment_impls.rs +++ b/diesel/src/pg/query_builder/query_fragment_impls.rs @@ -1,5 +1,5 @@ use crate::expression::array_comparison::{In, Many, MaybeEmpty, NotIn}; -use crate::pg::backend::PgStyleArrayComparision; +use crate::pg::backend::PgStyleArrayComparison; use crate::pg::types::sql_types::Array; use crate::pg::Pg; use crate::query_builder::locking_clause::{ @@ -60,7 +60,7 @@ impl QueryFragment for NoWait { } } -impl QueryFragment for In +impl QueryFragment for In where T: QueryFragment, U: QueryFragment + MaybeEmpty, @@ -74,7 +74,7 @@ where } } -impl QueryFragment for NotIn +impl QueryFragment for NotIn where T: QueryFragment, U: QueryFragment + MaybeEmpty, @@ -88,7 +88,7 @@ where } } -impl QueryFragment for Many +impl QueryFragment for Many where ST: SingleValue, Vec: ToSql, Pg>, @@ -99,7 +99,7 @@ where } } -impl QueryFragment +impl QueryFragment for DecoratedConflictTarget where T: QueryFragment, diff --git a/diesel/src/pg/types/array.rs b/diesel/src/pg/types/array.rs index 63a210ca1dc3..8afcf878151f 100644 --- a/diesel/src/pg/types/array.rs +++ b/diesel/src/pg/types/array.rs @@ -64,7 +64,7 @@ use crate::expression::AsExpression; macro_rules! array_as_expression { ($ty:ty, $sql_type:ty) => { #[cfg(feature = "postgres_backend")] - // this simplifies the macro implemntation + // this simplifies the macro implementation // as some macro calls use this lifetime #[allow(clippy::extra_unused_lifetimes)] impl<'a, 'b, ST: 'static, T> AsExpression<$sql_type> for $ty { diff --git a/diesel/src/query_builder/from_clause.rs b/diesel/src/query_builder/from_clause.rs index 5e6df1849726..45bd0f4f9291 100644 --- a/diesel/src/query_builder/from_clause.rs +++ b/diesel/src/query_builder/from_clause.rs @@ -6,7 +6,7 @@ use crate::{QueryResult, QuerySource}; /// This type represents a not existing from clause /// /// Custom backends can provide a custom [`QueryFragment`] -/// impl by specializing the implementtion via +/// impl by specializing the implementation via /// [`SqlDialect::EmptyFromClauseSyntax`](crate::backend::SqlDialect::EmptyFromClauseSyntax) #[cfg_attr( feature = "i-implement-a-third-party-backend-and-opt-into-breaking-changes", diff --git a/diesel/src/query_builder/insert_statement/insert_with_default_for_sqlite.rs b/diesel/src/query_builder/insert_statement/insert_with_default_for_sqlite.rs index 704b5d30c5cf..181ea4ed44ef 100644 --- a/diesel/src/query_builder/insert_statement/insert_with_default_for_sqlite.rs +++ b/diesel/src/query_builder/insert_statement/insert_with_default_for_sqlite.rs @@ -290,8 +290,8 @@ impl CanInsertInSingleQuery for SqliteBatchInsertWrapper>, T, QId, STATIC_QUERY_ID> where // We constrain that here on an internal helper type - // to make sure that this does not accidently leak - // so that noone does really implement normal batch + // to make sure that this does not accidentally leak + // so that none does really implement normal batch // insert for inserts with default values here SqliteCanInsertInSingleQueryHelper: CanInsertInSingleQuery, { diff --git a/diesel/src/query_builder/mod.rs b/diesel/src/query_builder/mod.rs index 256503df680c..6da06828c1bb 100644 --- a/diesel/src/query_builder/mod.rs +++ b/diesel/src/query_builder/mod.rs @@ -343,7 +343,7 @@ pub trait AsQuery { // This method is part of our public API, // so we won't change the name to just appease clippy // (Also the trait is literally named `AsQuery` so - // naming the method similary is fine) + // naming the method similarity is fine) #[allow(clippy::wrong_self_convention)] fn as_query(self) -> Self::Query; } diff --git a/diesel/src/query_builder/select_statement/boxed.rs b/diesel/src/query_builder/select_statement/boxed.rs index 05e96a5f20b6..e3dacc0396a2 100644 --- a/diesel/src/query_builder/select_statement/boxed.rs +++ b/diesel/src/query_builder/select_statement/boxed.rs @@ -133,7 +133,7 @@ impl<'a, ST, DB, GB> BoxedSelectStatement<'a, ST, NoFromClause, DB, GB> { } } -// that's a trait to control who can access these method +// that's a trait to control who can access these methods #[doc(hidden)] // exported via internal::derives::multiconnection pub trait BoxedQueryHelper<'a, QS, DB> { fn build_query<'b, 'c>( diff --git a/diesel/src/query_builder/select_statement/mod.rs b/diesel/src/query_builder/select_statement/mod.rs index 1edd620095f7..2c6441756e2b 100644 --- a/diesel/src/query_builder/select_statement/mod.rs +++ b/diesel/src/query_builder/select_statement/mod.rs @@ -88,7 +88,7 @@ pub struct SelectStatement< pub(crate) group_by: GroupBy, /// The having clause of the query pub(crate) having: Having, - /// The locking clauise of the query + /// The locking clause of the query pub(crate) locking: Locking, } diff --git a/diesel/src/query_dsl/mod.rs b/diesel/src/query_dsl/mod.rs index 73d5f6af6831..cf30d40005ae 100644 --- a/diesel/src/query_dsl/mod.rs +++ b/diesel/src/query_dsl/mod.rs @@ -386,7 +386,7 @@ pub trait QueryDsl: Sized { /// SQL is referred as ["explicit join"](https://www.postgresql.org/docs/current/explicit-joins.html) /// by the PostgreSQL documentation and may have implications on the chosen query plan /// for large numbers of joins in the same query. Checkout the documentation of the - /// [`join_collapse_limit` paramater](https://www.postgresql.org/docs/current/runtime-config-query.html#GUC-JOIN-COLLAPSE-LIMIT) + /// [`join_collapse_limit` parameter](https://www.postgresql.org/docs/current/runtime-config-query.html#GUC-JOIN-COLLAPSE-LIMIT) /// to control this behaviour. /// /// [associations]: crate::associations @@ -1191,8 +1191,8 @@ pub trait QueryDsl: Sized { /// # use crate::schema::users; /// # let connection = &mut establish_connection(); /// // Executes `SELECT * FROM users FOR UPDATE SKIP LOCKED` - /// let user_skiped_locked = users::table.for_update().skip_locked().load(connection)?; - /// # let u: Vec<(i32, String)> = user_skiped_locked; + /// let user_skipped_locked = users::table.for_update().skip_locked().load(connection)?; + /// # let u: Vec<(i32, String)> = user_skipped_locked; /// # Ok(()) /// # } /// # #[cfg(feature = "sqlite")] @@ -1557,9 +1557,9 @@ pub trait RunQueryDsl: Sized { /// a tuple of the values, or a struct which implements [`Queryable`]. /// This type is specified by the first generic type of this function. /// - /// The second generic type paramater specifies the so called loading mode, + /// The second generic type parameter specifies the so called loading mode, /// which describes how the connection implementation loads data from the database. - /// All connections should provide a implementaiton for + /// All connections should provide a implementation for /// [`DefaultLoadingMode`](crate::connection::DefaultLoadingMode). /// /// They may provide additional modes. Checkout the documentation of the concrete diff --git a/diesel/src/query_source/mod.rs b/diesel/src/query_source/mod.rs index aaf1be08060f..a404284227ba 100644 --- a/diesel/src/query_source/mod.rs +++ b/diesel/src/query_source/mod.rs @@ -33,7 +33,7 @@ pub trait QuerySource { /// The actual `FROM` clause of this type. This is typically only called in /// `QueryFragment` implementations. // from here is something different than from in rust - // as this literally refercs to SQL from. + // as this literally refers to SQL from. #[allow(clippy::wrong_self_convention)] fn from_clause(&self) -> Self::FromClause; /// The default select clause of this type, which should be used if no @@ -91,7 +91,7 @@ pub trait AppearsInFromClause { /// /// Troubleshooting /// --------------- -/// If you encounter an error mentionning this trait, it could mean that either: +/// If you encounter an error mentioning this trait, it could mean that either: /// - You are attempting to use tables that don't belong to the same database together /// (no call to [`allow_tables_to_appear_in_same_query!`] was made) /// - You are attempting to use two aliases to the same table in the same query, but they @@ -133,7 +133,7 @@ pub(crate) mod private { /// >, /// ``` /// - /// In order to aquire the counts in the first place, we must already know + /// In order to acquire the counts in the first place, we must already know /// the table we're searching for. #[doc(hidden)] // This is used as part of the `table!` implementation pub trait Pick { @@ -161,7 +161,7 @@ pub(crate) mod private { )] /// Everything in this module is here to give something more helpful than: /// -/// > (Never, Never): Pick is not satisifed +/// > (Never, Never): Pick is not satisfied /// /// Any of these impls can be deleted if they are getting in the way of /// other functionality. Any code which is using these impls is already diff --git a/diesel/src/r2d2.rs b/diesel/src/r2d2.rs index 66d3e112a831..523baf80bbe1 100644 --- a/diesel/src/r2d2.rs +++ b/diesel/src/r2d2.rs @@ -232,6 +232,10 @@ where ))) } + fn begin_test_transaction(&mut self) -> QueryResult<()> { + (**self).begin_test_transaction() + } + fn execute_returning_count(&mut self, source: &T) -> QueryResult where T: QueryFragment + QueryId, @@ -244,10 +248,6 @@ where ) -> &mut >::TransactionStateData { (**self).transaction_state() } - - fn begin_test_transaction(&mut self) -> QueryResult<()> { - (**self).begin_test_transaction() - } } impl LoadConnection for PooledConnection @@ -505,7 +505,7 @@ mod tests { assert_eq!(checkout_count.load(Ordering::Relaxed), 2); // check that we remove a connection from the pool that was - // open during panicing + // open during panicking #[allow(unreachable_code, unused_variables)] std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { let conn = pool.get(); diff --git a/diesel/src/row.rs b/diesel/src/row.rs index 20473d50991b..02d48ea07859 100644 --- a/diesel/src/row.rs +++ b/diesel/src/row.rs @@ -149,7 +149,7 @@ where // These traits are not part of the public API // because: -// * we want to control who can implment `Row` (for `RowSealed`) +// * we want to control who can implement `Row` (for `RowSealed`) // * `PartialRow` is an implementation detail // * `RowLifetimeHelper` is an internal backward compatibility helper pub(crate) mod private { diff --git a/diesel/src/serialize.rs b/diesel/src/serialize.rs index 2592a2f5775b..f0bd9426ac62 100644 --- a/diesel/src/serialize.rs +++ b/diesel/src/serialize.rs @@ -207,14 +207,14 @@ where /// } /// ``` /// -/// Using temporary values as part of the `ToSql` implemenation requires additional +/// Using temporary values as part of the `ToSql` implementation requires additional /// work. /// /// Backends using [`RawBytesBindCollector`] as [`BindCollector`] copy the serialized values as part /// of `Write` implementation. This includes the `Mysql` and the `Pg` backend provided by diesel. -/// This means existing `ToSql` implemenations can be used even with +/// This means existing `ToSql` implementations can be used even with /// temporary values. For these it is required to call -/// [`Output::reborrow`] to shorten the lifetime of the `Output` type correspondenly. +/// [`Output::reborrow`] to shorten the lifetime of the `Output` type correspondingly. /// /// ``` /// # use diesel::backend::Backend; diff --git a/diesel/src/sqlite/connection/bind_collector.rs b/diesel/src/sqlite/connection/bind_collector.rs index db944681ba2d..8cce52c35ed3 100644 --- a/diesel/src/sqlite/connection/bind_collector.rs +++ b/diesel/src/sqlite/connection/bind_collector.rs @@ -16,7 +16,7 @@ impl SqliteBindCollector<'_> { } /// This type represents a value bound to -/// an sqlite prepared statement +/// a sqlite prepared statement /// /// It can be constructed via the various `From` implementations #[derive(Debug)] diff --git a/diesel/src/sqlite/connection/stmt.rs b/diesel/src/sqlite/connection/stmt.rs index 9aeebe5dc6c3..ffd8ed0a8def 100644 --- a/diesel/src/sqlite/connection/stmt.rs +++ b/diesel/src/sqlite/connection/stmt.rs @@ -213,7 +213,7 @@ impl Drop for Statement { } // A warning for future editors: -// Changing this code to something "simplier" may +// Changing this code to something "simpler" may // introduce undefined behaviour. Make sure you read // the following discussions for details about // the current version: @@ -223,7 +223,7 @@ impl Drop for Statement { // * https://github.com/rust-lang/unsafe-code-guidelines/issues/194 struct BoundStatement<'stmt, 'query> { statement: MaybeCached<'stmt, Statement>, - // we need to store the query here to ensure noone does + // we need to store the query here to ensure none does // drop it till the end ot the statement // We use a boxed queryfragment here just to erase the // generic type, we use NonNull to communicate @@ -306,7 +306,7 @@ impl<'stmt, 'query> BoundStatement<'stmt, 'query> { let res = unsafe { self.statement.bind(tpe, bind, bind_idx) }?; // it's important to push these only after - // the call to bind succeded, otherwise we might attempt to + // the call to bind succeeded, otherwise we might attempt to // call bind to an non-existing bind position in // the destructor if let Some(ptr) = res { diff --git a/diesel/src/type_impls/option.rs b/diesel/src/type_impls/option.rs index 2e97bcbcf2a6..dd87fb4a7bd0 100644 --- a/diesel/src/type_impls/option.rs +++ b/diesel/src/type_impls/option.rs @@ -151,7 +151,7 @@ fn option_to_sql() { let mut bytes = Output::test(ByteWrapper(&mut buffer)); ToSql::::to_sql(&Some("Sean"), &mut bytes).unwrap() }; - let expectd_bytes = b"Sean".to_vec(); + let expected_bytes = b"Sean".to_vec(); assert_eq!(IsNull::No, is_null); - assert_eq!(buffer, expectd_bytes); + assert_eq!(buffer, expected_bytes); } diff --git a/diesel_cli/src/migrations/diff_schema.rs b/diesel_cli/src/migrations/diff_schema.rs index 2e4fb8405fb7..b377890b1a8f 100644 --- a/diesel_cli/src/migrations/diff_schema.rs +++ b/diesel_cli/src/migrations/diff_schema.rs @@ -300,16 +300,16 @@ impl SchemaDiff { generate_drop_table(query_builder, &table.sql_name.to_lowercase())?; } SchemaDiff::CreateTable { - to_create: to_ceate, + to_create, foreign_keys, } => { - let table = &to_ceate.sql_name.to_lowercase(); - let primary_keys = to_ceate + let table = &to_create.sql_name.to_lowercase(); + let primary_keys = to_create .primary_keys .as_ref() .map(|keys| keys.keys.iter().map(|k| k.to_string()).collect()) .unwrap_or_else(|| vec![String::from("id")]); - let column_data = to_ceate + let column_data = to_create .column_defs .iter() .map(|c| { @@ -399,11 +399,8 @@ impl SchemaDiff { &fk, )?; } - SchemaDiff::CreateTable { - to_create: to_ceate, - .. - } => { - generate_drop_table(query_builder, &to_ceate.sql_name.to_lowercase())?; + SchemaDiff::CreateTable { to_create, .. } => { + generate_drop_table(query_builder, &to_create.sql_name.to_lowercase())?; } SchemaDiff::ChangeTable { table, diff --git a/diesel_compile_tests/tests/fail/alias_and_group_by.rs b/diesel_compile_tests/tests/fail/alias_and_group_by.rs index 34b19eb562b1..79c2a43a6ab7 100644 --- a/diesel_compile_tests/tests/fail/alias_and_group_by.rs +++ b/diesel_compile_tests/tests/fail/alias_and_group_by.rs @@ -14,7 +14,7 @@ fn main() { let conn = &mut PgConnection::establish("…").unwrap(); let user_alias = alias!(users as user1); - // allowed as this groups by the the same field + // allowed as this groups by the same field user_alias .group_by(user_alias.field(users::name)) .select(user_alias.field(users::name)) diff --git a/diesel_compile_tests/tests/fail/derive/aliases.rs b/diesel_compile_tests/tests/fail/derive/aliases.rs index 09b224b392f5..0946a894dbea 100644 --- a/diesel_compile_tests/tests/fail/derive/aliases.rs +++ b/diesel_compile_tests/tests/fail/derive/aliases.rs @@ -49,7 +49,7 @@ pub fn check(conn: &mut PgConnection) { let user2_alias = alias!(users as user3); - // dont't allow joins to not joinable tables + // don't allow joins to not joinable tables pets::table .inner_join(user_alias) .select(pets::id) diff --git a/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs b/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs index 84133d4577e2..828a7228021e 100644 --- a/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs +++ b/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs @@ -64,7 +64,7 @@ fn main() { .then_order_by(users::name) .distinct_on(users::name); - // it's not possible to set a invalid order clause after we set + // it's not possible to set an invalid order clause after we set // the distinct on clause let _ = users::table.distinct_on(users::name).order_by(users::id); @@ -83,7 +83,7 @@ fn main() { .distinct_on(users::name) .into_boxed(); - // it's not possible to set a invalid order clause after we set + // it's not possible to set an invalid order clause after we set // the distinct on clause let _ = users::table .distinct_on(users::name) diff --git a/diesel_compile_tests/tests/fail/ensure_sqlite_cannot_access_memory_of_droped_bind.rs b/diesel_compile_tests/tests/fail/ensure_sqlite_cannot_access_memory_of_dropped_bind.rs similarity index 95% rename from diesel_compile_tests/tests/fail/ensure_sqlite_cannot_access_memory_of_droped_bind.rs rename to diesel_compile_tests/tests/fail/ensure_sqlite_cannot_access_memory_of_dropped_bind.rs index ecf6f1619d06..367a643fb009 100644 --- a/diesel_compile_tests/tests/fail/ensure_sqlite_cannot_access_memory_of_droped_bind.rs +++ b/diesel_compile_tests/tests/fail/ensure_sqlite_cannot_access_memory_of_dropped_bind.rs @@ -12,7 +12,7 @@ fn main() { let mut iter = LoadConnection::load(&mut connection, query).unwrap(); - // Sqlite borrows the buffer internally, so droping it here is not allowed + // Sqlite borrows the buffer internally, so dropping it here is not allowed // while the statement is still alive. std::mem::drop(buf); diff --git a/diesel_compile_tests/tests/fail/ensure_sqlite_cannot_access_memory_of_droped_bind.stderr b/diesel_compile_tests/tests/fail/ensure_sqlite_cannot_access_memory_of_dropped_bind.stderr similarity index 85% rename from diesel_compile_tests/tests/fail/ensure_sqlite_cannot_access_memory_of_droped_bind.stderr rename to diesel_compile_tests/tests/fail/ensure_sqlite_cannot_access_memory_of_dropped_bind.stderr index eb2c10af7eeb..adbc6ea83b66 100644 --- a/diesel_compile_tests/tests/fail/ensure_sqlite_cannot_access_memory_of_droped_bind.stderr +++ b/diesel_compile_tests/tests/fail/ensure_sqlite_cannot_access_memory_of_dropped_bind.stderr @@ -1,5 +1,5 @@ error[E0505]: cannot move out of `buf` because it is borrowed - --> tests/fail/ensure_sqlite_cannot_access_memory_of_droped_bind.rs:17:24 + --> tests/fail/ensure_sqlite_cannot_access_memory_of_dropped_bind.rs:17:24 | 11 | let query = diesel::select((&buf as &[u8]).into_sql::()); | ---- borrow of `buf` occurs here diff --git a/diesel_compile_tests/tests/fail/insert_on_conflict_do_update_where_not_supported_on_sqlite.stderr b/diesel_compile_tests/tests/fail/insert_on_conflict_do_update_where_not_supported_on_sqlite.stderr index f08eddda8cfd..4d00841ab0ff 100644 --- a/diesel_compile_tests/tests/fail/insert_on_conflict_do_update_where_not_supported_on_sqlite.stderr +++ b/diesel_compile_tests/tests/fail/insert_on_conflict_do_update_where_not_supported_on_sqlite.stderr @@ -6,7 +6,7 @@ error[E0277]: the trait bound `sqlite::backend::SqliteOnConflictClause: Supports | | | required by a bound introduced by this call | - = help: the trait `SupportsOnConflictClauseWhere` is implemented for `pg::backend::PgOnConflictClaues` + = help: the trait `SupportsOnConflictClauseWhere` is implemented for `pg::backend::PgOnConflictClause` = note: required for `OnConflictValues>, table>, ..., ..., ...>` to implement `QueryFragment` = note: 1 redundant requirement hidden = note: required for `InsertStatement, ...>, ..., ..., ...>>` to implement `QueryFragment` diff --git a/diesel_compile_tests/tests/fail/mysql_on_conflict_tests.stderr b/diesel_compile_tests/tests/fail/mysql_on_conflict_tests.stderr index f27f99b38fc9..cc9d75d91b4e 100644 --- a/diesel_compile_tests/tests/fail/mysql_on_conflict_tests.stderr +++ b/diesel_compile_tests/tests/fail/mysql_on_conflict_tests.stderr @@ -146,7 +146,7 @@ error[E0277]: the trait bound `diesel::query_builder::upsert::on_conflict_target | required by a bound introduced by this call | = help: the trait `QueryFragment` is implemented for `diesel::query_builder::upsert::on_conflict_target::ConflictTarget` - = note: required for `OnConflictValues>, ...), ...>, ..., ...>` to implement `QueryFragment` + = note: required for `OnConflictValues>, ...), ...>, ..., ...>` to implement `QueryFragment` = note: 2 redundant requirements hidden = note: required for `InsertStatement, ...), ...>, ..., ...>>` to implement `QueryFragment` = note: required for `InsertStatement, ...), ...>, ..., ...>>` to implement `ExecuteDsl` @@ -198,7 +198,7 @@ error[E0277]: the trait bound `diesel::query_builder::upsert::on_conflict_target as QueryFragment<_DB, _SP>> as QueryFragment<_DB, _SP>> and $N others - = note: required for `OnConflictValues>, ...), ...>, ..., ...>` to implement `QueryFragment` + = note: required for `OnConflictValues>, ...), ...>, ..., ...>` to implement `QueryFragment` = note: 2 redundant requirements hidden = note: required for `InsertStatement, ...), ...>, ..., ...>>` to implement `QueryFragment` = note: required for `InsertStatement, ...), ...>, ..., ...>>` to implement `ExecuteDsl` @@ -250,7 +250,7 @@ error[E0277]: the trait bound `diesel::query_builder::upsert::on_conflict_target as QueryFragment<_DB, _SP>> as QueryFragment<_DB, _SP>> and $N others - = note: required for `OnConflictValues>, ...), ...>, ..., ...>` to implement `QueryFragment` + = note: required for `OnConflictValues>, ...), ...>, ..., ...>` to implement `QueryFragment` = note: 2 redundant requirements hidden = note: required for `InsertStatement, ...), ...>, ..., ...>>` to implement `QueryFragment` = note: required for `InsertStatement, ...), ...>, ..., ...>>` to implement `ExecuteDsl` diff --git a/diesel_compile_tests/tests/fail/select_requires_valid_grouping.rs b/diesel_compile_tests/tests/fail/select_requires_valid_grouping.rs index 6b0beb4b550b..e812709c6706 100644 --- a/diesel_compile_tests/tests/fail/select_requires_valid_grouping.rs +++ b/diesel_compile_tests/tests/fail/select_requires_valid_grouping.rs @@ -43,7 +43,7 @@ fn main() { use diesel::dsl; // cases thas should compile - // A column appering in the group by clause should be considered valid for the select clause + // A column appearing in the group by clause should be considered valid for the select clause let source = users::table.group_by(users::name).select(users::name); // If the column appearing in the group by clause is the primary key, any column of that table is a // valid group by clause diff --git a/diesel_derives/src/diesel_public_if.rs b/diesel_derives/src/diesel_public_if.rs index 54b4c6431817..3ba26ebe1be0 100644 --- a/diesel_derives/src/diesel_public_if.rs +++ b/diesel_derives/src/diesel_public_if.rs @@ -55,7 +55,7 @@ impl syn::parse::Parse for CfgInput { } else { panic!( "Incompatible argument list detected. `__diesel_public_if` \ - expects a cfg argument and a optional public_fields" + expects a cfg argument and an optional public_fields" ) } } diff --git a/diesel_derives/src/multiconnection.rs b/diesel_derives/src/multiconnection.rs index a18b0b35dc61..86e5014d5cb6 100644 --- a/diesel_derives/src/multiconnection.rs +++ b/diesel_derives/src/multiconnection.rs @@ -700,8 +700,8 @@ fn generate_bind_collector(connection_types: &[ConnectionVariant]) -> TokenStrea p: std::marker::PhantomData<(ST, T)> } - // we need to have seperate impls for Sized values and str/[u8] as otherwise - // we need seperate impls for `Sized` and `str`/`[u8]` here as + // we need to have separate impls for Sized values and str/[u8] as otherwise + // we need separate impls for `Sized` and `str`/`[u8]` here as // we cannot use `Any::downcast_ref` otherwise (which implies `Sized`) impl PushBoundValueToCollectorDB for PushBoundValueToCollectorImpl where DB: diesel::backend::Backend diff --git a/diesel_dynamic_schema/examples/querying_multiple_types.rs b/diesel_dynamic_schema/examples/querying_multiple_types.rs index 88b28f230f0d..8add6b208795 100644 --- a/diesel_dynamic_schema/examples/querying_multiple_types.rs +++ b/diesel_dynamic_schema/examples/querying_multiple_types.rs @@ -3,7 +3,7 @@ //! To run this: //! //! ```sh -//! cargo run --example querying_mutiple_types --features="diesel/sqlite" +//! cargo run --example querying_multiple_types --features="diesel/sqlite" //! ``` extern crate diesel; extern crate diesel_dynamic_schema; diff --git a/diesel_dynamic_schema/src/table.rs b/diesel_dynamic_schema/src/table.rs index 156398eb6a8d..af402dc3b3b2 100644 --- a/diesel_dynamic_schema/src/table.rs +++ b/diesel_dynamic_schema/src/table.rs @@ -37,7 +37,7 @@ impl Table { Column::new(self.clone(), name) } - /// Gets the name of the table, as especified on creation. + /// Gets the name of the table, as specified on creation. pub fn name(&self) -> &T { &self.name } diff --git a/diesel_migrations/migrations_macros/src/embed_migrations.rs b/diesel_migrations/migrations_macros/src/embed_migrations.rs index 5a887ae2c68b..a3dabb5b3044 100644 --- a/diesel_migrations/migrations_macros/src/embed_migrations.rs +++ b/diesel_migrations/migrations_macros/src/embed_migrations.rs @@ -18,11 +18,11 @@ pub fn expand(path: String) -> proc_macro2::TokenStream { migrations_path_opt ) }); - let embeded_migrations = + let embedded_migrations = migration_literals_from_path(&migrations_expr).expect("Failed to read migration literals"); quote! { - diesel_migrations::EmbeddedMigrations::new(&[#(#embeded_migrations,)*]) + diesel_migrations::EmbeddedMigrations::new(&[#(#embedded_migrations,)*]) } } diff --git a/diesel_migrations/migrations_macros/src/lib.rs b/diesel_migrations/migrations_macros/src/lib.rs index 2e1495fe1b1a..c66961bcedfa 100644 --- a/diesel_migrations/migrations_macros/src/lib.rs +++ b/diesel_migrations/migrations_macros/src/lib.rs @@ -118,5 +118,5 @@ pub fn embed_migrations(input: TokenStream) -> TokenStream { embed_migrations::expand(input.to_string()) .to_string() .parse() - .expect("Failed create embedded migrations instance") + .expect("Failed to create embedded migrations instance") } diff --git a/diesel_migrations/src/embedded_migrations.rs b/diesel_migrations/src/embedded_migrations.rs index 3c8d540d07ae..1cbf1d2f5569 100644 --- a/diesel_migrations/src/embedded_migrations.rs +++ b/diesel_migrations/src/embedded_migrations.rs @@ -8,7 +8,7 @@ use diesel::migration::{Migration, MigrationName, MigrationSource, MigrationVers /// A migration source that embeds migrations into the final binary /// -/// This source can be create via the [`embed_migrations!`](crate::embed_migrations!) +/// This source can be created via the [`embed_migrations!`](crate::embed_migrations!) /// at compile time. #[allow(missing_copy_implementations)] pub struct EmbeddedMigrations { @@ -90,7 +90,7 @@ impl<'a, DB: Backend> Migration for &'a EmbeddedMigration { fn run(&self, conn: &mut dyn diesel::connection::BoxableConnection) -> Result<()> { Ok(conn.batch_execute(self.up).map_err(|e| { let name = DieselMigrationName::from_name(self.name.name) - .expect("We have a vaild name here, we checked this in `embed_migration!`"); + .expect("We have a valid name here, we checked this in `embed_migration!`"); RunMigrationsError::QueryError(name, e) })?) } @@ -99,7 +99,7 @@ impl<'a, DB: Backend> Migration for &'a EmbeddedMigration { match self.down { Some(down) => Ok(conn.batch_execute(down).map_err(|e| { let name = DieselMigrationName::from_name(self.name.name) - .expect("We have a vaild name here, we checked this in `embed_migration!`"); + .expect("We have a valid name here, we checked this in `embed_migration!`"); RunMigrationsError::QueryError(name, e) })?), None => Err(MigrationError::NoMigrationRevertFile.into()), diff --git a/diesel_migrations/src/errors.rs b/diesel_migrations/src/errors.rs index 1bae799cf075..b7239c8e6357 100644 --- a/diesel_migrations/src/errors.rs +++ b/diesel_migrations/src/errors.rs @@ -88,7 +88,7 @@ impl From for MigrationError { #[allow(clippy::enum_variant_names)] #[non_exhaustive] pub enum RunMigrationsError { - /// A general migration error occured + /// A general migration error occurred MigrationError(DieselMigrationName, MigrationError), /// The provided migration included an invalid query QueryError(DieselMigrationName, diesel::result::Error), diff --git a/diesel_tests/tests/insert_from_select.rs b/diesel_tests/tests/insert_from_select.rs index c9a44a51602b..4238d5124554 100644 --- a/diesel_tests/tests/insert_from_select.rs +++ b/diesel_tests/tests/insert_from_select.rs @@ -241,7 +241,7 @@ fn on_conflict_do_nothing_with_select() { let query = users .select((id, name.concat(" says hi"))) - .filter(id.ge(0)) // Sqlite needs a where claues + .filter(id.ge(0)) // Sqlite needs a where clause .insert_into(posts) .into_columns((user_id, title)) .on_conflict_do_nothing(); diff --git a/guide_drafts/migration_guide.md b/guide_drafts/migration_guide.md index 24418630c1ff..591c0354777d 100644 --- a/guide_drafts/migration_guide.md +++ b/guide_drafts/migration_guide.md @@ -22,7 +22,7 @@ Users of `BoxableExpression` might be affected by the following change: Users of tables containing a column of the type `Array` are affected by the following change: -* [Changed nullability of array elemetns](#2-0-0-nullability-of-array-elements) +* [Changed nullability of array elements](#2-0-0-nullability-of-array-elements) Users that implement support for their SQL types or type mappings are affected by the following changes: From ecac494cbf8581117dec216f4270acebad8ef45b Mon Sep 17 00:00:00 2001 From: Omid Rad Date: Mon, 15 May 2023 15:22:08 +0200 Subject: [PATCH 3/9] revert some forgotten bits --- diesel/src/query_dsl/distinct_dsl.rs | 2 +- diesel/src/query_dsl/group_by_dsl.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/diesel/src/query_dsl/distinct_dsl.rs b/diesel/src/query_dsl/distinct_dsl.rs index 9b8bc5981be2..64dab4cbfcc9 100644 --- a/diesel/src/query_dsl/distinct_dsl.rs +++ b/diesel/src/query_dsl/distinct_dsl.rs @@ -31,7 +31,7 @@ where { type Output = dsl::Distinct>>; - fn distinct(self) -> Self::Output { + fn distinct(self) -> dsl::Distinct>> { self.as_query().distinct() } } diff --git a/diesel/src/query_dsl/group_by_dsl.rs b/diesel/src/query_dsl/group_by_dsl.rs index d2bada7e005c..7a712b5af533 100644 --- a/diesel/src/query_dsl/group_by_dsl.rs +++ b/diesel/src/query_dsl/group_by_dsl.rs @@ -18,7 +18,7 @@ pub trait GroupByDsl { type Output; /// See the trait documentation. - fn group_by(self, expr: Expr) -> Self::Output; + fn group_by(self, expr: Expr) -> dsl::GroupBy; } impl GroupByDsl for T From 47392e71b535a2149abb5c6ec082420e7090e38b Mon Sep 17 00:00:00 2001 From: Omid Rad Date: Mon, 22 May 2023 18:26:41 +0200 Subject: [PATCH 4/9] 25 impls, for up to five `order by`s and `distinct on`s --- diesel/Cargo.toml | 1 + diesel/src/pg/mod.rs | 2 + diesel/src/pg/query_builder/distinct_on.rs | 34 ++-- diesel/src/pg/query_builder/mod.rs | 1 + diesel/src/type_impls/tuples.rs | 208 +++++++++++++++++++++ diesel_tests/tests/distinct.rs | 101 +++++----- 6 files changed, 283 insertions(+), 64 deletions(-) diff --git a/diesel/Cargo.toml b/diesel/Cargo.toml index 61389bec0810..1c9b1309d221 100644 --- a/diesel/Cargo.toml +++ b/diesel/Cargo.toml @@ -34,6 +34,7 @@ bitflags = { version = "2.0.0", optional = true } r2d2 = { version = ">= 0.8.2, < 0.9.0", optional = true } itoa = { version = "1.0.0", optional = true } time = { version = "0.3.9", optional = true, features = ["macros"] } +paste = "1.0.12" [dependencies.diesel_derives] version = "~2.0.0" diff --git a/diesel/src/pg/mod.rs b/diesel/src/pg/mod.rs index 0c1f872dfc32..952e8e49d217 100644 --- a/diesel/src/pg/mod.rs +++ b/diesel/src/pg/mod.rs @@ -24,6 +24,8 @@ pub use self::metadata_lookup::PgMetadataLookup; #[doc(inline)] pub use self::query_builder::DistinctOnClause; #[doc(inline)] +pub use self::query_builder::OrderDecorator; +#[doc(inline)] pub use self::query_builder::PgQueryBuilder; #[doc(inline)] pub use self::transaction::TransactionBuilder; diff --git a/diesel/src/pg/query_builder/distinct_on.rs b/diesel/src/pg/query_builder/distinct_on.rs index a8e8bf6af33d..91772bd3605d 100644 --- a/diesel/src/pg/query_builder/distinct_on.rs +++ b/diesel/src/pg/query_builder/distinct_on.rs @@ -1,6 +1,6 @@ use crate::expression::SelectableExpression; use crate::pg::Pg; -use crate::query_builder::order_clause::{NoOrderClause, OrderClause}; +use crate::query_builder::order_clause::NoOrderClause; use crate::query_builder::{ AstPass, FromClause, QueryFragment, QueryId, SelectQuery, SelectStatement, }; @@ -8,6 +8,7 @@ use crate::query_dsl::methods::DistinctOnDsl; use crate::query_dsl::order_dsl::ValidOrderingForDistinct; use crate::result::QueryResult; use crate::{Expression, QuerySource}; +use diesel::query_builder::order_clause::OrderClause; /// Represents `DISTINCT ON (...)` #[derive(Debug, Clone, Copy, QueryId)] @@ -17,26 +18,27 @@ pub struct DistinctOnClause(pub(crate) T); impl ValidOrderingForDistinct> for NoOrderClause {} impl ValidOrderingForDistinct> for OrderClause<(T,)> {} impl ValidOrderingForDistinct> for OrderClause where T: Expression {} -impl ValidOrderingForDistinct> for OrderClause> where - T: Expression -{ -} -impl ValidOrderingForDistinct> for OrderClause> where - T: Expression -{ + +/// A decorator trait for [`OrderClause`] +/// It helps to have bounds on either Col, Asc and Desc. +pub trait OrderDecorator { + /// A column on a database table. + type Column; } -impl ValidOrderingForDistinct> - for OrderClause<(crate::helper_types::Desc,)> +impl OrderDecorator for C where - T: Expression, + C: crate::Column, { + type Column = C; } -impl ValidOrderingForDistinct> - for OrderClause<(crate::helper_types::Asc,)> -where - T: Expression, -{ + +impl OrderDecorator for crate::helper_types::Asc { + type Column = C; +} + +impl OrderDecorator for crate::helper_types::Desc { + type Column = C; } impl QueryFragment for DistinctOnClause diff --git a/diesel/src/pg/query_builder/mod.rs b/diesel/src/pg/query_builder/mod.rs index 0889fe6283f7..3d56825cfee8 100644 --- a/diesel/src/pg/query_builder/mod.rs +++ b/diesel/src/pg/query_builder/mod.rs @@ -8,6 +8,7 @@ pub(crate) mod on_constraint; pub(crate) mod only; mod query_fragment_impls; pub use self::distinct_on::DistinctOnClause; +pub use self::distinct_on::OrderDecorator; /// The PostgreSQL query builder #[allow(missing_debug_implementations)] diff --git a/diesel/src/type_impls/tuples.rs b/diesel/src/type_impls/tuples.rs index 7b252bd7e914..017a0b8e3763 100644 --- a/diesel/src/type_impls/tuples.rs +++ b/diesel/src/type_impls/tuples.rs @@ -16,6 +16,7 @@ use crate::result::QueryResult; use crate::row::*; use crate::sql_types::{HasSqlType, IntoNullable, Nullable, OneIsNullable, SqlType}; use crate::util::{TupleAppend, TupleSize}; +use paste::paste; impl TupleSize for T where @@ -541,3 +542,210 @@ macro_rules! impl_sql_type { } diesel_derives::__diesel_for_each_tuple!(tuple_impls); + +macro_rules! valid_distinct_order { + ( + distinct_t = [$($distinct_t: ident),*], + order_t = [$($order_t: ident),*], + bounds_on = [$($bounds_on: ident),*], + )=> { + valid_distinct_order!{ + @build + distinct_t = [$($distinct_t),*], + order_t = [$($order_t),*], + bounds = [], + bounds_on = [$($bounds_on),*], + } + }; + ( + @build + distinct_t = [$($distinct_t: tt)*], + order_t = [$($order_t: tt)*], + bounds = [$($bounds: tt)*], + bounds_on = [$T1: ident, $($T: ident),*], + )=> { + paste! { + valid_distinct_order!{ + @build + distinct_t = [$($distinct_t)*], + order_t = [$($order_t)*], + bounds = [$($bounds)* [<_O $T1>]: crate::pg::OrderDecorator, $T1: Column,], + bounds_on = [$($T),*], + } + } + }; + ( + @build + distinct_t = [$($distinct_t: tt)*], + order_t = [$($order_t: tt)*], + bounds = [$($bounds: tt)*], + bounds_on = [$T1: ident], + )=> { + paste! { + #[allow(unused_parens)] + #[cfg(feature = "postgres")] + impl<$($order_t)* , $($distinct_t)*> crate::query_dsl::order_dsl::ValidOrderingForDistinct> + for crate::query_builder::order_clause::OrderClause<($($order_t)*,)> + where $($bounds)* [<_O $T1>]: crate::pg::OrderDecorator, $T1: Column, + {} + } + }; +} + +valid_distinct_order! { + distinct_t = [T1], + order_t = [_OT1], + bounds_on = [T1], +} + +valid_distinct_order! { + distinct_t = [T1], + order_t = [_OT1, _OT2], + bounds_on = [T1], +} + +valid_distinct_order! { + distinct_t = [T1], + order_t = [_OT1, _OT2, _OT3], + bounds_on = [T1], +} + +valid_distinct_order! { + distinct_t = [T1], + order_t = [_OT1, _OT2, _OT3, _OT4], + bounds_on = [T1], +} + +valid_distinct_order! { + distinct_t = [T1], + order_t = [_OT1, _OT2, _OT3, _OT4, _OT5], + bounds_on = [T1], +} + +// + +valid_distinct_order! { + distinct_t = [T1, T2], + order_t = [_OT1], + bounds_on = [T1], +} + +valid_distinct_order! { + distinct_t = [T1, T2], + order_t = [_OT1, _OT2], + bounds_on = [T1, T2], +} + +valid_distinct_order! { + distinct_t = [T1, T2], + order_t = [_OT1, _OT2, _OT3], + bounds_on = [T1, T2], +} + +valid_distinct_order! { + distinct_t = [T1, T2], + order_t = [_OT1, _OT2, _OT3, _OT4], + bounds_on = [T1, T2], +} + +valid_distinct_order! { + distinct_t = [T1, T2], + order_t = [_OT1, _OT2, _OT3, _OT4, _OT5], + bounds_on = [T1, T2], +} + +// + +valid_distinct_order! { + distinct_t = [T1, T2, T3], + order_t = [_OT1], + bounds_on = [T1], +} + +valid_distinct_order! { + distinct_t = [T1, T2, T3], + order_t = [_OT1, _OT2], + bounds_on = [T1, T2], +} + +valid_distinct_order! { + distinct_t = [T1, T2, T3], + order_t = [_OT1, _OT2, _OT3], + bounds_on = [T1, T2, T3], +} + +valid_distinct_order! { + distinct_t = [T1, T2, T3], + order_t = [_OT1, _OT2, _OT3, _OT4], + bounds_on = [T1, T2, T3], +} + +valid_distinct_order! { + distinct_t = [T1, T2, T3], + order_t = [_OT1, _OT2, _OT3, _OT4, _OT5], + bounds_on = [T1, T2, T3], +} + +// + +valid_distinct_order! { + distinct_t = [T1, T2, T3, T4], + order_t = [_OT1], + bounds_on = [T1], +} + +valid_distinct_order! { + distinct_t = [T1, T2, T3, T4], + order_t = [_OT1, _OT2], + bounds_on = [T1, T2], +} + +valid_distinct_order! { + distinct_t = [T1, T2, T3, T4], + order_t = [_OT1, _OT2, _OT3], + bounds_on = [T1, T2, T3], +} + +valid_distinct_order! { + distinct_t = [T1, T2, T3, T4], + order_t = [_OT1, _OT2, _OT3, _OT4], + bounds_on = [T1, T2, T3, T4], +} + +valid_distinct_order! { + distinct_t = [T1, T2, T3, T4], + order_t = [_OT1, _OT2, _OT3, _OT4, _OT5], + bounds_on = [T1, T2, T3, T4], +} + +// + +valid_distinct_order! { + distinct_t = [T1, T2, T3, T4, T5], + order_t = [_OT1], + bounds_on = [T1], +} + +valid_distinct_order! { + distinct_t = [T1, T2, T3, T4, T5], + order_t = [_OT1, _OT2], + bounds_on = [T1, T2], +} + +valid_distinct_order! { + distinct_t = [T1, T2, T3, T4, T5], + order_t = [_OT1, _OT2, _OT3], + bounds_on = [T1, T2, T3], +} + +valid_distinct_order! { + distinct_t = [T1, T2, T3, T4, T5], + order_t = [_OT1, _OT2, _OT3, _OT4], + bounds_on = [T1, T2, T3, T4], +} + +valid_distinct_order! { + distinct_t = [T1, T2, T3, T4, T5], + order_t = [_OT1, _OT2, _OT3, _OT4, _OT5], + bounds_on = [T1, T2, T3, T4, T5], +} diff --git a/diesel_tests/tests/distinct.rs b/diesel_tests/tests/distinct.rs index 84ba2e72b08f..4797e0fed4fa 100644 --- a/diesel_tests/tests/distinct.rs +++ b/diesel_tests/tests/distinct.rs @@ -140,56 +140,61 @@ fn distinct_on_select_order_by_two_columns() { #[cfg(feature = "postgres")] #[test] -fn distinct_on_two_columns() { - use crate::schema::users::dsl::*; - - let connection = &mut connection(); - diesel::sql_query( - "INSERT INTO users (name, hair_color) VALUES ('Sean', 'black'), ('Sean', 'aqua'), ('Tess', 'bronze'), ('Tess', 'champagne')", - ).execute(connection) - .unwrap(); - - let source = users - .select((name, hair_color)) - .order((name, hair_color.desc())) - .distinct_on((id, name)); - let expected_data = vec![ - NewUser::new("Sean", Some("black")), - NewUser::new("Tess", Some("champagne")), - ]; - let data: Vec<_> = source.load(connection).unwrap(); - - assert_eq!(expected_data, data); - - let source = users - .select((name, hair_color)) - .order((name.asc(), hair_color.desc())) - .distinct_on((id, name)); - let data: Vec<_> = source.load(connection).unwrap(); - - assert_eq!(expected_data, data); - - let source = users - .select((name, hair_color)) - .order((name.desc(), hair_color.desc())) - .distinct_on((id, name)); - let expected_data = vec![ - NewUser::new("Tess", Some("champagne")), - NewUser::new("Sean", Some("black")), +fn distinct_of_multiple_columns() { + use crate::schema::posts; + use crate::schema::users; + + let mut connection = connection_with_sean_and_tess_in_users_table(); + + let sean = find_user_by_name("Sean", &mut connection); + let tess = find_user_by_name("Tess", &mut connection); + + let new_posts = vec![ + NewPost::new(sean.id, "1", Some("1")), + NewPost::new(sean.id, "2", Some("2")), + NewPost::new(sean.id, "3", Some("1")), + NewPost::new(sean.id, "4", Some("2")), + NewPost::new(tess.id, "5", Some("1")), + NewPost::new(tess.id, "6", Some("2")), + NewPost::new(tess.id, "7", Some("1")), + NewPost::new(tess.id, "8", Some("2")), ]; - let data: Vec<_> = source.load(connection).unwrap(); - - assert_eq!(expected_data, data); + insert_into(posts::table) + .values(&new_posts) + .execute(&mut connection) + .unwrap(); + let posts = posts::table + .order(posts::id) + .load::(&mut connection) + .unwrap(); - let source = users - .select((name, hair_color)) - .order((name.desc(), hair_color)) - .distinct_on((id, name)); - let expected_data = vec![ - NewUser::new("Tess", Some("bronze")), - NewUser::new("Sean", Some("aqua")), + let data = posts::table + .inner_join(users::table) + .order((users::id, posts::body, posts::title)) + .distinct_on((users::id, posts::body)) + .load(&mut connection); + let expected = vec![ + ((posts[0].clone(), sean.clone())), + ((posts[1].clone(), sean.clone())), + ((posts[4].clone(), tess.clone())), + ((posts[5].clone(), tess.clone())), ]; - let data: Vec<_> = source.load(connection).unwrap(); - assert_eq!(expected_data, data); + assert_eq!(Ok(expected), data); } + +// #[cfg(feature = "postgres")] +// #[test] +// #[should_panic(expected = "TBD")] +// fn invalid_distinct_on_or_order_detected() { +// use crate::schema::users; +// use crate::schema::posts; +// +// let mut connection = connection(); +// +// posts::table +// .inner_join(users::table) +// .order((users::id, posts::title)) +// .distinct_on((users::id, posts::body)) +// .load(&mut connection); +// } From 20e00a46d5362b5cb2049b3b51ebce61b7acb32a Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Wed, 24 May 2023 16:58:58 +0200 Subject: [PATCH 5/9] Provide non-conflicting impls up to a tuple size of 5 (This also uses a different strategy to generate these impls as that results in more maintainable code) --- diesel/src/pg/query_builder/distinct_on.rs | 53 ++++- diesel/src/type_impls/tuples.rs | 220 -------------------- diesel_derives/src/diesel_for_each_tuple.rs | 29 ++- 3 files changed, 76 insertions(+), 226 deletions(-) diff --git a/diesel/src/pg/query_builder/distinct_on.rs b/diesel/src/pg/query_builder/distinct_on.rs index 91772bd3605d..68703c9df160 100644 --- a/diesel/src/pg/query_builder/distinct_on.rs +++ b/diesel/src/pg/query_builder/distinct_on.rs @@ -7,7 +7,7 @@ use crate::query_builder::{ use crate::query_dsl::methods::DistinctOnDsl; use crate::query_dsl::order_dsl::ValidOrderingForDistinct; use crate::result::QueryResult; -use crate::{Expression, QuerySource}; +use crate::QuerySource; use diesel::query_builder::order_clause::OrderClause; /// Represents `DISTINCT ON (...)` @@ -17,7 +17,56 @@ pub struct DistinctOnClause(pub(crate) T); impl ValidOrderingForDistinct> for NoOrderClause {} impl ValidOrderingForDistinct> for OrderClause<(T,)> {} -impl ValidOrderingForDistinct> for OrderClause where T: Expression {} +impl ValidOrderingForDistinct> for OrderClause where T: crate::Column {} + +macro_rules! valid_ordering { + (@skip: ($ST1: ident, $($ST:ident,)*), $T1:ident, ) => {}; + (@skip: ($ST1: ident, $($ST:ident,)*), $T1:ident, $($T:ident,)+) => { + valid_ordering!(($($ST,)*), ($ST1,), $($T,)*); + }; + (($ST1: ident,), ($($OT:ident,)*), $T1:ident,) => { + impl<$T1, $ST1, $($OT,)*> ValidOrderingForDistinct> for OrderClause<($T1,)> + where $T1: crate::pg::OrderDecorator, + {} + impl<$T1, $ST1, $($OT,)*> ValidOrderingForDistinct> for OrderClause<($ST1, $($OT,)*)> + where $ST1: crate::pg::OrderDecorator, + {} + + impl<$T1, $ST1, $($OT,)*> ValidOrderingForDistinct> for OrderClause<($ST1, $($OT,)*)> + where $ST1: crate::pg::OrderDecorator, + $T1: crate::Column, + {} + }; + (($ST1: ident, $($ST:ident,)*), ($($OT: ident,)*), $T1:ident, $($T:ident,)+) => { + impl<$T1, $($T,)* $ST1, $($ST,)* $($OT,)*> ValidOrderingForDistinct> for OrderClause<($T1, $($T,)*)> + where $T1: crate::pg::OrderDecorator, + $($T: crate::pg::OrderDecorator,)* + {} + impl<$T1, $($T,)* $ST1, $($ST,)* $($OT,)*> ValidOrderingForDistinct> for OrderClause<($ST1, $($ST,)* $($OT,)*)> + where $ST1: crate::pg::OrderDecorator, + $($ST: crate::pg::OrderDecorator,)* + {} + valid_ordering!(($($ST,)*), ($($OT,)* $ST1,), $($T,)*); + }; + ($( + $Tuple:tt { + $(($idx:tt) -> $T:ident, $ST:ident, $TT:ident,)+ + } + )+) => { + $( + impl<$($T,)* $($ST,)*> ValidOrderingForDistinct> for OrderClause<($($ST,)*)> + where $($ST: crate::pg::OrderDecorator,)* + {} + valid_ordering!(@skip: ($($ST,)*), $($T,)*); + )* + } +} + +// we only generate these impl up to a tuple size of 5 as we generate n*n + 4 impls here +// If we would generate these impls up to max_table_column_count tuple elements that +// would be a really large number for 128 tuple elements (~64k trait impls) +// It's fine to increase this number at some point in the future gradually +diesel_derives::__diesel_for_each_tuple!(valid_ordering, 5); /// A decorator trait for [`OrderClause`] /// It helps to have bounds on either Col, Asc and Desc. diff --git a/diesel/src/type_impls/tuples.rs b/diesel/src/type_impls/tuples.rs index 017a0b8e3763..e47cc6273949 100644 --- a/diesel/src/type_impls/tuples.rs +++ b/diesel/src/type_impls/tuples.rs @@ -16,7 +16,6 @@ use crate::result::QueryResult; use crate::row::*; use crate::sql_types::{HasSqlType, IntoNullable, Nullable, OneIsNullable, SqlType}; use crate::util::{TupleAppend, TupleSize}; -use paste::paste; impl TupleSize for T where @@ -363,18 +362,6 @@ macro_rules! tuple_impls { impl<$($ST,)*> SqlTypeOrSelectable for Nullable<($($ST,)*)> where ($($ST,)*): SqlTypeOrSelectable {} - - #[cfg(feature = "postgres")] - impl<__D, $($T,)*> crate::query_dsl::order_dsl::ValidOrderingForDistinct> - for crate::query_builder::order_clause::OrderClause<(__D, $($T,)*)> {} - - #[cfg(feature = "postgres")] - impl<__D, $($T,)*> crate::query_dsl::order_dsl::ValidOrderingForDistinct> - for crate::query_builder::order_clause::OrderClause<(crate::helper_types::Desc<__D>, $($T,)*)> {} - - #[cfg(feature = "postgres")] - impl<__D, $($T,)*> crate::query_dsl::order_dsl::ValidOrderingForDistinct> - for crate::query_builder::order_clause::OrderClause<(crate::helper_types::Asc<__D>, $($T,)*)> {} )+ } } @@ -542,210 +529,3 @@ macro_rules! impl_sql_type { } diesel_derives::__diesel_for_each_tuple!(tuple_impls); - -macro_rules! valid_distinct_order { - ( - distinct_t = [$($distinct_t: ident),*], - order_t = [$($order_t: ident),*], - bounds_on = [$($bounds_on: ident),*], - )=> { - valid_distinct_order!{ - @build - distinct_t = [$($distinct_t),*], - order_t = [$($order_t),*], - bounds = [], - bounds_on = [$($bounds_on),*], - } - }; - ( - @build - distinct_t = [$($distinct_t: tt)*], - order_t = [$($order_t: tt)*], - bounds = [$($bounds: tt)*], - bounds_on = [$T1: ident, $($T: ident),*], - )=> { - paste! { - valid_distinct_order!{ - @build - distinct_t = [$($distinct_t)*], - order_t = [$($order_t)*], - bounds = [$($bounds)* [<_O $T1>]: crate::pg::OrderDecorator, $T1: Column,], - bounds_on = [$($T),*], - } - } - }; - ( - @build - distinct_t = [$($distinct_t: tt)*], - order_t = [$($order_t: tt)*], - bounds = [$($bounds: tt)*], - bounds_on = [$T1: ident], - )=> { - paste! { - #[allow(unused_parens)] - #[cfg(feature = "postgres")] - impl<$($order_t)* , $($distinct_t)*> crate::query_dsl::order_dsl::ValidOrderingForDistinct> - for crate::query_builder::order_clause::OrderClause<($($order_t)*,)> - where $($bounds)* [<_O $T1>]: crate::pg::OrderDecorator, $T1: Column, - {} - } - }; -} - -valid_distinct_order! { - distinct_t = [T1], - order_t = [_OT1], - bounds_on = [T1], -} - -valid_distinct_order! { - distinct_t = [T1], - order_t = [_OT1, _OT2], - bounds_on = [T1], -} - -valid_distinct_order! { - distinct_t = [T1], - order_t = [_OT1, _OT2, _OT3], - bounds_on = [T1], -} - -valid_distinct_order! { - distinct_t = [T1], - order_t = [_OT1, _OT2, _OT3, _OT4], - bounds_on = [T1], -} - -valid_distinct_order! { - distinct_t = [T1], - order_t = [_OT1, _OT2, _OT3, _OT4, _OT5], - bounds_on = [T1], -} - -// - -valid_distinct_order! { - distinct_t = [T1, T2], - order_t = [_OT1], - bounds_on = [T1], -} - -valid_distinct_order! { - distinct_t = [T1, T2], - order_t = [_OT1, _OT2], - bounds_on = [T1, T2], -} - -valid_distinct_order! { - distinct_t = [T1, T2], - order_t = [_OT1, _OT2, _OT3], - bounds_on = [T1, T2], -} - -valid_distinct_order! { - distinct_t = [T1, T2], - order_t = [_OT1, _OT2, _OT3, _OT4], - bounds_on = [T1, T2], -} - -valid_distinct_order! { - distinct_t = [T1, T2], - order_t = [_OT1, _OT2, _OT3, _OT4, _OT5], - bounds_on = [T1, T2], -} - -// - -valid_distinct_order! { - distinct_t = [T1, T2, T3], - order_t = [_OT1], - bounds_on = [T1], -} - -valid_distinct_order! { - distinct_t = [T1, T2, T3], - order_t = [_OT1, _OT2], - bounds_on = [T1, T2], -} - -valid_distinct_order! { - distinct_t = [T1, T2, T3], - order_t = [_OT1, _OT2, _OT3], - bounds_on = [T1, T2, T3], -} - -valid_distinct_order! { - distinct_t = [T1, T2, T3], - order_t = [_OT1, _OT2, _OT3, _OT4], - bounds_on = [T1, T2, T3], -} - -valid_distinct_order! { - distinct_t = [T1, T2, T3], - order_t = [_OT1, _OT2, _OT3, _OT4, _OT5], - bounds_on = [T1, T2, T3], -} - -// - -valid_distinct_order! { - distinct_t = [T1, T2, T3, T4], - order_t = [_OT1], - bounds_on = [T1], -} - -valid_distinct_order! { - distinct_t = [T1, T2, T3, T4], - order_t = [_OT1, _OT2], - bounds_on = [T1, T2], -} - -valid_distinct_order! { - distinct_t = [T1, T2, T3, T4], - order_t = [_OT1, _OT2, _OT3], - bounds_on = [T1, T2, T3], -} - -valid_distinct_order! { - distinct_t = [T1, T2, T3, T4], - order_t = [_OT1, _OT2, _OT3, _OT4], - bounds_on = [T1, T2, T3, T4], -} - -valid_distinct_order! { - distinct_t = [T1, T2, T3, T4], - order_t = [_OT1, _OT2, _OT3, _OT4, _OT5], - bounds_on = [T1, T2, T3, T4], -} - -// - -valid_distinct_order! { - distinct_t = [T1, T2, T3, T4, T5], - order_t = [_OT1], - bounds_on = [T1], -} - -valid_distinct_order! { - distinct_t = [T1, T2, T3, T4, T5], - order_t = [_OT1, _OT2], - bounds_on = [T1, T2], -} - -valid_distinct_order! { - distinct_t = [T1, T2, T3, T4, T5], - order_t = [_OT1, _OT2, _OT3], - bounds_on = [T1, T2, T3], -} - -valid_distinct_order! { - distinct_t = [T1, T2, T3, T4, T5], - order_t = [_OT1, _OT2, _OT3, _OT4], - bounds_on = [T1, T2, T3, T4], -} - -valid_distinct_order! { - distinct_t = [T1, T2, T3, T4, T5], - order_t = [_OT1, _OT2, _OT3, _OT4, _OT5], - bounds_on = [T1, T2, T3, T4, T5], -} diff --git a/diesel_derives/src/diesel_for_each_tuple.rs b/diesel_derives/src/diesel_for_each_tuple.rs index 948ddfb8f22a..0d3723a011fe 100644 --- a/diesel_derives/src/diesel_for_each_tuple.rs +++ b/diesel_derives/src/diesel_for_each_tuple.rs @@ -10,10 +10,10 @@ pub const MAX_TUPLE_SIZE: i32 = 64; #[cfg(feature = "128-column-tables")] pub const MAX_TUPLE_SIZE: i32 = 128; -pub(crate) fn expand(input: Ident) -> TokenStream { +pub(crate) fn expand(input: ForEachTupleInput) -> TokenStream { let call_side = Span::mixed_site(); - let pairs = (0..MAX_TUPLE_SIZE as usize) + let pairs = (0..input.max_size as usize) .map(|i| { let t = Ident::new(&format!("T{i}"), call_side); let st = Ident::new(&format!("ST{i}"), call_side); @@ -23,9 +23,9 @@ pub(crate) fn expand(input: Ident) -> TokenStream { }) .collect::>(); - let mut out = Vec::with_capacity(MAX_TUPLE_SIZE as usize); + let mut out = Vec::with_capacity(input.max_size as usize); - for i in 0..MAX_TUPLE_SIZE { + for i in 0..input.max_size { let items = &pairs[0..=i as usize]; let tuple = i + 1; out.push(quote! { @@ -34,6 +34,7 @@ pub(crate) fn expand(input: Ident) -> TokenStream { } }); } + let input = input.inner; quote! { #input! { @@ -41,3 +42,23 @@ pub(crate) fn expand(input: Ident) -> TokenStream { } } } + +pub struct ForEachTupleInput { + inner: Ident, + max_size: i32, +} + +impl syn::parse::Parse for ForEachTupleInput { + fn parse(input: syn::parse::ParseStream) -> syn::Result { + let inner = input.parse()?; + let max_size = if input.peek(syn::Token![,]) { + let _ = input.parse::(); + input.parse::()?.base10_parse()? + } else if input.is_empty() { + MAX_TUPLE_SIZE + } else { + unreachable!("Invalid syntax") + }; + Ok(Self { inner, max_size }) + } +} From 258d64b27996c33dce4654893e99d421f38cbc51 Mon Sep 17 00:00:00 2001 From: Ten0 <9094255+Ten0@users.noreply.github.com> Date: Tue, 23 May 2023 15:05:24 +0200 Subject: [PATCH 6/9] Implement field length introspection and make available in schema (#3552) Resolves #3551 --- .github/workflows/ci.yml | 4 + diesel/src/lib.rs | 1 + diesel/src/query_source/mod.rs | 9 ++ .../infer_schema_internals/data_structures.rs | 67 ++++---- .../src/infer_schema_internals/mysql.rs | 61 +++++++- diesel_cli/src/infer_schema_internals/pg.rs | 69 ++++++++- .../src/infer_schema_internals/sqlite.rs | 16 ++ diesel_cli/src/migrations/diff_schema.rs | 18 ++- diesel_cli/src/print_schema.rs | 22 ++- .../mysql/schema_out.rs/expected.snap | 3 +- .../postgres/schema_out.rs/expected.snap | 3 +- .../mysql/down.sql/expected.snap | 3 +- .../postgres/down.sql/expected.snap | 9 +- .../mysql/expected.snap | 1 + .../mysql/expected.snap | 3 + .../mysql/expected.snap | 2 + diesel_derives/src/table.rs | 10 ++ diesel_table_macro_syntax/src/lib.rs | 143 +++++++++--------- diesel_table_macro_syntax/tests/basic.rs | 18 +++ diesel_table_macro_syntax/tests/basic.rs.in | 7 + 20 files changed, 327 insertions(+), 142 deletions(-) create mode 100644 diesel_table_macro_syntax/tests/basic.rs create mode 100644 diesel_table_macro_syntax/tests/basic.rs.in diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d5f76f7095dd..322d62d727c7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -246,6 +246,10 @@ jobs: shell: bash run: cargo +${{ matrix.rust }} test --manifest-path diesel_migrations/migrations_macros/Cargo.toml --features "diesel/${{ matrix.backend }} ${{ matrix.backend }}" + - name: Test table-macro-syntax + shell: bash + run: cargo +${{ matrix.rust }} test --manifest-path diesel_table_macro_syntax/Cargo.toml + - name: Test diesel_migrations shell: bash run: cargo +${{ matrix.rust }} test --manifest-path diesel_migrations/Cargo.toml --features "${{ matrix.backend }} diesel/${{ matrix.backend }}" diff --git a/diesel/src/lib.rs b/diesel/src/lib.rs index af6cde080a9e..83736ae9df99 100644 --- a/diesel/src/lib.rs +++ b/diesel/src/lib.rs @@ -649,6 +649,7 @@ pub mod prelude { pub use crate::query_dsl::{ BelongingToDsl, CombineDsl, JoinOnDsl, QueryDsl, RunQueryDsl, SaveChangesDsl, }; + pub use crate::query_source::SizeRestrictedColumn as _; #[doc(inline)] pub use crate::query_source::{Column, JoinTo, QuerySource, Table}; #[doc(inline)] diff --git a/diesel/src/query_source/mod.rs b/diesel/src/query_source/mod.rs index a404284227ba..6c898d25e973 100644 --- a/diesel/src/query_source/mod.rs +++ b/diesel/src/query_source/mod.rs @@ -189,3 +189,12 @@ mod impls_which_are_only_here_to_improve_error_messages { type Selection = this_table_appears_in_your_query_more_than_once_and_must_be_aliased; } } + +/// Max length for columns of type Char/Varchar... +/// +/// If a given column has a such constraint, this trait will be implemented and specify that +/// length. +pub trait SizeRestrictedColumn: Column { + /// Max length of that column + const MAX_LENGTH: usize; +} diff --git a/diesel_cli/src/infer_schema_internals/data_structures.rs b/diesel_cli/src/infer_schema_internals/data_structures.rs index 2a7964a4a061..4f51b04e73e1 100644 --- a/diesel_cli/src/infer_schema_internals/data_structures.rs +++ b/diesel_cli/src/infer_schema_internals/data_structures.rs @@ -1,11 +1,7 @@ -#[cfg(feature = "uses_information_schema")] -use diesel::backend::Backend; -use diesel::deserialize::{self, FromStaticSqlRow, Queryable}; -#[cfg(feature = "sqlite")] -use diesel::sqlite::Sqlite; +use diesel_table_macro_syntax::ColumnDef; + +use std::error::Error; -#[cfg(feature = "uses_information_schema")] -use super::information_schema::DefaultSchema; use super::table_data::TableName; #[derive(Debug, Clone, PartialEq, Eq)] @@ -14,6 +10,7 @@ pub struct ColumnInformation { pub type_name: String, pub type_schema: Option, pub nullable: bool, + pub max_length: Option, pub comment: Option, } @@ -25,10 +22,26 @@ pub struct ColumnType { pub is_array: bool, pub is_nullable: bool, pub is_unsigned: bool, + pub max_length: Option, } -impl From<&syn::TypePath> for ColumnType { - fn from(t: &syn::TypePath) -> Self { +impl ColumnType { + pub(crate) fn for_column_def(c: &ColumnDef) -> Result> { + Ok(Self::for_type_path( + &c.tpe, + c.max_length + .as_ref() + .map(|l| { + l.base10_parse::() + .map_err(|e| -> Box { + format!("Column length literal can't be parsed as u64: {e}").into() + }) + }) + .transpose()?, + )) + } + + fn for_type_path(t: &syn::TypePath, max_length: Option) -> Self { let last = t .path .segments @@ -42,6 +55,7 @@ impl From<&syn::TypePath> for ColumnType { is_array: last.ident == "Array", is_nullable: last.ident == "Nullable", is_unsigned: last.ident == "Unsigned", + max_length, }; let sql_name = if !ret.is_nullable && !ret.is_array && !ret.is_unsigned { @@ -53,7 +67,7 @@ impl From<&syn::TypePath> for ColumnType { } else if let syn::PathArguments::AngleBracketed(ref args) = last.arguments { let arg = args.args.first().expect("There is at least one argument"); if let syn::GenericArgument::Type(syn::Type::Path(p)) = arg { - let s = Self::from(p); + let s = Self::for_type_path(p, max_length); ret.is_nullable |= s.is_nullable; ret.is_array |= s.is_array; ret.is_unsigned |= s.is_unsigned; @@ -110,6 +124,7 @@ impl ColumnInformation { type_name: U, type_schema: Option, nullable: bool, + max_length: Option, comment: Option, ) -> Self where @@ -121,42 +136,12 @@ impl ColumnInformation { type_name: type_name.into(), type_schema, nullable, + max_length, comment, } } } -#[cfg(feature = "uses_information_schema")] -impl Queryable for ColumnInformation -where - DB: Backend + DefaultSchema, - (String, String, Option, String, Option): FromStaticSqlRow, -{ - type Row = (String, String, Option, String, Option); - - fn build(row: Self::Row) -> deserialize::Result { - Ok(ColumnInformation::new( - row.0, - row.1, - row.2, - row.3 == "YES", - row.4, - )) - } -} - -#[cfg(feature = "sqlite")] -impl Queryable for ColumnInformation -where - (i32, String, String, bool, Option, bool, i32): FromStaticSqlRow, -{ - type Row = (i32, String, String, bool, Option, bool, i32); - - fn build(row: Self::Row) -> deserialize::Result { - Ok(ColumnInformation::new(row.1, row.2, None, !row.3, None)) - } -} - #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] pub struct ForeignKeyConstraint { pub child_table: TableName, diff --git a/diesel_cli/src/infer_schema_internals/mysql.rs b/diesel_cli/src/infer_schema_internals/mysql.rs index 9a4e14ea7912..6a07a0443c1e 100644 --- a/diesel_cli/src/infer_schema_internals/mysql.rs +++ b/diesel_cli/src/infer_schema_internals/mysql.rs @@ -1,3 +1,4 @@ +use diesel::deserialize::{self, FromStaticSqlRow, Queryable}; use diesel::mysql::{Mysql, MysqlConnection}; use diesel::*; use heck::ToUpperCamelCase; @@ -33,14 +34,60 @@ pub fn get_table_data( column_type, type_schema, __is_nullable, + character_maximum_length, // MySQL comments are not nullable and are empty strings if not set null_if_text(column_comment, ""), )) .filter(table_name.eq(&table.sql_name)) .filter(table_schema.eq(schema_name)); - match column_sorting { - ColumnSorting::OrdinalPosition => query.order(ordinal_position).load(conn), - ColumnSorting::Name => query.order(column_name).load(conn), + let mut table_columns: Vec = match column_sorting { + ColumnSorting::OrdinalPosition => query.order(ordinal_position).load(conn)?, + ColumnSorting::Name => query.order(column_name).load(conn)?, + }; + for c in &mut table_columns { + if c.max_length.is_some() && !c.type_name.contains('(') { + // Mysql returns something in character_maximum_length regardless + // of whether it's specified at field creation time + // In addition there is typically a shared limitation at row level, + // so it's typically not even the real max. + // This basically means no max. + // https://dev.mysql.com/doc/refman/8.0/en/column-count-limit.html + // https://chartio.com/resources/tutorials/understanding-strorage-sizes-for-mysql-text-data-types/ + c.max_length = None; + } + } + Ok(table_columns) +} + +impl Queryable for ColumnInformation +where + ( + String, + String, + Option, + String, + Option, + Option, + ): FromStaticSqlRow, +{ + type Row = ( + String, + String, + Option, + String, + Option, + Option, + ); + + fn build(row: Self::Row) -> deserialize::Result { + Ok(ColumnInformation::new( + row.0, + row.1, + row.2, + row.3 == "YES", + row.4, + row.5, + )) } } @@ -84,7 +131,8 @@ mod information_schema { column_name -> VarChar, #[sql_name = "is_nullable"] __is_nullable -> VarChar, - ordinal_position -> BigInt, + character_maximum_length -> Nullable>, + ordinal_position -> Unsigned, udt_name -> VarChar, udt_schema -> VarChar, column_type -> VarChar, @@ -171,6 +219,7 @@ pub fn determine_column_type( is_array: false, is_nullable: attr.nullable, is_unsigned: unsigned, + max_length: attr.max_length, }) } @@ -324,9 +373,11 @@ mod test { "varchar(255)", None, false, + Some(255), Some("column comment".to_string()), ); - let id_without_comment = ColumnInformation::new("id", "varchar(255)", None, false, None); + let id_without_comment = + ColumnInformation::new("id", "varchar(255)", None, false, Some(255), None); assert_eq!( Ok(vec![id_with_comment]), get_table_data(&mut connection, &table_1, &ColumnSorting::OrdinalPosition) diff --git a/diesel_cli/src/infer_schema_internals/pg.rs b/diesel_cli/src/infer_schema_internals/pg.rs index 1cf7f5968649..bb0635979aa6 100644 --- a/diesel_cli/src/infer_schema_internals/pg.rs +++ b/diesel_cli/src/infer_schema_internals/pg.rs @@ -2,7 +2,14 @@ use super::data_structures::*; use super::information_schema::DefaultSchema; use super::TableName; use crate::print_schema::ColumnSorting; -use diesel::{dsl::AsExprOf, expression::AsExpression, pg::Pg, prelude::*, sql_types}; +use diesel::{ + deserialize::{self, FromStaticSqlRow, Queryable}, + dsl::AsExprOf, + expression::AsExpression, + pg::Pg, + prelude::*, + sql_types, +}; use heck::ToUpperCamelCase; use std::borrow::Cow; use std::error::Error; @@ -48,6 +55,7 @@ pub fn determine_column_type( is_array, is_nullable: attr.nullable, is_unsigned: false, + max_length: attr.max_length, }) } @@ -79,6 +87,7 @@ pub fn get_table_data( udt_name, udt_schema.nullable(), __is_nullable, + character_maximum_length, col_description(regclass(table), ordinal_position), )) .filter(table_name.eq(&table.sql_name)) @@ -89,6 +98,44 @@ pub fn get_table_data( } } +impl Queryable for ColumnInformation +where + ( + String, + String, + Option, + String, + Option, + Option, + ): FromStaticSqlRow, +{ + type Row = ( + String, + String, + Option, + String, + Option, + Option, + ); + + fn build(row: Self::Row) -> deserialize::Result { + Ok(ColumnInformation::new( + row.0, + row.1, + row.2, + row.3 == "YES", + row.4 + .map(|n| { + std::convert::TryInto::try_into(n).map_err(|e| { + format!("Max column length can't be converted to u64: {e} (got: {n})") + }) + }) + .transpose()?, + row.5, + )) + } +} + sql_function!(fn obj_description(oid: sql_types::Oid, catalog: sql_types::Text) -> Nullable); pub fn get_table_comment( @@ -108,6 +155,7 @@ mod information_schema { column_name -> VarChar, #[sql_name = "is_nullable"] __is_nullable -> VarChar, + character_maximum_length -> Nullable, ordinal_position -> BigInt, udt_name -> VarChar, udt_schema -> VarChar, @@ -221,7 +269,7 @@ mod test { .execute(&mut connection) .unwrap(); diesel::sql_query( - "CREATE TABLE test_schema.table_1 (id SERIAL PRIMARY KEY, text_col VARCHAR, not_null TEXT NOT NULL)", + "CREATE TABLE test_schema.table_1 (id SERIAL PRIMARY KEY, text_col VARCHAR(128), not_null TEXT NOT NULL)", ).execute(&mut connection) .unwrap(); diesel::sql_query("COMMENT ON COLUMN test_schema.table_1.id IS 'column comment'") @@ -239,12 +287,21 @@ mod test { "int4", pg_catalog.clone(), false, + None, Some("column comment".to_string()), ); - let text_col = - ColumnInformation::new("text_col", "varchar", pg_catalog.clone(), true, None); - let not_null = ColumnInformation::new("not_null", "text", pg_catalog.clone(), false, None); - let array_col = ColumnInformation::new("array_col", "_varchar", pg_catalog, false, None); + let text_col = ColumnInformation::new( + "text_col", + "varchar", + pg_catalog.clone(), + true, + Some(128), + None, + ); + let not_null = + ColumnInformation::new("not_null", "text", pg_catalog.clone(), false, None, None); + let array_col = + ColumnInformation::new("array_col", "_varchar", pg_catalog, false, None, None); 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/sqlite.rs b/diesel_cli/src/infer_schema_internals/sqlite.rs index f77fe843c591..cbc64e267988 100644 --- a/diesel_cli/src/infer_schema_internals/sqlite.rs +++ b/diesel_cli/src/infer_schema_internals/sqlite.rs @@ -1,6 +1,8 @@ use std::error::Error; +use diesel::deserialize::{self, FromStaticSqlRow, Queryable}; use diesel::dsl::sql; +use diesel::sqlite::Sqlite; use diesel::*; use super::data_structures::*; @@ -151,6 +153,19 @@ pub fn get_table_data( Ok(result) } +impl Queryable for ColumnInformation +where + (i32, String, String, bool, Option, bool, i32): FromStaticSqlRow, +{ + type Row = (i32, String, String, bool, Option, bool, i32); + + fn build(row: Self::Row) -> deserialize::Result { + Ok(ColumnInformation::new( + row.1, row.2, None, !row.3, None, None, + )) + } +} + #[derive(Queryable)] struct FullTableInfo { _cid: i32, @@ -232,6 +247,7 @@ pub fn determine_column_type( is_array: false, is_nullable: attr.nullable, is_unsigned: false, + max_length: attr.max_length, }) } diff --git a/diesel_cli/src/migrations/diff_schema.rs b/diesel_cli/src/migrations/diff_schema.rs index b377890b1a8f..c2945492ab68 100644 --- a/diesel_cli/src/migrations/diff_schema.rs +++ b/diesel_cli/src/migrations/diff_schema.rs @@ -126,7 +126,7 @@ pub fn generate_sql_based_on_diff_schema( for c in columns.column_data { if let Some(def) = expected_column_map.remove(&c.sql_name.to_lowercase()) { - let tpe = ColumnType::from(&def.tpe); + let tpe = ColumnType::for_column_def(&def)?; if !is_same_type(&c.ty, tpe) { changed_columns.push((c, def)); } @@ -242,6 +242,7 @@ fn is_same_type(ty: &ColumnType, tpe: ColumnType) -> bool { if ty.is_array != tpe.is_array || ty.is_nullable != tpe.is_nullable || ty.is_unsigned != tpe.is_unsigned + || ty.max_length != tpe.max_length { return false; } @@ -313,15 +314,16 @@ impl SchemaDiff { .column_defs .iter() .map(|c| { - let ty = ColumnType::from(&c.tpe); - ColumnDefinition { + let ty = ColumnType::for_column_def(c) + .map_err(diesel::result::Error::QueryBuilderError)?; + Ok(ColumnDefinition { sql_name: c.sql_name.to_lowercase(), rust_name: c.sql_name.clone(), ty, comment: None, - } + }) }) - .collect::>(); + .collect::>>()?; let foreign_keys = foreign_keys .iter() .map(|(f, pk)| { @@ -361,7 +363,8 @@ impl SchemaDiff { query_builder, &table.to_lowercase(), &c.column_name.to_string().to_lowercase(), - &ColumnType::from(&c.tpe), + &ColumnType::for_column_def(c) + .map_err(diesel::result::Error::QueryBuilderError)?, )?; query_builder.push_sql("\n"); } @@ -545,6 +548,9 @@ where { // TODO: handle schema query_builder.push_sql(&format!(" {}", ty.sql_name.to_uppercase())); + if let Some(max_length) = ty.max_length { + query_builder.push_sql(&format!("({max_length})")); + } if !ty.is_nullable { query_builder.push_sql(" NOT NULL"); } diff --git a/diesel_cli/src/print_schema.rs b/diesel_cli/src/print_schema.rs index 72f7bc1e7a7d..c1dca8a4f893 100644 --- a/diesel_cli/src/print_schema.rs +++ b/diesel_cli/src/print_schema.rs @@ -696,12 +696,24 @@ impl<'a> Display for ColumnDefinitions<'a> { } } - if column.rust_name == column.sql_name { - writeln!(out, "{} -> {},", column.sql_name, column_type)?; - } else { - writeln!(out, r#"#[sql_name = "{}"]"#, column.sql_name)?; - writeln!(out, "{} -> {},", column.rust_name, column_type)?; + // Write out attributes + if column.rust_name != column.sql_name || column.ty.max_length.is_some() { + let mut is_first = true; + write!(out, r#"#["#)?; + if column.rust_name != column.sql_name { + write!(out, r#"sql_name = {:?}"#, column.sql_name)?; + is_first = false; + } + if let Some(max_length) = column.ty.max_length { + if !is_first { + write!(out, ", ")?; + } + write!(out, "max_length = {}", max_length)?; + } + writeln!(out, r#"]"#)?; } + + writeln!(out, "{} -> {},", column.rust_name, column_type)?; } } writeln!(f, "}}")?; diff --git a/diesel_cli/tests/generate_migrations/diff_add_table_all_the_types/mysql/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_add_table_all_the_types/mysql/schema_out.rs/expected.snap index 2e888b34447b..535aa4253fd6 100644 --- a/diesel_cli/tests/generate_migrations/diff_add_table_all_the_types/mysql/schema_out.rs/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_add_table_all_the_types/mysql/schema_out.rs/expected.snap @@ -1,6 +1,5 @@ --- source: diesel_cli/tests/migration_generate.rs -assertion_line: 354 description: "Test: diff_add_table_all_the_types" --- // @generated automatically by Diesel CLI. @@ -12,6 +11,7 @@ diesel::table! { integer_column -> Integer, small_int_col -> Smallint, big_int_col -> Bigint, + #[max_length = 1] binary_col -> Binary, text_col -> Text, double_col -> Double, @@ -28,6 +28,7 @@ diesel::table! { big_int2_col -> Bigint, float8_col -> Double, decimal_col -> Decimal, + #[max_length = 1] char_col -> Char, tinytext_col -> Tinytext, mediumtext_col -> Mediumtext, diff --git a/diesel_cli/tests/generate_migrations/diff_add_table_all_the_types/postgres/schema_out.rs/expected.snap b/diesel_cli/tests/generate_migrations/diff_add_table_all_the_types/postgres/schema_out.rs/expected.snap index 549367029c3d..256bc5c28f57 100644 --- a/diesel_cli/tests/generate_migrations/diff_add_table_all_the_types/postgres/schema_out.rs/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_add_table_all_the_types/postgres/schema_out.rs/expected.snap @@ -1,6 +1,5 @@ --- source: diesel_cli/tests/migration_generate.rs -assertion_line: 354 description: "Test: diff_add_table_all_the_types" --- // @generated automatically by Diesel CLI. @@ -28,7 +27,9 @@ diesel::table! { decimal_col -> Numeric, varchar_col -> Varchar, varchar2_col -> Varchar, + #[max_length = 1] char_col -> Bpchar, + #[max_length = 1] bit_col -> Bit, cidr_col -> Cidr, inet_col -> Inet, diff --git a/diesel_cli/tests/generate_migrations/diff_drop_table_all_the_types/mysql/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_drop_table_all_the_types/mysql/down.sql/expected.snap index 369d2e5c1b48..c2d617bf1cdd 100644 --- a/diesel_cli/tests/generate_migrations/diff_drop_table_all_the_types/mysql/down.sql/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_drop_table_all_the_types/mysql/down.sql/expected.snap @@ -1,6 +1,5 @@ --- source: diesel_cli/tests/migration_generate.rs -assertion_line: 338 description: "Test: diff_drop_table_all_the_types" --- -- This file should undo anything in `up.sql` @@ -19,7 +18,7 @@ CREATE TABLE `test`( `datetime` DATETIME, `timestamp` TIMESTAMP NOT NULL, `time` TIME, - `char` CHAR, + `char` CHAR(50), `blob` BLOB, `text` TEXT ); diff --git a/diesel_cli/tests/generate_migrations/diff_drop_table_all_the_types/postgres/down.sql/expected.snap b/diesel_cli/tests/generate_migrations/diff_drop_table_all_the_types/postgres/down.sql/expected.snap index d4af36ca206e..24a46f5d935a 100644 --- a/diesel_cli/tests/generate_migrations/diff_drop_table_all_the_types/postgres/down.sql/expected.snap +++ b/diesel_cli/tests/generate_migrations/diff_drop_table_all_the_types/postgres/down.sql/expected.snap @@ -1,6 +1,5 @@ --- source: diesel_cli/tests/migration_generate.rs -assertion_line: 319 description: "Test: diff_drop_table_all_the_types" --- -- This file should undo anything in `up.sql` @@ -12,10 +11,10 @@ CREATE TABLE "test"( "bigserial2" INT8 NOT NULL, "boolean" BOOL, "bytea" BYTEA, - "character" BPCHAR, - "char" BPCHAR, - "varchar" VARCHAR, - "varchar2" VARCHAR, + "character" BPCHAR(5), + "char" BPCHAR(5), + "varchar" VARCHAR(5), + "varchar2" VARCHAR(5), "cidr" CIDR, "date" DATE, "double" FLOAT8, diff --git a/diesel_cli/tests/print_schema/print_schema_column_order/mysql/expected.snap b/diesel_cli/tests/print_schema/print_schema_column_order/mysql/expected.snap index 5d7de645d117..23127a80cb6c 100644 --- a/diesel_cli/tests/print_schema/print_schema_column_order/mysql/expected.snap +++ b/diesel_cli/tests/print_schema/print_schema_column_order/mysql/expected.snap @@ -7,6 +7,7 @@ description: "Test: print_schema_column_order" diesel::table! { abc (a) { a -> Integer, + #[max_length = 16] b -> Varchar, c -> Nullable, } diff --git a/diesel_cli/tests/print_schema/print_schema_several_keys_with_compound_key/mysql/expected.snap b/diesel_cli/tests/print_schema/print_schema_several_keys_with_compound_key/mysql/expected.snap index ed9f20fc399e..5099596c0383 100644 --- a/diesel_cli/tests/print_schema/print_schema_several_keys_with_compound_key/mysql/expected.snap +++ b/diesel_cli/tests/print_schema/print_schema_several_keys_with_compound_key/mysql/expected.snap @@ -7,6 +7,7 @@ description: "Test: print_schema_several_keys_with_compound_key" diesel::table! { payment_card (id) { id -> Integer, + #[max_length = 50] code -> Varchar, } } @@ -14,6 +15,7 @@ diesel::table! { diesel::table! { transaction_one (id) { id -> Integer, + #[max_length = 50] card_code -> Varchar, payment_card_id -> Integer, by_card_id -> Integer, @@ -24,6 +26,7 @@ diesel::table! { transaction_two (id) { id -> Integer, payment_card_id -> Integer, + #[max_length = 50] card_code -> Varchar, } } diff --git a/diesel_cli/tests/print_schema/print_schema_with_enum_set_types/mysql/expected.snap b/diesel_cli/tests/print_schema/print_schema_with_enum_set_types/mysql/expected.snap index 9138e4b0e558..cfa188fcfc1b 100644 --- a/diesel_cli/tests/print_schema/print_schema_with_enum_set_types/mysql/expected.snap +++ b/diesel_cli/tests/print_schema/print_schema_with_enum_set_types/mysql/expected.snap @@ -45,12 +45,14 @@ diesel::table! { /// Its SQL type is `Users1UserStateEnum`. /// /// (Automatically generated by Diesel.) + #[max_length = 8] user_state -> Users1UserStateEnum, /// The `enabled_features` column of the `users1` table. /// /// Its SQL type is `Users1EnabledFeaturesSet`. /// /// (Automatically generated by Diesel.) + #[max_length = 19] enabled_features -> Users1EnabledFeaturesSet, } } diff --git a/diesel_derives/src/table.rs b/diesel_derives/src/table.rs index a32e0d4413df..377004bc99b5 100644 --- a/diesel_derives/src/table.rs +++ b/diesel_derives/src/table.rs @@ -663,6 +663,14 @@ fn expand_column_def(column_def: &ColumnDef) -> TokenStream { None }; + let max_length = column_def.max_length.as_ref().map(|column_max_length| { + quote::quote! { + impl self::diesel::query_source::SizeRestrictedColumn for #column_name { + const MAX_LENGTH: usize = #column_max_length; + } + } + }); + quote::quote_spanned! {span=> #(#meta)* #[allow(non_camel_case_types, dead_code)] @@ -768,6 +776,8 @@ fn expand_column_def(column_def: &ColumnDef) -> TokenStream { } } + #max_length + #ops_impls #backend_specific_column_impl } diff --git a/diesel_table_macro_syntax/src/lib.rs b/diesel_table_macro_syntax/src/lib.rs index f32084a0a933..c1f8cf2b6869 100644 --- a/diesel_table_macro_syntax/src/lib.rs +++ b/diesel_table_macro_syntax/src/lib.rs @@ -2,59 +2,31 @@ use syn::spanned::Spanned; use syn::Ident; use syn::MetaNameValue; -#[allow(dead_code)] // paren_token is currently unused -pub struct PrimaryKey { - paren_token: syn::token::Paren, - pub keys: syn::punctuated::Punctuated, -} - -#[allow(dead_code)] // arrow is currently unused -pub struct ColumnDef { - pub meta: Vec, - pub column_name: Ident, - pub sql_name: String, - arrow: syn::Token![->], - pub tpe: syn::TypePath, -} - -#[allow(dead_code)] // punct and brace_token is currently unused pub struct TableDecl { pub use_statements: Vec, pub meta: Vec, pub schema: Option, - punct: Option, + _punct: Option, pub sql_name: String, pub table_name: Ident, pub primary_keys: Option, - brace_token: syn::token::Brace, + _brace_token: syn::token::Brace, pub column_defs: syn::punctuated::Punctuated, } -#[allow(dead_code)] // eq is currently unused -struct SqlNameAttribute { - eq: syn::Token![=], - lit: syn::LitStr, +#[allow(dead_code)] // paren_token is currently unused +pub struct PrimaryKey { + paren_token: syn::token::Paren, + pub keys: syn::punctuated::Punctuated, } -impl SqlNameAttribute { - fn from_attribute(element: syn::Attribute) -> Result { - if let syn::Meta::NameValue(MetaNameValue { - eq_token, - value: - syn::Expr::Lit(syn::ExprLit { - lit: syn::Lit::Str(lit), - .. - }), - .. - }) = element.meta - { - Ok(SqlNameAttribute { eq: eq_token, lit }) - } else { - Err(syn::Error::new( - element.span(), - "Invalid `#[sql_name = \"column_name\"]` attribute", - )) - } - } + +pub struct ColumnDef { + pub meta: Vec, + pub column_name: Ident, + pub sql_name: String, + _arrow: syn::Token![->], + pub tpe: syn::TypePath, + pub max_length: Option, } impl syn::parse::Parse for TableDecl { @@ -68,7 +40,7 @@ impl syn::parse::Parse for TableDecl { break; }; } - let meta = syn::Attribute::parse_outer(buf)?; + let mut meta = syn::Attribute::parse_outer(buf)?; let fork = buf.fork(); let (schema, punct, table_name) = if parse_table_with_schema(&fork).is_ok() { let (schema, punct, table_name) = parse_table_with_schema(buf)?; @@ -86,16 +58,16 @@ impl syn::parse::Parse for TableDecl { let content; let brace_token = syn::braced!(content in buf); let column_defs = syn::punctuated::Punctuated::parse_terminated(&content)?; - let (sql_name, meta) = get_sql_name(meta, &table_name)?; + let sql_name = get_sql_name(&mut meta, &table_name)?; Ok(Self { use_statements, meta, table_name, primary_keys, - brace_token, + _brace_token: brace_token, column_defs, sql_name, - punct, + _punct: punct, schema, }) } @@ -112,29 +84,28 @@ impl syn::parse::Parse for PrimaryKey { impl syn::parse::Parse for ColumnDef { fn parse(input: syn::parse::ParseStream) -> syn::Result { - let meta = syn::Attribute::parse_outer(input)?; - let column_name = input.parse()?; - let arrow = input.parse()?; - let tpe = input.parse()?; - let (sql_name, meta) = get_sql_name(meta, &column_name)?; + let mut meta = syn::Attribute::parse_outer(input)?; + let column_name: syn::Ident = input.parse()?; + let _arrow: syn::Token![->] = input.parse()?; + let tpe: syn::TypePath = input.parse()?; + + let sql_name = get_sql_name(&mut meta, &column_name)?; + let max_length = take_lit(&mut meta, "max_length", |lit| match lit { + syn::Lit::Int(lit_int) => Some(lit_int), + _ => None, + })?; + Ok(Self { meta, column_name, - arrow, + _arrow, tpe, + max_length, sql_name, }) } } -impl syn::parse::Parse for SqlNameAttribute { - fn parse(input: syn::parse::ParseStream) -> syn::Result { - let eq = input.parse()?; - let lit = input.parse()?; - Ok(Self { eq, lit }) - } -} - pub fn parse_table_with_schema( input: &syn::parse::ParseBuffer<'_>, ) -> Result<(syn::Ident, syn::Token![.], syn::Ident), syn::Error> { @@ -142,19 +113,51 @@ pub fn parse_table_with_schema( } fn get_sql_name( - mut meta: Vec, - ident: &syn::Ident, -) -> Result<(String, Vec), syn::Error> { - if let Some(pos) = meta.iter().position(|m| { + meta: &mut Vec, + fallback_ident: &syn::Ident, +) -> Result { + Ok( + match take_lit(meta, "sql_name", |lit| match lit { + syn::Lit::Str(lit_str) => Some(lit_str), + _ => None, + })? { + None => fallback_ident.to_string(), + Some(str_lit) => str_lit.value(), + }, + ) +} + +fn take_lit( + meta: &mut Vec, + attribute_name: &'static str, + extraction_fn: F, +) -> Result, syn::Error> +where + F: FnOnce(syn::Lit) -> Option, +{ + if let Some(index) = meta.iter().position(|m| { m.path() .get_ident() - .map(|i| i == "sql_name") + .map(|i| i == attribute_name) .unwrap_or(false) }) { - let element = meta.remove(pos); - let inner = SqlNameAttribute::from_attribute(element)?; - Ok((inner.lit.value(), meta)) - } else { - Ok((ident.to_string(), meta)) + let attribute = meta.remove(index); + let span = attribute.span(); + let extraction_after_finding_attr = if let syn::Meta::NameValue(MetaNameValue { + value: syn::Expr::Lit(syn::ExprLit { lit, .. }), + .. + }) = attribute.meta + { + extraction_fn(lit) + } else { + None + }; + return Ok(Some(extraction_after_finding_attr.ok_or_else(|| { + syn::Error::new( + span, + format_args!("Invalid `#[sql_name = {attribute_name:?}]` attribute"), + ) + })?)); } + Ok(None) } diff --git a/diesel_table_macro_syntax/tests/basic.rs b/diesel_table_macro_syntax/tests/basic.rs new file mode 100644 index 000000000000..b1b4e2929533 --- /dev/null +++ b/diesel_table_macro_syntax/tests/basic.rs @@ -0,0 +1,18 @@ +use diesel_table_macro_syntax::*; + +#[test] +fn basic() { + let input = include_str!("basic.rs.in"); + let t: TableDecl = syn::parse_str(input).unwrap(); + assert_eq!(t.column_defs.len(), 3); + assert_eq!( + t.column_defs + .iter() + .map(|c| c + .max_length + .as_ref() + .map(|n| n.base10_parse::().unwrap())) + .collect::>(), + &[None, Some(120), Some(120)] + ) +} diff --git a/diesel_table_macro_syntax/tests/basic.rs.in b/diesel_table_macro_syntax/tests/basic.rs.in new file mode 100644 index 000000000000..5637c71e021d --- /dev/null +++ b/diesel_table_macro_syntax/tests/basic.rs.in @@ -0,0 +1,7 @@ +t1 (id) { + f0 -> Varchar, + #[max_length = 120] + f1 -> Varchar, + #[max_length = 120] + f2 -> Nullable, +} From d31630a0168a5044ab9ff7311df98233f6075583 Mon Sep 17 00:00:00 2001 From: Omid Rad Date: Wed, 24 May 2023 19:06:03 +0200 Subject: [PATCH 7/9] implement missing cases --- diesel/Cargo.toml | 1 - diesel/src/pg/query_builder/distinct_on.rs | 15 +++- diesel_tests/tests/distinct.rs | 83 +++++++++++++++++----- 3 files changed, 80 insertions(+), 19 deletions(-) diff --git a/diesel/Cargo.toml b/diesel/Cargo.toml index 1c9b1309d221..61389bec0810 100644 --- a/diesel/Cargo.toml +++ b/diesel/Cargo.toml @@ -34,7 +34,6 @@ bitflags = { version = "2.0.0", optional = true } r2d2 = { version = ">= 0.8.2, < 0.9.0", optional = true } itoa = { version = "1.0.0", optional = true } time = { version = "0.3.9", optional = true, features = ["macros"] } -paste = "1.0.12" [dependencies.diesel_derives] version = "~2.0.0" diff --git a/diesel/src/pg/query_builder/distinct_on.rs b/diesel/src/pg/query_builder/distinct_on.rs index 68703c9df160..c9a119a4749e 100644 --- a/diesel/src/pg/query_builder/distinct_on.rs +++ b/diesel/src/pg/query_builder/distinct_on.rs @@ -18,13 +18,24 @@ pub struct DistinctOnClause(pub(crate) T); impl ValidOrderingForDistinct> for NoOrderClause {} impl ValidOrderingForDistinct> for OrderClause<(T,)> {} impl ValidOrderingForDistinct> for OrderClause where T: crate::Column {} - +impl ValidOrderingForDistinct> for OrderClause> + where + T: crate::Column, +{} +impl ValidOrderingForDistinct> for OrderClause> + where + T: crate::Column, +{} macro_rules! valid_ordering { (@skip: ($ST1: ident, $($ST:ident,)*), $T1:ident, ) => {}; (@skip: ($ST1: ident, $($ST:ident,)*), $T1:ident, $($T:ident,)+) => { valid_ordering!(($($ST,)*), ($ST1,), $($T,)*); }; (($ST1: ident,), ($($OT:ident,)*), $T1:ident,) => { + #[allow(unused_parens)] + impl<$T1, $ST1, $($OT,)*> ValidOrderingForDistinct> for OrderClause<($T1)> + where $T1: crate::pg::OrderDecorator, + {} impl<$T1, $ST1, $($OT,)*> ValidOrderingForDistinct> for OrderClause<($T1,)> where $T1: crate::pg::OrderDecorator, {} @@ -44,7 +55,7 @@ macro_rules! valid_ordering { {} impl<$T1, $($T,)* $ST1, $($ST,)* $($OT,)*> ValidOrderingForDistinct> for OrderClause<($ST1, $($ST,)* $($OT,)*)> where $ST1: crate::pg::OrderDecorator, - $($ST: crate::pg::OrderDecorator,)* + $($ST: crate::pg::OrderDecorator,)* {} valid_ordering!(($($ST,)*), ($($OT,)* $ST1,), $($T,)*); }; diff --git a/diesel_tests/tests/distinct.rs b/diesel_tests/tests/distinct.rs index 4797e0fed4fa..f3908aa30212 100644 --- a/diesel_tests/tests/distinct.rs +++ b/diesel_tests/tests/distinct.rs @@ -168,6 +168,52 @@ fn distinct_of_multiple_columns() { .load::(&mut connection) .unwrap(); + // one order by + // one distinct on + let data = posts::table + .order(posts::body) + .distinct_on(posts::body) + .load(&mut connection); + let expected = vec![ + (posts[0].clone()), + (posts[7].clone()), + ]; + + assert_eq!(Ok(expected), data); + + // multi order by + // one distinct on + let data = posts::table + .inner_join(users::table) + .order((users::id, posts::body, posts::title)) + .distinct_on(users::id) + .load(&mut connection); + let expected = vec![ + ((posts[0].clone(), sean.clone())), + ((posts[4].clone(), tess.clone())), + ]; + + assert_eq!(Ok(expected), data); + + // one order by + // multi distinct on + let data = posts::table + .inner_join(users::table) + .order(users::id) + .distinct_on((users::id, posts::body)) + .load(&mut connection); + let expected = vec![ + ((posts[0].clone(), sean.clone())), + ((posts[1].clone(), sean.clone())), + ((posts[4].clone(), tess.clone())), + ((posts[7].clone(), tess.clone())), + ]; + + assert_eq!(Ok(expected), data); + + // multi order by + // multi distinct on + // order by > distinct on let data = posts::table .inner_join(users::table) .order((users::id, posts::body, posts::title)) @@ -181,20 +227,25 @@ fn distinct_of_multiple_columns() { ]; assert_eq!(Ok(expected), data); -} -// #[cfg(feature = "postgres")] -// #[test] -// #[should_panic(expected = "TBD")] -// fn invalid_distinct_on_or_order_detected() { -// use crate::schema::users; -// use crate::schema::posts; -// -// let mut connection = connection(); -// -// posts::table -// .inner_join(users::table) -// .order((users::id, posts::title)) -// .distinct_on((users::id, posts::body)) -// .load(&mut connection); -// } + // multi order by + // multi distinct on + // order by < distinct on + let data = posts::table + .inner_join(users::table) + .order((users::id, posts::body)) + .distinct_on((users::id, posts::body, posts::title)) + .load(&mut connection); + let expected = vec![ + ((posts[0].clone(), sean.clone())), + ((posts[2].clone(), sean.clone())), + ((posts[1].clone(), sean.clone())), + ((posts[3].clone(), sean.clone())), + ((posts[4].clone(), tess.clone())), + ((posts[6].clone(), tess.clone())), + ((posts[5].clone(), tess.clone())), + ((posts[7].clone(), tess.clone())), + ]; + + assert_eq!(Ok(expected), data); +} From 1de31ad4e16e4bb52b238d04c541fe299988be89 Mon Sep 17 00:00:00 2001 From: Omid Rad Date: Wed, 24 May 2023 23:40:05 +0200 Subject: [PATCH 8/9] Add compile tests and fix CI issues --- diesel/src/pg/query_builder/distinct_on.rs | 4 +- ...inct_on_allows_only_fields_of_table.stderr | 25 -- ...tinct_on_requires_matching_order_clause.rs | 37 +++ ...t_on_requires_matching_order_clause.stderr | 275 ++++++++---------- diesel_tests/tests/distinct.rs | 38 +-- 5 files changed, 187 insertions(+), 192 deletions(-) diff --git a/diesel/src/pg/query_builder/distinct_on.rs b/diesel/src/pg/query_builder/distinct_on.rs index c9a119a4749e..0ac2856ce223 100644 --- a/diesel/src/pg/query_builder/distinct_on.rs +++ b/diesel/src/pg/query_builder/distinct_on.rs @@ -74,12 +74,12 @@ macro_rules! valid_ordering { } // we only generate these impl up to a tuple size of 5 as we generate n*n + 4 impls here -// If we would generate these impls up to max_table_column_count tuple elements that +// If we would generate these impls up to max_table_column_count tuple elements that // would be a really large number for 128 tuple elements (~64k trait impls) // It's fine to increase this number at some point in the future gradually diesel_derives::__diesel_for_each_tuple!(valid_ordering, 5); -/// A decorator trait for [`OrderClause`] +/// A decorator trait for `OrderClause` /// It helps to have bounds on either Col, Asc and Desc. pub trait OrderDecorator { /// A column on a database table. diff --git a/diesel_compile_tests/tests/fail/distinct_on_allows_only_fields_of_table.stderr b/diesel_compile_tests/tests/fail/distinct_on_allows_only_fields_of_table.stderr index 8b5e11fb93f4..9c94b7aaf2f1 100644 --- a/diesel_compile_tests/tests/fail/distinct_on_allows_only_fields_of_table.stderr +++ b/diesel_compile_tests/tests/fail/distinct_on_allows_only_fields_of_table.stderr @@ -21,31 +21,6 @@ note: required by a bound in `diesel::QueryDsl::distinct_on` | Self: methods::DistinctOnDsl, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `QueryDsl::distinct_on` -error[E0277]: the trait bound `(diesel::sql_types::Text, diesel::sql_types::Text): SingleValue` is not satisfied - --> tests/fail/distinct_on_allows_only_fields_of_table.rs:25:30 - | -25 | posts::table.distinct_on((posts::name, users::name)).get_result(&mut connection); - | ----------- ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `SingleValue` is not implemented for `(diesel::sql_types::Text, diesel::sql_types::Text)` - | | - | required by a bound introduced by this call - | - = help: the following other types implement trait `SingleValue`: - Array - BigInt - Bool - CChar - Cidr - Datetime - Inet - Interval - and $N others - = note: required for `posts::table` to implement `DistinctOnDsl<(posts::columns::name, users::columns::name)>` -note: required by a bound in `diesel::QueryDsl::distinct_on` - --> $DIESEL/src/query_dsl/mod.rs - | - | Self: methods::DistinctOnDsl, - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `QueryDsl::distinct_on` - error[E0277]: Cannot select `users::columns::name` from `posts::table` --> tests/fail/distinct_on_allows_only_fields_of_table.rs:25:18 | diff --git a/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs b/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs index 828a7228021e..a1374f4e4206 100644 --- a/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs +++ b/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs @@ -21,6 +21,15 @@ fn main() { // verify that we could use distinct on with an order clause that contains also a different column let _ = users::table.order_by((users::name, users::id)).distinct_on(users::name); + // verify that we could use multiple columns for both order by and distinct on + let _ = users::table.order_by((users::name, users::id)).distinct_on((users::name, users::id)); + + // verify that we could use multiple columns for both order by and distinct on and distinct on has more columns than order by + let _ = users::table.order_by((users::name, users::id)).distinct_on((users::name, users::id, users::hair_color)); + + // verify that we could use multiple columns for both order by and distinct on and distinct on has less columns than order by + let _ = users::table.order_by((users::name, users::id, users::hair_color)).distinct_on((users::name, users::id)); + // verify that we could use distinct on with a select expression and an order clause that contains a different column let _ = users::table .distinct_on(users::id) @@ -43,6 +52,18 @@ fn main() { .order_by((users::name, users::id)) .distinct_on(users::name) .into_boxed(); + let _ = users::table + .order_by((users::name, users::id)) + .distinct_on((users::name, users::id)) + .into_boxed(); + let _ = users::table + .order_by((users::name, users::id)) + .distinct_on((users::name, users::id, users::hair_color)) + .into_boxed(); + let _ = users::table + .order_by((users::name, users::id, users::hair_color)) + .distinct_on((users::name, users::id)) + .into_boxed(); let _ = users::table .order_by(users::name) .then_order_by(users::id) @@ -71,11 +92,27 @@ fn main() { // we cannot box invalid queries let _ = users::table.order_by(users::id).distinct_on(users::name).into_boxed(); + // it's not possible to set an invalid order clause after we set + // for multiple order by and one distinct on let _ = users::table .order_by((users::id, users::name)) .distinct_on(users::name) .into_boxed(); + // it's not possible to set an invalid order clause after we set + // for multiple order by and distinct on + let _ = users::table + .order_by((users::id, users::name)) + .distinct_on((users::name, users::id)) + .into_boxed(); + + // it's not possible to set an invalid order clause after we set + // for one order by and multiple distinct on + let _ = users::table + .order_by(users::id) + .distinct_on((users::name, users::id)) + .into_boxed(); + // we cannot workaround that with `then_order_by` let _ = users::table .order_by(users::id) diff --git a/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.stderr b/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.stderr index 7aeaeab35d5e..d79dbf2a3756 100644 --- a/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.stderr +++ b/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.stderr @@ -1,87 +1,63 @@ -error[E0308]: mismatched types - --> tests/fail/distinct_on_requires_matching_order_clause.rs:55:58 +error[E0277]: the trait bound `diesel::query_builder::order_clause::OrderClause: query_dsl::order_dsl::ValidOrderingForDistinct>` is not satisfied + --> tests/fail/distinct_on_requires_matching_order_clause.rs:76:58 | -55 | let _ = users::table.order_by(users::id).distinct_on(users::name); - | ----------- ^^^^^^^^^^^ expected struct `id`, found struct `name` +76 | let _ = users::table.order_by(users::id).distinct_on(users::name); + | ----------- ^^^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct>` is not implemented for `diesel::query_builder::order_clause::OrderClause` | | - | arguments to this method are incorrect + | required by a bound introduced by this call | -help: the return type of this call is `columns::name` due to the type of the argument passed - --> tests/fail/distinct_on_requires_matching_order_clause.rs:55:13 - | -55 | let _ = users::table.order_by(users::id).distinct_on(users::name); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-----------^ - | | - | this argument influences the return type of `distinct_on` -note: associated function defined here - --> $DIESEL/src/query_dsl/mod.rs - | - | fn distinct_on(self, expr: Expr) -> DistinctOn - | ^^^^^^^^^^^ + = help: the following other types implement trait `query_dsl::order_dsl::ValidOrderingForDistinct`: + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + and $N others + = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` +note: required by a bound in `diesel::QueryDsl::distinct_on` + --> $DIESEL/src/query_dsl/mod.rs + | + | Self: methods::DistinctOnDsl, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `QueryDsl::distinct_on` -error[E0308]: mismatched types - --> tests/fail/distinct_on_requires_matching_order_clause.rs:59:73 +error[E0271]: type mismatch resolving `::Column == name` + --> tests/fail/distinct_on_requires_matching_order_clause.rs:80:61 | -59 | let _ = users::table.order_by((users::id, users::name)).distinct_on(users::name); - | ----------- ^^^^^^^^^^^ expected struct `id`, found struct `name` - | | - | arguments to this method are incorrect +80 | let _ = users::table.order_by((users::id, users::name)).distinct_on(users::name); + | ^^^^^^^^^^^ expected struct `id`, found struct `name` | -help: the return type of this call is `columns::name` due to the type of the argument passed - --> tests/fail/distinct_on_requires_matching_order_clause.rs:59:13 - | -59 | let _ = users::table.order_by((users::id, users::name)).distinct_on(users::name); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-----------^ - | | - | this argument influences the return type of `distinct_on` -note: associated function defined here - --> $DIESEL/src/query_dsl/mod.rs - | - | fn distinct_on(self, expr: Expr) -> DistinctOn - | ^^^^^^^^^^^ + = note: required for `diesel::query_builder::order_clause::OrderClause<(columns::id, columns::name)>` to implement `query_dsl::order_dsl::ValidOrderingForDistinct>` + = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` -error[E0308]: mismatched types - --> tests/fail/distinct_on_requires_matching_order_clause.rs:65:22 +error[E0271]: type mismatch resolving `::Column == name` + --> tests/fail/distinct_on_requires_matching_order_clause.rs:86:10 | -65 | .distinct_on(users::name); - | ----------- ^^^^^^^^^^^ expected struct `id`, found struct `name` - | | - | arguments to this method are incorrect +86 | .distinct_on(users::name); + | ^^^^^^^^^^^ expected struct `id`, found struct `name` | -help: the return type of this call is `columns::name` due to the type of the argument passed - --> tests/fail/distinct_on_requires_matching_order_clause.rs:62:13 - | -62 | let _ = users::table - | _____________^ -63 | | .order_by(users::id) -64 | | .then_order_by(users::name) -65 | | .distinct_on(users::name); - | |______________________-----------^ - | | - | this argument influences the return type of `distinct_on` -note: associated function defined here - --> $DIESEL/src/query_dsl/mod.rs - | - | fn distinct_on(self, expr: Expr) -> DistinctOn - | ^^^^^^^^^^^ + = note: required for `diesel::query_builder::order_clause::OrderClause<(columns::id, columns::name)>` to implement `query_dsl::order_dsl::ValidOrderingForDistinct>` + = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` error[E0277]: the trait bound `diesel::query_builder::order_clause::OrderClause: query_dsl::order_dsl::ValidOrderingForDistinct>` is not satisfied - --> tests/fail/distinct_on_requires_matching_order_clause.rs:69:60 + --> tests/fail/distinct_on_requires_matching_order_clause.rs:90:60 | -69 | let _ = users::table.distinct_on(users::name).order_by(users::id); +90 | let _ = users::table.distinct_on(users::name).order_by(users::id); | -------- ^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct>` is not implemented for `diesel::query_builder::order_clause::OrderClause` | | | required by a bound introduced by this call | = help: the following other types implement trait `query_dsl::order_dsl::ValidOrderingForDistinct`: - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> and $N others = note: required for `SelectStatement, DefaultSelectClause>, DistinctOnClause>` to implement `OrderDsl` note: required by a bound in `order_by` @@ -90,97 +66,104 @@ note: required by a bound in `order_by` | Self: methods::OrderDsl, | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `QueryDsl::order_by` -error[E0308]: mismatched types - --> tests/fail/distinct_on_requires_matching_order_clause.rs:72:58 +error[E0277]: the trait bound `diesel::query_builder::order_clause::OrderClause: query_dsl::order_dsl::ValidOrderingForDistinct>` is not satisfied + --> tests/fail/distinct_on_requires_matching_order_clause.rs:93:58 | -72 | let _ = users::table.order_by(users::id).distinct_on(users::name).into_boxed(); - | ----------- ^^^^^^^^^^^ expected struct `id`, found struct `name` +93 | let _ = users::table.order_by(users::id).distinct_on(users::name).into_boxed(); + | ----------- ^^^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct>` is not implemented for `diesel::query_builder::order_clause::OrderClause` | | - | arguments to this method are incorrect + | required by a bound introduced by this call | -help: the return type of this call is `columns::name` due to the type of the argument passed - --> tests/fail/distinct_on_requires_matching_order_clause.rs:72:13 - | -72 | let _ = users::table.order_by(users::id).distinct_on(users::name).into_boxed(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^-----------^ - | | - | this argument influences the return type of `distinct_on` -note: associated function defined here - --> $DIESEL/src/query_dsl/mod.rs - | - | fn distinct_on(self, expr: Expr) -> DistinctOn - | ^^^^^^^^^^^ + = help: the following other types implement trait `query_dsl::order_dsl::ValidOrderingForDistinct`: + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + and $N others + = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` +note: required by a bound in `diesel::QueryDsl::distinct_on` + --> $DIESEL/src/query_dsl/mod.rs + | + | Self: methods::DistinctOnDsl, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `QueryDsl::distinct_on` -error[E0308]: mismatched types - --> tests/fail/distinct_on_requires_matching_order_clause.rs:76:22 +error[E0271]: type mismatch resolving `::Column == name` + --> tests/fail/distinct_on_requires_matching_order_clause.rs:99:10 | -76 | .distinct_on(users::name) - | ----------- ^^^^^^^^^^^ expected struct `id`, found struct `name` - | | - | arguments to this method are incorrect +99 | .distinct_on(users::name) + | ^^^^^^^^^^^ expected struct `id`, found struct `name` | -help: the return type of this call is `columns::name` due to the type of the argument passed - --> tests/fail/distinct_on_requires_matching_order_clause.rs:74:13 + = note: required for `diesel::query_builder::order_clause::OrderClause<(columns::id, columns::name)>` to implement `query_dsl::order_dsl::ValidOrderingForDistinct>` + = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` + +error[E0277]: the trait bound `diesel::query_builder::order_clause::OrderClause<(columns::id, columns::name)>: query_dsl::order_dsl::ValidOrderingForDistinct>` is not satisfied + --> tests/fail/distinct_on_requires_matching_order_clause.rs:106:22 + | +106 | .distinct_on((users::name, users::id)) + | ----------- ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct>` is not implemented for `diesel::query_builder::order_clause::OrderClause<(columns::id, columns::name)>` + | | + | required by a bound introduced by this call | -74 | let _ = users::table - | _____________^ -75 | | .order_by((users::id, users::name)) -76 | | .distinct_on(users::name) - | |______________________-----------^ - | | - | this argument influences the return type of `distinct_on` -note: associated function defined here + = help: the following other types implement trait `query_dsl::order_dsl::ValidOrderingForDistinct`: + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + and $N others + = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl<(columns::name, columns::id)>` +note: required by a bound in `diesel::QueryDsl::distinct_on` --> $DIESEL/src/query_dsl/mod.rs | - | fn distinct_on(self, expr: Expr) -> DistinctOn - | ^^^^^^^^^^^ + | Self: methods::DistinctOnDsl, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `QueryDsl::distinct_on` -error[E0308]: mismatched types - --> tests/fail/distinct_on_requires_matching_order_clause.rs:83:22 - | -83 | .distinct_on(users::name) - | ----------- ^^^^^^^^^^^ expected struct `id`, found struct `name` - | | - | arguments to this method are incorrect - | -help: the return type of this call is `columns::name` due to the type of the argument passed - --> tests/fail/distinct_on_requires_matching_order_clause.rs:80:13 +error[E0271]: type mismatch resolving `::Column == name` + --> tests/fail/distinct_on_requires_matching_order_clause.rs:113:10 | -80 | let _ = users::table - | _____________^ -81 | | .order_by(users::id) -82 | | .then_order_by(users::name) -83 | | .distinct_on(users::name) - | |______________________-----------^ - | | - | this argument influences the return type of `distinct_on` -note: associated function defined here - --> $DIESEL/src/query_dsl/mod.rs +113 | .distinct_on((users::name, users::id)) + | ^^^^^^^^^^^ expected struct `id`, found struct `name` | - | fn distinct_on(self, expr: Expr) -> DistinctOn - | ^^^^^^^^^^^ + = note: required for `diesel::query_builder::order_clause::OrderClause` to implement `query_dsl::order_dsl::ValidOrderingForDistinct>` + = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl<(columns::name, columns::id)>` + +error[E0271]: type mismatch resolving `::Column == name` + --> tests/fail/distinct_on_requires_matching_order_clause.rs:120:10 + | +120 | .distinct_on(users::name) + | ^^^^^^^^^^^ expected struct `id`, found struct `name` + | + = note: required for `diesel::query_builder::order_clause::OrderClause<(columns::id, columns::name)>` to implement `query_dsl::order_dsl::ValidOrderingForDistinct>` + = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` error[E0277]: the trait bound `diesel::query_builder::order_clause::OrderClause: query_dsl::order_dsl::ValidOrderingForDistinct>` is not satisfied - --> tests/fail/distinct_on_requires_matching_order_clause.rs:90:19 - | -90 | .order_by(users::id) - | -------- ^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct>` is not implemented for `diesel::query_builder::order_clause::OrderClause` - | | - | required by a bound introduced by this call - | - = help: the following other types implement trait `query_dsl::order_dsl::ValidOrderingForDistinct`: - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - and $N others - = note: required for `SelectStatement, DefaultSelectClause>, DistinctOnClause>` to implement `OrderDsl` + --> tests/fail/distinct_on_requires_matching_order_clause.rs:127:19 + | +127 | .order_by(users::id) + | -------- ^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct>` is not implemented for `diesel::query_builder::order_clause::OrderClause` + | | + | required by a bound introduced by this call + | + = help: the following other types implement trait `query_dsl::order_dsl::ValidOrderingForDistinct`: + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + and $N others + = note: required for `SelectStatement, DefaultSelectClause>, DistinctOnClause>` to implement `OrderDsl` note: required by a bound in `order_by` - --> $DIESEL/src/query_dsl/mod.rs - | - | Self: methods::OrderDsl, - | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `QueryDsl::order_by` + --> $DIESEL/src/query_dsl/mod.rs + | + | Self: methods::OrderDsl, + | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `QueryDsl::order_by` diff --git a/diesel_tests/tests/distinct.rs b/diesel_tests/tests/distinct.rs index f3908aa30212..3682f5f34dfb 100644 --- a/diesel_tests/tests/distinct.rs +++ b/diesel_tests/tests/distinct.rs @@ -189,8 +189,8 @@ fn distinct_of_multiple_columns() { .distinct_on(users::id) .load(&mut connection); let expected = vec![ - ((posts[0].clone(), sean.clone())), - ((posts[4].clone(), tess.clone())), + (posts[0].clone(), sean.clone()), + (posts[4].clone(), tess.clone()), ]; assert_eq!(Ok(expected), data); @@ -203,12 +203,12 @@ fn distinct_of_multiple_columns() { .distinct_on((users::id, posts::body)) .load(&mut connection); let expected = vec![ - ((posts[0].clone(), sean.clone())), - ((posts[1].clone(), sean.clone())), - ((posts[4].clone(), tess.clone())), - ((posts[7].clone(), tess.clone())), + (posts[0].clone(), sean.clone()), + (posts[1].clone(), sean.clone()), + (posts[4].clone(), tess.clone()), + (posts[7].clone(), tess.clone()), ]; - + assert_eq!(Ok(expected), data); // multi order by @@ -220,10 +220,10 @@ fn distinct_of_multiple_columns() { .distinct_on((users::id, posts::body)) .load(&mut connection); let expected = vec![ - ((posts[0].clone(), sean.clone())), - ((posts[1].clone(), sean.clone())), - ((posts[4].clone(), tess.clone())), - ((posts[5].clone(), tess.clone())), + (posts[0].clone(), sean.clone()), + (posts[1].clone(), sean.clone()), + (posts[4].clone(), tess.clone()), + (posts[5].clone(), tess.clone()), ]; assert_eq!(Ok(expected), data); @@ -237,14 +237,14 @@ fn distinct_of_multiple_columns() { .distinct_on((users::id, posts::body, posts::title)) .load(&mut connection); let expected = vec![ - ((posts[0].clone(), sean.clone())), - ((posts[2].clone(), sean.clone())), - ((posts[1].clone(), sean.clone())), - ((posts[3].clone(), sean.clone())), - ((posts[4].clone(), tess.clone())), - ((posts[6].clone(), tess.clone())), - ((posts[5].clone(), tess.clone())), - ((posts[7].clone(), tess.clone())), + (posts[0].clone(), sean.clone()), + (posts[2].clone(), sean.clone()), + (posts[1].clone(), sean.clone()), + (posts[3].clone(), sean), + (posts[4].clone(), tess.clone()), + (posts[6].clone(), tess.clone()), + (posts[5].clone(), tess.clone()), + (posts[7].clone(), tess), ]; assert_eq!(Ok(expected), data); From 630731de5bfbc7782a6d46be0049bbc40793dde5 Mon Sep 17 00:00:00 2001 From: Omid Rad Date: Thu, 25 May 2023 09:24:19 +0200 Subject: [PATCH 9/9] add more tests + cs --- diesel/src/pg/query_builder/distinct_on.rs | 16 +- ...tinct_on_requires_matching_order_clause.rs | 27 +++ ...t_on_requires_matching_order_clause.stderr | 190 +++++++++--------- diesel_tests/tests/distinct.rs | 26 ++- 4 files changed, 150 insertions(+), 109 deletions(-) diff --git a/diesel/src/pg/query_builder/distinct_on.rs b/diesel/src/pg/query_builder/distinct_on.rs index 0ac2856ce223..0177c8652cb3 100644 --- a/diesel/src/pg/query_builder/distinct_on.rs +++ b/diesel/src/pg/query_builder/distinct_on.rs @@ -18,14 +18,14 @@ pub struct DistinctOnClause(pub(crate) T); impl ValidOrderingForDistinct> for NoOrderClause {} impl ValidOrderingForDistinct> for OrderClause<(T,)> {} impl ValidOrderingForDistinct> for OrderClause where T: crate::Column {} -impl ValidOrderingForDistinct> for OrderClause> - where - T: crate::Column, -{} -impl ValidOrderingForDistinct> for OrderClause> - where - T: crate::Column, -{} +impl ValidOrderingForDistinct> for OrderClause> where + T: crate::Column +{ +} +impl ValidOrderingForDistinct> for OrderClause> where + T: crate::Column +{ +} macro_rules! valid_ordering { (@skip: ($ST1: ident, $($ST:ident,)*), $T1:ident, ) => {}; (@skip: ($ST1: ident, $($ST:ident,)*), $T1:ident, $($T:ident,)+) => { diff --git a/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs b/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs index a1374f4e4206..b6fbbbb30cb6 100644 --- a/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs +++ b/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs @@ -42,6 +42,33 @@ fn main() { .then_order_by(users::id) .distinct_on(users::name); + // same as above, with asc/desc + let _ = users::table + .order_by(users::name.asc()) + .distinct_on(users::name) + .into_boxed(); + let _ = users::table + .order_by((users::name.asc(), users::id.desc())) + .distinct_on(users::name) + .into_boxed(); + let _ = users::table + .order_by((users::name.asc(), users::id.desc())) + .distinct_on((users::name, users::id)) + .into_boxed(); + let _ = users::table + .order_by((users::name.asc(), users::id.desc())) + .distinct_on((users::name, users::id, users::hair_color)) + .into_boxed(); + let _ = users::table + .order_by((users::name.asc(), users::id.desc(), users::hair_color)) + .distinct_on((users::name, users::id)) + .into_boxed(); + let _ = users::table + .order_by(users::name.asc()) + .then_order_by(users::id) + .distinct_on(users::name) + .into_boxed(); + // verify that this all works with boxed queries let _ = users::table.distinct_on(users::name).into_boxed(); let _ = users::table diff --git a/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.stderr b/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.stderr index d79dbf2a3756..780196688dbc 100644 --- a/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.stderr +++ b/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.stderr @@ -1,109 +1,109 @@ error[E0277]: the trait bound `diesel::query_builder::order_clause::OrderClause: query_dsl::order_dsl::ValidOrderingForDistinct>` is not satisfied - --> tests/fail/distinct_on_requires_matching_order_clause.rs:76:58 - | -76 | let _ = users::table.order_by(users::id).distinct_on(users::name); - | ----------- ^^^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct>` is not implemented for `diesel::query_builder::order_clause::OrderClause` - | | - | required by a bound introduced by this call - | - = help: the following other types implement trait `query_dsl::order_dsl::ValidOrderingForDistinct`: - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - and $N others - = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` + --> tests/fail/distinct_on_requires_matching_order_clause.rs:103:58 + | +103 | let _ = users::table.order_by(users::id).distinct_on(users::name); + | ----------- ^^^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct>` is not implemented for `diesel::query_builder::order_clause::OrderClause` + | | + | required by a bound introduced by this call + | + = help: the following other types implement trait `query_dsl::order_dsl::ValidOrderingForDistinct`: + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + and $N others + = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` note: required by a bound in `diesel::QueryDsl::distinct_on` - --> $DIESEL/src/query_dsl/mod.rs - | - | Self: methods::DistinctOnDsl, - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `QueryDsl::distinct_on` + --> $DIESEL/src/query_dsl/mod.rs + | + | Self: methods::DistinctOnDsl, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `QueryDsl::distinct_on` error[E0271]: type mismatch resolving `::Column == name` - --> tests/fail/distinct_on_requires_matching_order_clause.rs:80:61 - | -80 | let _ = users::table.order_by((users::id, users::name)).distinct_on(users::name); - | ^^^^^^^^^^^ expected struct `id`, found struct `name` - | - = note: required for `diesel::query_builder::order_clause::OrderClause<(columns::id, columns::name)>` to implement `query_dsl::order_dsl::ValidOrderingForDistinct>` - = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` + --> tests/fail/distinct_on_requires_matching_order_clause.rs:107:61 + | +107 | let _ = users::table.order_by((users::id, users::name)).distinct_on(users::name); + | ^^^^^^^^^^^ expected struct `id`, found struct `name` + | + = note: required for `diesel::query_builder::order_clause::OrderClause<(columns::id, columns::name)>` to implement `query_dsl::order_dsl::ValidOrderingForDistinct>` + = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` error[E0271]: type mismatch resolving `::Column == name` - --> tests/fail/distinct_on_requires_matching_order_clause.rs:86:10 - | -86 | .distinct_on(users::name); - | ^^^^^^^^^^^ expected struct `id`, found struct `name` - | - = note: required for `diesel::query_builder::order_clause::OrderClause<(columns::id, columns::name)>` to implement `query_dsl::order_dsl::ValidOrderingForDistinct>` - = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` + --> tests/fail/distinct_on_requires_matching_order_clause.rs:113:10 + | +113 | .distinct_on(users::name); + | ^^^^^^^^^^^ expected struct `id`, found struct `name` + | + = note: required for `diesel::query_builder::order_clause::OrderClause<(columns::id, columns::name)>` to implement `query_dsl::order_dsl::ValidOrderingForDistinct>` + = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` error[E0277]: the trait bound `diesel::query_builder::order_clause::OrderClause: query_dsl::order_dsl::ValidOrderingForDistinct>` is not satisfied - --> tests/fail/distinct_on_requires_matching_order_clause.rs:90:60 - | -90 | let _ = users::table.distinct_on(users::name).order_by(users::id); - | -------- ^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct>` is not implemented for `diesel::query_builder::order_clause::OrderClause` - | | - | required by a bound introduced by this call - | - = help: the following other types implement trait `query_dsl::order_dsl::ValidOrderingForDistinct`: - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - and $N others - = note: required for `SelectStatement, DefaultSelectClause>, DistinctOnClause>` to implement `OrderDsl` + --> tests/fail/distinct_on_requires_matching_order_clause.rs:117:60 + | +117 | let _ = users::table.distinct_on(users::name).order_by(users::id); + | -------- ^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct>` is not implemented for `diesel::query_builder::order_clause::OrderClause` + | | + | required by a bound introduced by this call + | + = help: the following other types implement trait `query_dsl::order_dsl::ValidOrderingForDistinct`: + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + and $N others + = note: required for `SelectStatement, DefaultSelectClause>, DistinctOnClause>` to implement `OrderDsl` note: required by a bound in `order_by` - --> $DIESEL/src/query_dsl/mod.rs - | - | Self: methods::OrderDsl, - | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `QueryDsl::order_by` + --> $DIESEL/src/query_dsl/mod.rs + | + | Self: methods::OrderDsl, + | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `QueryDsl::order_by` error[E0277]: the trait bound `diesel::query_builder::order_clause::OrderClause: query_dsl::order_dsl::ValidOrderingForDistinct>` is not satisfied - --> tests/fail/distinct_on_requires_matching_order_clause.rs:93:58 - | -93 | let _ = users::table.order_by(users::id).distinct_on(users::name).into_boxed(); - | ----------- ^^^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct>` is not implemented for `diesel::query_builder::order_clause::OrderClause` - | | - | required by a bound introduced by this call - | - = help: the following other types implement trait `query_dsl::order_dsl::ValidOrderingForDistinct`: - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - as query_dsl::order_dsl::ValidOrderingForDistinct>> - and $N others - = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` + --> tests/fail/distinct_on_requires_matching_order_clause.rs:120:58 + | +120 | let _ = users::table.order_by(users::id).distinct_on(users::name).into_boxed(); + | ----------- ^^^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct>` is not implemented for `diesel::query_builder::order_clause::OrderClause` + | | + | required by a bound introduced by this call + | + = help: the following other types implement trait `query_dsl::order_dsl::ValidOrderingForDistinct`: + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + as query_dsl::order_dsl::ValidOrderingForDistinct>> + and $N others + = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` note: required by a bound in `diesel::QueryDsl::distinct_on` - --> $DIESEL/src/query_dsl/mod.rs - | - | Self: methods::DistinctOnDsl, - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `QueryDsl::distinct_on` + --> $DIESEL/src/query_dsl/mod.rs + | + | Self: methods::DistinctOnDsl, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `QueryDsl::distinct_on` error[E0271]: type mismatch resolving `::Column == name` - --> tests/fail/distinct_on_requires_matching_order_clause.rs:99:10 - | -99 | .distinct_on(users::name) - | ^^^^^^^^^^^ expected struct `id`, found struct `name` - | - = note: required for `diesel::query_builder::order_clause::OrderClause<(columns::id, columns::name)>` to implement `query_dsl::order_dsl::ValidOrderingForDistinct>` - = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` + --> tests/fail/distinct_on_requires_matching_order_clause.rs:126:10 + | +126 | .distinct_on(users::name) + | ^^^^^^^^^^^ expected struct `id`, found struct `name` + | + = note: required for `diesel::query_builder::order_clause::OrderClause<(columns::id, columns::name)>` to implement `query_dsl::order_dsl::ValidOrderingForDistinct>` + = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` error[E0277]: the trait bound `diesel::query_builder::order_clause::OrderClause<(columns::id, columns::name)>: query_dsl::order_dsl::ValidOrderingForDistinct>` is not satisfied - --> tests/fail/distinct_on_requires_matching_order_clause.rs:106:22 + --> tests/fail/distinct_on_requires_matching_order_clause.rs:133:22 | -106 | .distinct_on((users::name, users::id)) +133 | .distinct_on((users::name, users::id)) | ----------- ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct>` is not implemented for `diesel::query_builder::order_clause::OrderClause<(columns::id, columns::name)>` | | | required by a bound introduced by this call @@ -126,27 +126,27 @@ note: required by a bound in `diesel::QueryDsl::distinct_on` | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `QueryDsl::distinct_on` error[E0271]: type mismatch resolving `::Column == name` - --> tests/fail/distinct_on_requires_matching_order_clause.rs:113:10 + --> tests/fail/distinct_on_requires_matching_order_clause.rs:140:10 | -113 | .distinct_on((users::name, users::id)) +140 | .distinct_on((users::name, users::id)) | ^^^^^^^^^^^ expected struct `id`, found struct `name` | = note: required for `diesel::query_builder::order_clause::OrderClause` to implement `query_dsl::order_dsl::ValidOrderingForDistinct>` = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl<(columns::name, columns::id)>` error[E0271]: type mismatch resolving `::Column == name` - --> tests/fail/distinct_on_requires_matching_order_clause.rs:120:10 + --> tests/fail/distinct_on_requires_matching_order_clause.rs:147:10 | -120 | .distinct_on(users::name) +147 | .distinct_on(users::name) | ^^^^^^^^^^^ expected struct `id`, found struct `name` | = note: required for `diesel::query_builder::order_clause::OrderClause<(columns::id, columns::name)>` to implement `query_dsl::order_dsl::ValidOrderingForDistinct>` = note: required for `SelectStatement, DefaultSelectClause>, NoDistinctClause, ..., ...>` to implement `DistinctOnDsl` error[E0277]: the trait bound `diesel::query_builder::order_clause::OrderClause: query_dsl::order_dsl::ValidOrderingForDistinct>` is not satisfied - --> tests/fail/distinct_on_requires_matching_order_clause.rs:127:19 + --> tests/fail/distinct_on_requires_matching_order_clause.rs:154:19 | -127 | .order_by(users::id) +154 | .order_by(users::id) | -------- ^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct>` is not implemented for `diesel::query_builder::order_clause::OrderClause` | | | required by a bound introduced by this call diff --git a/diesel_tests/tests/distinct.rs b/diesel_tests/tests/distinct.rs index 3682f5f34dfb..a400b241f44c 100644 --- a/diesel_tests/tests/distinct.rs +++ b/diesel_tests/tests/distinct.rs @@ -174,10 +174,7 @@ fn distinct_of_multiple_columns() { .order(posts::body) .distinct_on(posts::body) .load(&mut connection); - let expected = vec![ - (posts[0].clone()), - (posts[7].clone()), - ]; + let expected = vec![(posts[0].clone()), (posts[7].clone())]; assert_eq!(Ok(expected), data); @@ -240,11 +237,28 @@ fn distinct_of_multiple_columns() { (posts[0].clone(), sean.clone()), (posts[2].clone(), sean.clone()), (posts[1].clone(), sean.clone()), - (posts[3].clone(), sean), + (posts[3].clone(), sean.clone()), (posts[4].clone(), tess.clone()), (posts[6].clone(), tess.clone()), (posts[5].clone(), tess.clone()), - (posts[7].clone(), tess), + (posts[7].clone(), tess.clone()), + ]; + + assert_eq!(Ok(expected), data); + + // multi order by + // multi distinct on + // including asc and desc + let data = posts::table + .inner_join(users::table) + .order((users::id.asc(), posts::body.desc(), posts::title)) + .distinct_on((users::id, posts::body)) + .load(&mut connection); + let expected = vec![ + (posts[1].clone(), sean.clone()), + (posts[0].clone(), sean), + (posts[5].clone(), tess.clone()), + (posts[4].clone(), tess), ]; assert_eq!(Ok(expected), data);