Implement on_conflict_clause::SupportsOnConflictClause when using #[derive(diesel::MultiConnection)]

[stormshield-gt]

It would be really nice if #[derive(diesel::MultiConnection)] implemented on_conflict_clause::SupportsOnConflictClause if each variant of the enum supports it.

Right now, if we have

#[derive(diesel::MultiConnection)]
enum AnyConnection {
    Postgresql(diesel::PgConnection),
    Sqlite(diesel::SqliteConnection),
}

The macro expension will do

impl diesel::backend::SqlDialect for MultiBackend {
    type OnConflictClause = diesel::internal::derives::multiconnection::sql_dialect::on_conflict_clause::DoesNotSupportOnConflictClause;
    // ...
}

[weiznich]

That would be indeed nice to have those impls. PR's for this are definitively welcome, otherwise I fear for me personally that's just nothing I can currently spend time on.

[stormshield-gt]

I tried to implement it but it's quite hard. I'm working with an enum of 2 variants for now

#[derive(diesel::MultiConnection)]
enum AnyConnection {
    Postgresql(diesel::PgConnection),
    Sqlite(diesel::SqliteConnection),
}

And I've added this to the macro

#[derive(Debug, Copy, Clone)]
pub struct AnyConnectionSupportOnConflict;

// sqlite supports less than Pg, we support only what sqlite supports
impl diesel::backend::sql_dialect::on_conflict_clause::SupportsOnConflictClause
    for AnyConnectionSupportOnConflict
{
}

impl diesel::backend::sql_dialect::on_conflict_clause::PgLikeOnConflictClause
    for AnyConnectionSupportOnConflict
{
}

Then I suppose I should implement QueryFragment<MultiBackend, <MultiBackend as SqlDialect>::OnConflictClause> for a type, so this blanket implementation will be triggered. Am I right ?

[weiznich]

From a high level view that's mostly correct.

For the QueryFragment impl:

• There might be others, you need to check where SupportsOnConflictClause is used to get a full list
• You don't need to write these impls on your own, you can just add the relevant types here:

  diesel/diesel_derives/src/multiconnection.rs

  Lines 1156 to 1205 in a5325d0

  	let query_fragment_impls = IntoIterator::into_iter([
  	quote::quote!{
  	<L, O> #query_fragment for diesel::internal::derives::multiconnection::LimitOffsetClause<L, O>
  	},
  	quote::quote! {
  	<L, R> diesel::query_builder::QueryFragment<super::backend::MultiBackend, super::backend::MultiConcatClauseSyntax>
  	for diesel::internal::derives::multiconnection::Concat<L, R>
  	},
  	quote::quote! {
  	<T, U> diesel::query_builder::QueryFragment<super::backend::MultiBackend, super::backend::MultiArrayComparisonSyntax>
  	for diesel::internal::derives::multiconnection::array_comparison::In<T, U>
  	},
  	quote::quote! {
  	<T, U> diesel::query_builder::QueryFragment<super::backend::MultiBackend, super::backend::MultiArrayComparisonSyntax>
  	for diesel::internal::derives::multiconnection::array_comparison::NotIn<T, U>
  	},
  	quote::quote! {
  	<ST, I> diesel::query_builder::QueryFragment<super::backend::MultiBackend, super::backend::MultiArrayComparisonSyntax>
  	for diesel::internal::derives::multiconnection::array_comparison::Many<ST, I>
  	},
  	quote::quote! {
  	<T> diesel::query_builder::QueryFragment<super::backend::MultiBackend, super::backend::MultiExistsSyntax>
  	for diesel::internal::derives::multiconnection::Exists<T>
  	},
  	quote::quote! {
  	diesel::query_builder::QueryFragment<super::backend::MultiBackend, super::backend::MultiEmptyFromClauseSyntax>
  	for diesel::internal::derives::multiconnection::NoFromClause
  	},
  	quote::quote! {
  	diesel::query_builder::QueryFragment<super::backend::MultiBackend, super::backend::MultiDefaultValueClauseForInsert>
  	for diesel::internal::derives::multiconnection::DefaultValues
  	},
  	quote::quote! {
  	<Expr> diesel::query_builder::QueryFragment<super::backend::MultiBackend, super::backend::MultiReturningClause>
  	for diesel::internal::derives::multiconnection::ReturningClause<Expr>
  	},
  	quote::quote! {
  	<Expr> diesel::query_builder::QueryFragment<super::backend::MultiBackend, super::backend::MultiInsertWithDefaultKeyword>
  	for diesel::insertable::DefaultableColumnInsertValue<Expr>
  	},
  	quote::quote! {
  	<Tab, V, QId, const HAS_STATIC_QUERY_ID: bool> diesel::query_builder::QueryFragment<super::backend::MultiBackend, super::backend::MultiBatchInsertSupport>
  	for diesel::internal::derives::multiconnection::BatchInsert<V, Tab, QId, HAS_STATIC_QUERY_ID>
  	},
  	quote::quote! {
  	<S> diesel::query_builder::QueryFragment<super::backend::MultiBackend, super::backend::MultiAliasSyntax>
  	for diesel::query_source::Alias<S>
  	}
  	])
  	.map(|t| generate_queryfragment_impls(t, &query_fragment_bounds));

For the PgLikeOnConflictClause: I'm not sure if that's always the correct thing to have. For example it wouldn't be correct if one of the connection types is MysqlConnection. Given that this macro also works with third party connection types (e.g those from diesel-oci, diesel-mssql) I also would like to avoid checking for specific names there. I'm not sure if we can have something like a conditional impl there which restricts this based on the actual backend impls, e.g. by having bounds like Sqlite::SupportsOnConflictClause: PgLikeOnConflictClause, Pg::SupportsOnConflictClause: PgLikeOnConflictClause there. Maybe that works, maybe we need to restructure how we express these condition in diesel.